Skip to content

Commit

Permalink
Remove //@ compare-output-lines-by-subset
Browse files Browse the repository at this point in the history
There was only ever one test which used this flag, and it was removed in #132244. I think this is a bad flag that should never have been added; comparing by subset makes the test failures extremely hard to debug. Any test that needs complicated output filtering like this should just use run-make instead.

Note that this does not remove the underlying comparison code, because it's still used if `runner` is set. I don't quite understand what's going on there, but since we still test on other platforms and in CI that the full output is accurate, I think it will be easier to debug than a test that uses compare-by-subset unconditionally.
  • Loading branch information
jyn514 committed Dec 2, 2024
1 parent 5bbbc09 commit 2f17ea0
Show file tree
Hide file tree
Showing 3 changed files with 4 additions and 18 deletions.
1 change: 0 additions & 1 deletion src/tools/compiletest/src/directive-list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ const KNOWN_DIRECTIVE_NAMES: &[&str] = &[
"check-run-results",
"check-stdout",
"check-test-line-numbers-match",
"compare-output-lines-by-subset",
"compile-flags",
"doc-flags",
"dont-check-compiler-stderr",
Expand Down
10 changes: 0 additions & 10 deletions src/tools/compiletest/src/header.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,9 +115,6 @@ pub struct TestProps {
pub dont_check_compiler_stdout: bool,
// For UI tests, allows compiler to generate arbitrary output to stderr
pub dont_check_compiler_stderr: bool,
// When checking the output of stdout or stderr check
// that the lines of expected output are a subset of the actual output.
pub compare_output_lines_by_subset: bool,
// Don't force a --crate-type=dylib flag on the command line
//
// Set this for example if you have an auxiliary test file that contains
Expand Down Expand Up @@ -240,7 +237,6 @@ mod directives {
pub const KNOWN_BUG: &'static str = "known-bug";
pub const TEST_MIR_PASS: &'static str = "test-mir-pass";
pub const REMAP_SRC_BASE: &'static str = "remap-src-base";
pub const COMPARE_OUTPUT_LINES_BY_SUBSET: &'static str = "compare-output-lines-by-subset";
pub const LLVM_COV_FLAGS: &'static str = "llvm-cov-flags";
pub const FILECHECK_FLAGS: &'static str = "filecheck-flags";
pub const NO_AUTO_CHECK_CFG: &'static str = "no-auto-check-cfg";
Expand Down Expand Up @@ -274,7 +270,6 @@ impl TestProps {
check_run_results: false,
dont_check_compiler_stdout: false,
dont_check_compiler_stderr: false,
compare_output_lines_by_subset: false,
no_prefer_dynamic: false,
pretty_mode: "normal".to_string(),
pretty_compare_only: false,
Expand Down Expand Up @@ -550,11 +545,6 @@ impl TestProps {
|s| s.trim().to_string(),
);
config.set_name_directive(ln, REMAP_SRC_BASE, &mut self.remap_src_base);
config.set_name_directive(
ln,
COMPARE_OUTPUT_LINES_BY_SUBSET,
&mut self.compare_output_lines_by_subset,
);

if let Some(flags) = config.parse_name_value_directive(ln, LLVM_COV_FLAGS) {
self.llvm_cov_flags.extend(split_flags(&flags));
Expand Down
11 changes: 4 additions & 7 deletions src/tools/compiletest/src/runtest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2541,13 +2541,10 @@ impl<'test> TestCx<'test> {
return 0;
}

// If `compare-output-lines-by-subset` is not explicitly enabled then
// auto-enable it when a `runner` is in use since wrapper tools might
// provide extra output on failure, for example a WebAssembly runtime
// might print the stack trace of an `unreachable` instruction by
// default.
let compare_output_by_lines =
self.props.compare_output_lines_by_subset || self.config.runner.is_some();
// Wrapper tools set by `runner` might provide extra output on failure,
// for example a WebAssembly runtime might print the stack trace of an
// `unreachable` instruction by default.
let compare_output_by_lines = self.config.runner.is_some();

let tmp;
let (expected, actual): (&str, &str) = if compare_output_by_lines {
Expand Down

0 comments on commit 2f17ea0

Please sign in to comment.