Skip to content

Commit 608bdb4

Browse files
committed
Stop UI tests if an unknown revision name is specified
1 parent bd71213 commit 608bdb4

File tree

3 files changed

+63
-25
lines changed

3 files changed

+63
-25
lines changed

src/tools/compiletest/src/errors.rs

+31-11
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,11 @@ enum WhichLine {
7474
///
7575
/// If revision is not None, then we look
7676
/// for `//[X]~` instead, where `X` is the current revision.
77-
pub fn load_errors(testfile: &Path, revision: Option<&str>) -> Vec<Error> {
77+
pub fn load_errors(
78+
testfile: &Path,
79+
revision: Option<&str>,
80+
all_revisions: &[Option<&str>],
81+
) -> Vec<Error> {
7882
let rdr = BufReader::new(File::open(testfile).unwrap());
7983

8084
// `last_nonfollow_error` tracks the most recently seen
@@ -90,16 +94,20 @@ pub fn load_errors(testfile: &Path, revision: Option<&str>) -> Vec<Error> {
9094
rdr.lines()
9195
.enumerate()
9296
.filter_map(|(line_num, line)| {
93-
parse_expected(last_nonfollow_error, line_num + 1, &line.unwrap(), revision).map(
94-
|(which, error)| {
95-
match which {
96-
FollowPrevious(_) => {}
97-
_ => last_nonfollow_error = Some(error.line_num),
98-
}
99-
100-
error
101-
},
97+
parse_expected(
98+
last_nonfollow_error,
99+
line_num + 1,
100+
&line.unwrap(),
101+
revision,
102+
all_revisions,
102103
)
104+
.map(|(which, error)| {
105+
if !matches!(which, FollowPrevious(_)) {
106+
last_nonfollow_error = Some(error.line_num);
107+
}
108+
109+
error
110+
})
103111
})
104112
.collect()
105113
}
@@ -109,6 +117,7 @@ fn parse_expected(
109117
line_num: usize,
110118
line: &str,
111119
test_revision: Option<&str>,
120+
all_revisions: &[Option<&str>],
112121
) -> Option<(WhichLine, Error)> {
113122
// Matches comments like:
114123
// //~
@@ -125,7 +134,18 @@ fn parse_expected(
125134
match (test_revision, captures.name("revs")) {
126135
// Only error messages that contain our revision between the square brackets apply to us.
127136
(Some(test_revision), Some(revision_filters)) => {
128-
if !revision_filters.as_str().split(',').any(|r| r == test_revision) {
137+
let mut error_revisions = revision_filters.as_str().split(',');
138+
// Make sure all revisions are valid
139+
for rev in error_revisions.clone() {
140+
if !all_revisions.contains(&Some(rev)) {
141+
panic!(
142+
"found unexpected revision {rev} at line {line_num}. \
143+
Expected one of {all_revisions:?}"
144+
);
145+
}
146+
}
147+
148+
if !error_revisions.any(|r| r == test_revision) {
129149
return None;
130150
}
131151
}

src/tools/compiletest/src/lib.rs

+9-7
Original file line numberDiff line numberDiff line change
@@ -770,21 +770,21 @@ fn make_test(
770770
};
771771

772772
revisions
773-
.into_iter()
773+
.iter()
774774
.map(|revision| {
775775
let src_file =
776776
std::fs::File::open(&test_path).expect("open test file to parse ignores");
777-
let test_name = crate::make_test_name(&config, testpaths, revision);
777+
let test_name = crate::make_test_name(&config, testpaths, *revision);
778778
let mut desc = make_test_description(
779-
&config, cache, test_name, &test_path, src_file, revision, poisoned,
779+
&config, cache, test_name, &test_path, src_file, *revision, poisoned,
780780
);
781781
// Ignore tests that already run and are up to date with respect to inputs.
782782
if !config.force_rerun {
783-
desc.ignore |= is_up_to_date(&config, testpaths, &early_props, revision, inputs);
783+
desc.ignore |= is_up_to_date(&config, testpaths, &early_props, *revision, inputs);
784784
}
785785
test::TestDescAndFn {
786786
desc,
787-
testfn: make_test_closure(config.clone(), testpaths, revision),
787+
testfn: make_test_closure(config.clone(), testpaths, *revision, &revisions),
788788
}
789789
})
790790
.collect()
@@ -922,12 +922,14 @@ fn make_test_closure(
922922
config: Arc<Config>,
923923
testpaths: &TestPaths,
924924
revision: Option<&str>,
925+
all_revisions: &[Option<&str>],
925926
) -> test::TestFn {
926927
let config = config.clone();
927928
let testpaths = testpaths.clone();
928-
let revision = revision.map(str::to_owned);
929+
let revision = revision.map(ToOwned::to_owned);
930+
let all_revisions: Vec<_> = all_revisions.iter().map(|v| (*v).map(ToOwned::to_owned)).collect();
929931
test::DynTestFn(Box::new(move || {
930-
runtest::run(config, &testpaths, revision.as_deref());
932+
runtest::run(config, &testpaths, revision.as_deref(), &all_revisions);
931933
Ok(())
932934
}))
933935
}

src/tools/compiletest/src/runtest.rs

+23-7
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,14 @@ fn get_lib_name(lib: &str, aux_type: AuxType) -> Option<String> {
100100
}
101101
}
102102

103-
pub fn run(config: Arc<Config>, testpaths: &TestPaths, revision: Option<&str>) {
103+
pub fn run(
104+
config: Arc<Config>,
105+
testpaths: &TestPaths,
106+
revision: Option<&str>,
107+
all_revisions: &[Option<String>],
108+
) {
109+
let all_revisions_refs: Vec<_> = all_revisions.iter().map(Option::as_deref).collect();
110+
let all_revisions = &all_revisions_refs;
104111
match &*config.target {
105112
"arm-linux-androideabi"
106113
| "armv7-linux-androideabi"
@@ -133,7 +140,7 @@ pub fn run(config: Arc<Config>, testpaths: &TestPaths, revision: Option<&str>) {
133140
props.incremental_dir = Some(incremental_dir(&config, testpaths, revision));
134141
}
135142

136-
let cx = TestCx { config: &config, props: &props, testpaths, revision };
143+
let cx = TestCx { config: &config, props: &props, testpaths, revision, all_revisions };
137144
create_dir_all(&cx.output_base_dir())
138145
.with_context(|| {
139146
format!("failed to create output base directory {}", cx.output_base_dir().display())
@@ -155,6 +162,7 @@ pub fn run(config: Arc<Config>, testpaths: &TestPaths, revision: Option<&str>) {
155162
props: &revision_props,
156163
testpaths,
157164
revision: Some(revision),
165+
all_revisions,
158166
};
159167
rev_cx.run_revision();
160168
}
@@ -209,6 +217,8 @@ struct TestCx<'test> {
209217
props: &'test TestProps,
210218
testpaths: &'test TestPaths,
211219
revision: Option<&'test str>,
220+
/// List of all revisions for the test, so we can reject any that are unexpected
221+
all_revisions: &'test [Option<&'test str>],
212222
}
213223

214224
enum ReadFrom {
@@ -339,7 +349,8 @@ impl<'test> TestCx<'test> {
339349
self.check_no_compiler_crash(&proc_res, self.props.should_ice);
340350

341351
let output_to_check = self.get_output(&proc_res);
342-
let expected_errors = errors::load_errors(&self.testpaths.file, self.revision);
352+
let expected_errors =
353+
errors::load_errors(&self.testpaths.file, self.revision, self.all_revisions);
343354
if !expected_errors.is_empty() {
344355
if !self.props.error_patterns.is_empty() || !self.props.regex_error_patterns.is_empty()
345356
{
@@ -417,7 +428,8 @@ impl<'test> TestCx<'test> {
417428
}
418429

419430
// FIXME(#41968): Move this check to tidy?
420-
if !errors::load_errors(&self.testpaths.file, self.revision).is_empty() {
431+
if !errors::load_errors(&self.testpaths.file, self.revision, self.all_revisions).is_empty()
432+
{
421433
self.fatal("compile-pass tests with expected warnings should be moved to ui/");
422434
}
423435
}
@@ -432,7 +444,8 @@ impl<'test> TestCx<'test> {
432444
}
433445

434446
// FIXME(#41968): Move this check to tidy?
435-
if !errors::load_errors(&self.testpaths.file, self.revision).is_empty() {
447+
if !errors::load_errors(&self.testpaths.file, self.revision, self.all_revisions).is_empty()
448+
{
436449
self.fatal("run-pass tests with expected warnings should be moved to ui/");
437450
}
438451

@@ -1922,6 +1935,7 @@ impl<'test> TestCx<'test> {
19221935
props: &aux_props,
19231936
testpaths: &aux_testpaths,
19241937
revision: self.revision,
1938+
all_revisions: self.all_revisions,
19251939
};
19261940
// Create the directory for the stdout/stderr files.
19271941
create_dir_all(aux_cx.output_base_dir()).unwrap();
@@ -2183,6 +2197,7 @@ impl<'test> TestCx<'test> {
21832197
props: &aux_props,
21842198
testpaths: &aux_testpaths,
21852199
revision: self.revision,
2200+
all_revisions: self.all_revisions,
21862201
};
21872202
// Create the directory for the stdout/stderr files.
21882203
create_dir_all(aux_cx.output_base_dir()).unwrap();
@@ -3318,7 +3333,7 @@ impl<'test> TestCx<'test> {
33183333
.map(|line| str_to_mono_item(line, true))
33193334
.collect();
33203335

3321-
let expected: Vec<MonoItem> = errors::load_errors(&self.testpaths.file, None)
3336+
let expected: Vec<MonoItem> = errors::load_errors(&self.testpaths.file, None, &[])
33223337
.iter()
33233338
.map(|e| str_to_mono_item(&e.msg[..], false))
33243339
.collect();
@@ -4207,7 +4222,8 @@ impl<'test> TestCx<'test> {
42074222
);
42084223
}
42094224

4210-
let expected_errors = errors::load_errors(&self.testpaths.file, self.revision);
4225+
let expected_errors =
4226+
errors::load_errors(&self.testpaths.file, self.revision, self.all_revisions);
42114227

42124228
if let WillExecute::Yes = should_run {
42134229
let proc_res = self.exec_compiled_test();

0 commit comments

Comments
 (0)