fix(test): drop serde_json dev-dependency to restore MSRV builds - #35
Open
JamBalaya56562 wants to merge 1 commit into
Open
fix(test): drop serde_json dev-dependency to restore MSRV builds#35JamBalaya56562 wants to merge 1 commit into
JamBalaya56562 wants to merge 1 commit into
Conversation
The example test harness gained a `serde_json` dev-dependency, but
`Cargo.lock.msrv` was never updated to match. The MSRV jobs restore that
lock file and let cargo fill in the gap, which resolves the newest
`serde_json` (1.0.151). That release declares `rust-version = "1.71"`,
so all three 1.63.0 jobs failed with:
error: package `serde_json v1.0.151` cannot be built because it
requires rustc 1.71 or newer, while the currently active rustc
version is 1.63.0
Pinning the dependency in `Cargo.lock.msrv` would work, but it also
means chasing the MSRV of `serde_json` and its transitive dependencies
(`itoa`, `ryu`, `serde`) from now on. Since only three fields of
cargo's artifact messages are needed, scan them directly instead and
drop the dependency again. The scanner tracks nesting and string
literals, so a key of the same name further down a message is never
mistaken for the one being looked for, and it unescapes string values so
that the backslashes in Windows executable paths survive.
The same commit also introduced a let-else, which is Rust 1.65 or later
and would have failed the MSRV jobs next; it is gone with the rewrite.
The new tests cover a real-world artifact message, escaped Windows
paths, structural characters inside string values, malformed input, the
escape sequences including surrogate pairs, and messages where the
fields exist but are shaped differently than cargo shapes them (a
`target` that is null, a string or an array, a non-array `kind`, and
non-string `name` and `executable` values).
Verified with the CI's own recipe (`cp Cargo.lock.msrv Cargo.lock` and
`cargo test --all`) on Rust 1.63.0, which now passes. With
`serde_json` gone, restoring the MSRV lock leaves it fully in sync
again.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CZz9vqNqNHujZgHxFMgPMy
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The
Testsworkflow is red onmain(d1356fd). Only the MSRV jobs fail; the latest-stable jobs pass:d1356fd("fix(test): resolve example binaries from Cargo output") added aserde_jsondev-dependency but did not updateCargo.lock.msrv. The MSRV jobs runcp Cargo.lock.msrv Cargo.lockand thencargo test --all, so cargo fills the gap by resolving the newestserde_json— 1.0.151, which declaresrust-version = "1.71":Reproduced locally on Rust 1.63.0 with the CI's own recipe.
The same commit also introduced a
let ... else, which is Rust 1.65 or later. That would have failed the MSRV jobs immediately after theserde_jsonproblem was solved.Fix
Pinning
serde_jsoninCargo.lock.msrvwould work, but it means tracking the MSRV ofserde_jsonplusitoa,ryuandserdefrom now on — the latest releases of all of those already require 1.68 or newer. Since the harness only needs three fields out of cargo's artifact messages, this scans them directly and drops the dependency again, leaving the crate with no dev-dependencies at all.Cargo.toml: remove the[dev-dependencies] serde_jsonsection.tests/test_examples.rs: replace theserde_json::Valuelookups with a small scanner (json_field/json_string/json_array_containsand askip_*cursor) plusparse_example_artifact.The behaviour
d1356fdwas after is preserved: example paths still come from cargo's--message-format=json-render-diagnosticsoutput rather than being derived from the test executable's location, so custom build directories such asCARGO_BUILD_BUILD_DIRkeep working. Thecargo build --examplesinvocation, theBUILT_EXAMPLEScache,compile_example()'s signature and every existing test are untouched.Two details the scanner gets right:
json_fieldonly ever matches a top-level key — a key of the same name deeper in the message is never mistaken for it.executablearrives as"C:\\src\\target\\debug\\examples\\hello.exe".Tests
Six unit tests are added for the scanner, covering a full real-world
compiler-artifactline, escaped Windows paths,"executable":nulland non-example targets, structural characters inside string values, malformed input, and the escape sequences (including\uXXXXsurrogate pairs). They are pure functions, so they add no measurable runtime.Verification
cargo test --allon stable — 13 tests + 5 doc-tests passmake lint(cargo clippy --all -- -F clippy::dbg-macro -D warnings) — cleanmake format-check— clean./demo.sh— exit 0cp Cargo.lock.msrv Cargo.lock && cargo test --allon Rust 1.63.0 — 13 tests + 5 doc-tests pass (fails before this change)serde_jsongone, restoringCargo.lock.msrvno longer pulls in anything new; the only remaining drift is the staleself-replaceversion entry (1.4.0vs1.5.0), which cargo rewrites harmlessly and which is left alone here.Note
Unrelated to this change:
test_self_delete_outside_path_force_exitis flaky on Linux, failing intermittently withExecutableFileBusy("Text file busy") when a copied example is executed while another test thread holds a write handle. This reproduces on unmodifiedd1356fdand is not addressed here.