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.

Sign up for GitHub

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

Stop UI tests if an unknown revision name is specified #123882

Closed
wants to merge 1 commit into from
Closed

Stop UI tests if an unknown revision name is specified #123882

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 31 additions & 11 deletions src/tools/compiletest/src/errors.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,11 @@ enum WhichLine {
///
/// If revision is not None, then we look
/// for `//[X]~` instead, where `X` is the current revision.
pub fn load_errors(testfile: &Path, revision: Option<&str>) -> Vec<Error> {
pub fn load_errors(
testfile: &Path,
revision: Option<&str>,
all_revisions: &[Option<&str>],
) -> Vec<Error> {
let rdr = BufReader::new(File::open(testfile).unwrap());

// `last_nonfollow_error` tracks the most recently seen
Expand All @@ -90,16 +94,20 @@ pub fn load_errors(testfile: &Path, revision: Option<&str>) -> Vec<Error> {
rdr.lines()
.enumerate()
.filter_map(|(line_num, line)| {
parse_expected(last_nonfollow_error, line_num + 1, &line.unwrap(), revision).map(
|(which, error)| {
match which {
FollowPrevious(_) => {}
_ => last_nonfollow_error = Some(error.line_num),
}

error
},
parse_expected(
last_nonfollow_error,
line_num + 1,
&line.unwrap(),
revision,
all_revisions,
)
.map(|(which, error)| {
if !matches!(which, FollowPrevious(_)) {
last_nonfollow_error = Some(error.line_num);
}

error
})
})
.collect()
}
Expand All @@ -109,6 +117,7 @@ fn parse_expected(
line_num: usize,
line: &str,
test_revision: Option<&str>,
all_revisions: &[Option<&str>],
) -> Option<(WhichLine, Error)> {
// Matches comments like:
// //~
Expand All @@ -125,7 +134,18 @@ fn parse_expected(
match (test_revision, captures.name("revs")) {
// Only error messages that contain our revision between the square brackets apply to us.
(Some(test_revision), Some(revision_filters)) => {
if !revision_filters.as_str().split(',').any(|r| r == test_revision) {
let mut error_revisions = revision_filters.as_str().split(',');
// Make sure all revisions are valid
for rev in error_revisions.clone() {
if !all_revisions.contains(&Some(rev)) {
panic!(
"found unexpected revision '{rev}' at line {line_num}. \
Expected one of {all_revisions:?}"
);
}
}

if !error_revisions.any(|r| r == test_revision) {
return None;
}
}
Expand Down
16 changes: 9 additions & 7 deletions src/tools/compiletest/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -770,21 +770,21 @@ fn make_test(
};

revisions
.into_iter()
.iter()
.map(|revision| {
let src_file =
std::fs::File::open(&test_path).expect("open test file to parse ignores");
let test_name = crate::make_test_name(&config, testpaths, revision);
let test_name = crate::make_test_name(&config, testpaths, *revision);
let mut desc = make_test_description(
&config, cache, test_name, &test_path, src_file, revision, poisoned,
&config, cache, test_name, &test_path, src_file, *revision, poisoned,
);
// Ignore tests that already run and are up to date with respect to inputs.
if !config.force_rerun {
desc.ignore |= is_up_to_date(&config, testpaths, &early_props, revision, inputs);
desc.ignore |= is_up_to_date(&config, testpaths, &early_props, *revision, inputs);
}
test::TestDescAndFn {
desc,
testfn: make_test_closure(config.clone(), testpaths, revision),
testfn: make_test_closure(config.clone(), testpaths, *revision, &revisions),
}
})
.collect()
Expand Down Expand Up @@ -922,12 +922,14 @@ fn make_test_closure(
config: Arc<Config>,
testpaths: &TestPaths,
revision: Option<&str>,
all_revisions: &[Option<&str>],
) -> test::TestFn {
let config = config.clone();
let testpaths = testpaths.clone();
let revision = revision.map(str::to_owned);
let revision = revision.map(ToOwned::to_owned);
let all_revisions: Vec<_> = all_revisions.iter().map(|v| (*v).map(ToOwned::to_owned)).collect();
test::DynTestFn(Box::new(move || {
runtest::run(config, &testpaths, revision.as_deref());
runtest::run(config, &testpaths, revision.as_deref(), &all_revisions);
Ok(())
}))
}
Expand Down
30 changes: 23 additions & 7 deletions src/tools/compiletest/src/runtest.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,14 @@ fn get_lib_name(lib: &str, aux_type: AuxType) -> Option<String> {
}
}

pub fn run(config: Arc<Config>, testpaths: &TestPaths, revision: Option<&str>) {
pub fn run(
config: Arc<Config>,
testpaths: &TestPaths,
revision: Option<&str>,
all_revisions: &[Option<String>],
) {
let all_revisions_refs: Vec<_> = all_revisions.iter().map(Option::as_deref).collect();
let all_revisions = &all_revisions_refs;
match &*config.target {
"arm-linux-androideabi"
| "armv7-linux-androideabi"
Expand Down Expand Up @@ -133,7 +140,7 @@ pub fn run(config: Arc<Config>, testpaths: &TestPaths, revision: Option<&str>) {
props.incremental_dir = Some(incremental_dir(&config, testpaths, revision));
}

let cx = TestCx { config: &config, props: &props, testpaths, revision };
let cx = TestCx { config: &config, props: &props, testpaths, revision, all_revisions };
create_dir_all(&cx.output_base_dir())
.with_context(|| {
format!("failed to create output base directory {}", cx.output_base_dir().display())
Expand All @@ -155,6 +162,7 @@ pub fn run(config: Arc<Config>, testpaths: &TestPaths, revision: Option<&str>) {
props: &revision_props,
testpaths,
revision: Some(revision),
all_revisions,
};
rev_cx.run_revision();
}
Expand Down Expand Up @@ -209,6 +217,8 @@ struct TestCx<'test> {
props: &'test TestProps,
testpaths: &'test TestPaths,
revision: Option<&'test str>,
/// List of all revisions for the test, so we can reject any that are unexpected
all_revisions: &'test [Option<&'test str>],
}

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

let output_to_check = self.get_output(&proc_res);
let expected_errors = errors::load_errors(&self.testpaths.file, self.revision);
let expected_errors =
errors::load_errors(&self.testpaths.file, self.revision, self.all_revisions);
if !expected_errors.is_empty() {
if !self.props.error_patterns.is_empty() || !self.props.regex_error_patterns.is_empty()
{
Expand Down Expand Up @@ -417,7 +428,8 @@ impl<'test> TestCx<'test> {
}

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

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

Expand Down Expand Up @@ -1922,6 +1935,7 @@ impl<'test> TestCx<'test> {
props: &aux_props,
testpaths: &aux_testpaths,
revision: self.revision,
all_revisions: self.all_revisions,
};
// Create the directory for the stdout/stderr files.
create_dir_all(aux_cx.output_base_dir()).unwrap();
Expand Down Expand Up @@ -2183,6 +2197,7 @@ impl<'test> TestCx<'test> {
props: &aux_props,
testpaths: &aux_testpaths,
revision: self.revision,
all_revisions: self.all_revisions,
};
// Create the directory for the stdout/stderr files.
create_dir_all(aux_cx.output_base_dir()).unwrap();
Expand Down Expand Up @@ -3318,7 +3333,7 @@ impl<'test> TestCx<'test> {
.map(|line| str_to_mono_item(line, true))
.collect();

let expected: Vec<MonoItem> = errors::load_errors(&self.testpaths.file, None)
let expected: Vec<MonoItem> = errors::load_errors(&self.testpaths.file, None, &[])
.iter()
.map(|e| str_to_mono_item(&e.msg[..], false))
.collect();
Expand Down Expand Up @@ -4207,7 +4222,8 @@ impl<'test> TestCx<'test> {
);
}

let expected_errors = errors::load_errors(&self.testpaths.file, self.revision);
let expected_errors =
errors::load_errors(&self.testpaths.file, self.revision, self.all_revisions);

if let WillExecute::Yes = should_run {
let proc_res = self.exec_compiled_test();
Expand Down
Loading