On tests that specify `--color=always` emit SVG file with stderr output #121877

estebank · 2024-03-01T23:39:16Z

Leverage anstyle-svg, as cargo does now, to emit .svg files instead of .stderr files for tests that explicitly enable color output. This will make reviewing changes to the graphical output of tests much more human friendly.

rustbot · 2024-03-01T23:39:24Z

r? @Nadrieril

rustbot has assigned @Nadrieril.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

estebank · 2024-03-01T23:41:32Z

All credit to @epage for the library as well as @Muscraft for showing me rust-lang/cargo#13461.

rustbot · 2024-03-01T23:46:57Z

These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

compiler-errors · 2024-03-02T00:37:55Z

src/tools/compiletest/src/runtest.rs

@@ -4014,9 +4014,17 @@ impl<'test> TestCx<'test> {
        explicit_format: bool,
    ) -> usize {
        let stderr_bits = format!("{}bit.stderr", self.config.get_pointer_width());
+        let force_color_svg = self.props.compile_flags.iter().any(|s| s.contains("--color"))
+            && !self.config.target.contains("windows");


Why is this !windows necessary?

It's not absolutely necessary, but I don't have access to a windows box to ensure this test ran and the colors for the ANSI escapes is subtly different between windows and non-windows. Can change that and do a full build on CI to get the windows svg file if we want to land it with it.

For annotate-snippets, @Muscraft recently added a testing-colors feature flag to force the colors the same across all platforms so we don't have to deal with platform-specific colors (except for the places we need to to verify they are platform-specific)

I'm just worried that when ui testing on windows platforms, this will cause test failures since compiletest expects stdout files but there are svg files here.

I believe that the filter I included was for explicit windows platform request from the test file. I will remove the platform check.

@epage is testing-colors included in 0.1.3?

testing-colors is a annotate-snippets feature. Not really relevant until rustc is converted over but just giving an idea of strategies.

I believe that the filter I included was for explicit windows platform request from the test file. I will remove the platform check.

For the record, the variable self.config.target is set for all UI tests, so I believe this will produce observable behavioral differences in compiletest, and not just for tests tagged // only-windows/// ignore-windows.

I will remove the platform check.

Cool, that sounds like the easiest solution.

Another solution to this might be to pull the colors out into a separate crate and share them between rustc and annotate-snippets. I have thought about doing this before, but have never talked with enough people to see if it was a good idea. It would make it easier to share the colors during the transition period and allow other crates to match colors in the long run.

compiler-errors · 2024-03-02T02:12:09Z

@rustbot author

should be good to go after fixing and blessing the window test, but i'd like to take a look at the final results

estebank · 2024-03-02T17:55:51Z

@bors try

bors · 2024-03-02T17:57:01Z

⌛ Trying commit d8961f0 with merge 63ee2ef...

On tests that specify `--color=always` emit SVG file with stderr output Leverage `anstyle-svg`, as `cargo` does now, to emit `.svg` files instead of `.stderr` files for tests that explicitly enable color output. This will make reviewing changes to the graphical output of tests much more human friendly.

compiler-errors · 2024-03-02T18:16:09Z

For the record, I think bors try just builds artifacts for x86_64-unknown-linux-gnu. I think you need to copy the windows tests into the ci jobs in some actions toml file.

compiler-errors

r=me when ci is green

Leverage `anstyle-svg`, as `cargo` does now, to emit `.svg` files instead of `.stderr` files for tests that explicitly enable color output. This will make reviewing changes to the graphical output of tests much more human friendly.

estebank · 2024-03-02T23:44:45Z

@bors r=compiler-errors

bors · 2024-03-02T23:44:47Z

📌 Commit b4bdb56 has been approved by compiler-errors

It is now in the queue for this repository.

bors · 2024-03-03T06:40:58Z

⌛ Testing commit b4bdb56 with merge a09d91b...

bors · 2024-03-03T09:06:16Z

☀️ Test successful - checks-actions
Approved by: compiler-errors
Pushing a09d91b to master...

rust-timer · 2024-03-03T10:21:28Z

Finished benchmarking commit (a09d91b): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.3%	[-0.3%, -0.3%]	1
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	2.7%	[2.2%, 3.2%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-4.5%	[-4.9%, -4.0%]	2
All ❌✅ (primary)	-	-	0

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-1.1%	[-1.1%, -1.1%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-1.1%	[-1.1%, -1.1%]	1

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 650.632s -> 650.052s (-0.09%)
Artifact size: 175.02 MiB -> 175.05 MiB (0.02%)

rustbot assigned Nadrieril Mar 1, 2024

This comment has been minimized.

Sign in to view

estebank force-pushed the fancy-svg branch from b65b353 to 4387811 Compare March 1, 2024 23:46

compiler-errors reviewed Mar 2, 2024

View reviewed changes

compiler-errors self-assigned this Mar 2, 2024

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 2, 2024

compiler-errors unassigned Nadrieril Mar 2, 2024

estebank force-pushed the fancy-svg branch from 1f266ba to a5f1665 Compare March 2, 2024 20:44

estebank added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 2, 2024

compiler-errors approved these changes Mar 2, 2024

View reviewed changes

compiler-errors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 2, 2024

estebank force-pushed the fancy-svg branch from a5f1665 to 692f8f1 Compare March 2, 2024 21:25

This comment has been minimized.

Sign in to view

estebank force-pushed the fancy-svg branch from 692f8f1 to 4095c00 Compare March 2, 2024 22:36

estebank force-pushed the fancy-svg branch from 4095c00 to b4bdb56 Compare March 2, 2024 22:47

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 2, 2024

bors added the merged-by-bors This PR was explicitly merged by bors. label Mar 3, 2024

bors merged commit a09d91b into rust-lang:master Mar 3, 2024
12 checks passed

rustbot added this to the 1.78.0 milestone Mar 3, 2024

estebank mentioned this pull request Mar 5, 2024

When displaying multispans, ignore empty lines adjacent to ... #122029

Merged

estebank mentioned this pull request Mar 18, 2024

Track stderr color in UI tests #88254

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

On tests that specify `--color=always` emit SVG file with stderr output #121877

On tests that specify `--color=always` emit SVG file with stderr output #121877

estebank commented Mar 1, 2024 •

edited

Loading

rustbot commented Mar 1, 2024

estebank commented Mar 1, 2024

This comment has been minimized.

rustbot commented Mar 1, 2024

compiler-errors Mar 2, 2024 •

edited

Loading

estebank Mar 2, 2024

epage Mar 2, 2024

compiler-errors Mar 2, 2024

estebank Mar 2, 2024

epage Mar 2, 2024

compiler-errors Mar 2, 2024

Muscraft Mar 2, 2024

compiler-errors commented Mar 2, 2024

estebank commented Mar 2, 2024

bors commented Mar 2, 2024

compiler-errors commented Mar 2, 2024 •

edited

Loading

compiler-errors left a comment

This comment has been minimized.

estebank commented Mar 2, 2024

bors commented Mar 2, 2024

bors commented Mar 3, 2024

bors commented Mar 3, 2024

rust-timer commented Mar 3, 2024

On tests that specify --color=always emit SVG file with stderr output #121877

On tests that specify --color=always emit SVG file with stderr output #121877

Conversation

estebank commented Mar 1, 2024 • edited Loading

rustbot commented Mar 1, 2024

estebank commented Mar 1, 2024

This comment has been minimized.

rustbot commented Mar 1, 2024

compiler-errors Mar 2, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

compiler-errors commented Mar 2, 2024

estebank commented Mar 2, 2024

bors commented Mar 2, 2024

compiler-errors commented Mar 2, 2024 • edited Loading

compiler-errors left a comment

Choose a reason for hiding this comment

This comment has been minimized.

estebank commented Mar 2, 2024

bors commented Mar 2, 2024

bors commented Mar 3, 2024

bors commented Mar 3, 2024

rust-timer commented Mar 3, 2024

Overall result: ✅ improvements - no action needed

On tests that specify `--color=always` emit SVG file with stderr output #121877

On tests that specify `--color=always` emit SVG file with stderr output #121877

estebank commented Mar 1, 2024 •

edited

Loading

compiler-errors Mar 2, 2024 •

edited

Loading

compiler-errors commented Mar 2, 2024 •

edited

Loading