compiletest: ignore SVG y offset in by-lines comparison#157350
Conversation
SVG diagnostic snapshots (`--color=always`) render one terminal row per line. Under the parallel front-end, independent diagnostics can be emitted in a different order, which only permutes the rows and re-renders each at a different vertical `y` offset; the content of every row is unchanged. The parallel front-end already compares ui output as a sorted line multiset, but the `y` attribute is part of each line, so a pure reordering was still reported as a difference. Strip the `y` coordinate (and the `<svg>` header line, whose width and height are already ignored by the exact comparison) before the multiset comparison. The normalization is only used for the equality check; the original strings are kept for the diff and for blessing, and single-threaded runs keep the exact comparison including `y`. With this, the multiline-removal-suggestion test passes under the parallel front-end, so drop its ignore directive.
rustbot
added
A-compiletest
|
Some changes occurred in src/tools/compiletest cc @jieyouxu |
rustbot
added
S-waiting-on-review
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @clubby789 (or someone else) some time within the next two weeks. Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (
Why was this reviewer chosen?The reviewer was selected based on:
|
| // such a reordering is not reported as a difference. The normalization | ||
| // is comparison-only: on mismatch the original strings are kept for | ||
| // the diff and for blessing. Single-threaded runs take the | ||
| // exact-comparison path above and still check `y`. |
There was a problem hiding this comment.
@zetanumbers
At least the root cause analyses look plausible (although they need to be verified by a human of course), and may be potentially put into the tracking issue, even if the change is not merged.
There was a problem hiding this comment.
When running the SVG test under the parallel frontend at head, then any run fails the existing line by line comparison, and compiletest writes the actual rendered SVG to disk:
RS=tests/ui/error-emitter/multiline-removal-suggestion.rs
for i in $(seq 1 40); do
./x test $RS --test-args "--parallel-frontend-threads=16" --force-rerun
done
# each failing run prints: Saved the actual svg to `<build>/.../multiline-removal-suggestion.svg`
Both files are 456 lines, but the diagnostic blocks come out in a different vertical order:
A: E0277@21:8 E0277@53:28 E0277@43:7 E0277@32:6 E0277@50:2 E0277@39:2 ...
B: E0277@43:7 E0277@39:2 E0277@53:28 E0277@32:6 E0277@21:8 E0277@50:2 ...
Raw sorted line comparison, which is what the test does before this PR:
$ diff <(sort A.svg) <(sort B.svg) | grep -c '^[<>]'
232 # nonzero, so the comparison reports a difference
By comparison, with this PR, we drop the <svg> header line, replace y="..px" with a constant, then sort:
$ norm(){ tail -n +2 "$1" | sed -E 's/y="[0-9]+px"/y="0px"/g' | sort; }
$ diff <(norm A.svg) <(norm B.svg) | grep -c '^[<>]'
0 # identical as a multiset
Each differing line is the same row at a different height. For example the rows for the span at :39:2:
A: <tspan x="10px" y="1648px"> ... rs:39:2</tspan>
B: <tspan x="10px" y="370px"> ... rs:39:2</tspan>
The content is the same but the y changes. After the substitution both become y="0px" and match.
4 participants
Tracked in #154314.
SVG diagnostic snapshots (tests compiled with
--color=always) render one terminal row per line, laid out top to bottom. Under the parallel front-end, independent diagnostics can be emitted in a different order. That permutes the rows and re-renders each one at a different verticalyoffset, even though the content of every row is unchanged.The parallel front-end already compares ui test output as a sorted line multiset, but the
yattribute is part of each line's text, so a pure reordering was still reported as a difference. This is whytests/ui/error-emitter/multiline-removal-suggestion.rswas markedignore-parallel-frontend.This change strips the
ycoordinate (and the<svg>header line, whose width and height are already ignored by the exact comparison) before the multiset comparison, so a reordering of whole rows is no longer treated as a difference.