Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

compiletest should note when test output is normalized #125537

Open
jyn514 opened this issue May 25, 2024 · 2 comments
Open

compiletest should note when test output is normalized #125537

jyn514 opened this issue May 25, 2024 · 2 comments
Labels
A-compiletest Area: The compiletest test runner A-contributor-roadblock Area: Makes things more difficult for new contributors to rust itself A-diagnostics Area: Messages for errors, warnings, and lints A-testsuite Area: The testsuite used to check the correctness of rustc D-confusing Diagnostics: Confusing error or lint that should be reworked. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)

Comments

@jyn514
Copy link
Member

jyn514 commented May 25, 2024

compiletest has two basic kinds of normalization:

  1. normalization it does itself, either built-in or through a normalize-stderr pattern. this is what the UI suite uses.
  2. normalization it doesn't know about, e.g. in a run-make test

normalizing outputs can lead to some extremely confusing errors, such as #121571 (comment). to avoid misleading contributors, it should note when the output has been normalized.

the first part is not too hard; it can save list of lines that have been modified in each test and print something like "note: 32/32 lines that did not match were normalized before comparison; see build/test/ui/foo.out.orig for unnormalized version"
(note that the unnormalized file already exists today but almost nobody knows about it).

the second part is trickier; run-make tests often run sed before comparing, and this is opaque to compiletest. perhaps fixing this can be part of the ongoing work to switch run-make to rust? and, in the meantime, introduce a new SED script, based off cut-and-grep.sh, to prevent the problem from getting worse? cc @jieyouxu

@rustbot label A-contributor-roadblock A-diagnostics D-confusing A-testsuite

@rustbot rustbot added needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. A-contributor-roadblock Area: Makes things more difficult for new contributors to rust itself A-diagnostics Area: Messages for errors, warnings, and lints A-testsuite Area: The testsuite used to check the correctness of rustc D-confusing Diagnostics: Confusing error or lint that should be reworked. labels May 25, 2024
@saethlin saethlin added T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels May 25, 2024
@jieyouxu
Copy link
Member

run-make tests often run sed before comparing, and this is opaque to compiletest. perhaps fixing this can be part of the ongoing work to switch run-make to rust? and, in the meantime, introduce a new SED script, based off cut-and-grep.sh, to prevent the problem from getting worse?

yes, if we're porting any run-make test that relies on some kind of normalization on some output, we should definitely make the error output as helpful as possible (i.e. a helper in the support lib), that basically on top of reporting what post-normalization check failed, also report what the pre-normalization output was.

@jyn514
Copy link
Member Author

jyn514 commented Dec 2, 2024

#133733 does the UI test side of this. the run-make bit should be simpler, actually, since the codebase is much less complicated. it just needs a couple lines around

for (regex, replacement) in &self.normalizers {
let re = Regex::new(regex).expect("bad regex in custom normalization rule");
actual = re.replace_all(&actual, replacement).into_owned();
}
let output = TextDiff::from_lines(expected, &actual)
.unified_diff()
.header(expected_name, actual_name)
.to_string();
(expected_name, actual_name, output, actual)
}
#[track_caller]
pub fn run(&mut self) {
self.drop_bomb.defuse();
let (expected_name, actual_name, output, actual) = self.run_common();
if !output.is_empty() {
if self.maybe_bless_expected_file(&actual) {
return;
}
panic!(
i think. you'll probably want to reuse the "diff-of-a-diff" logic from #133733, you can extract that into build_helper if that makes things easier.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-compiletest Area: The compiletest test runner A-contributor-roadblock Area: Makes things more difficult for new contributors to rust itself A-diagnostics Area: Messages for errors, warnings, and lints A-testsuite Area: The testsuite used to check the correctness of rustc D-confusing Diagnostics: Confusing error or lint that should be reworked. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
Development

No branches or pull requests

4 participants