Skip to content

Commit

Permalink
Error on stray .stderr/.stdout files for (un-)revisioned tests
Browse files Browse the repository at this point in the history
  • Loading branch information
jieyouxu committed Feb 29, 2024
1 parent d3d145e commit f11713b
Show file tree
Hide file tree
Showing 6 changed files with 202 additions and 24 deletions.
32 changes: 32 additions & 0 deletions src/tools/tidy/src/iter_header.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
const COMMENT: &str = "//@";

/// A header line, like `//@name: value` consists of the prefix `//@` and the directive
/// `name: value`. It is also possibly revisioned, e.g. `//@[revision] name: value`.
pub(crate) struct HeaderLine<'ln> {
pub(crate) revision: Option<&'ln str>,
pub(crate) directive: &'ln str,
}

/// Iterate through compiletest headers in a test contents.
///
/// Adjusted from compiletest/src/header.rs.
pub(crate) fn iter_header<'ln>(contents: &'ln str, it: &mut dyn FnMut(HeaderLine<'ln>)) {
for ln in contents.lines() {
let ln = ln.trim();

// We're left with potentially `[rev]name: value`.
let Some(remainder) = ln.strip_prefix(COMMENT) else {
continue;
};

if let Some(remainder) = remainder.trim_start().strip_prefix('[') {
let Some((revision, remainder)) = remainder.split_once(']') else {
panic!("malformed revision directive: expected `//@[rev]`, found `{ln}`");
};
// We trimmed off the `[rev]` portion, left with `name: value`.
it(HeaderLine { revision: Some(revision), directive: remainder.trim() });
} else {
it(HeaderLine { revision: None, directive: remainder.trim() });
}
}
}
2 changes: 2 additions & 0 deletions src/tools/tidy/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ pub mod ext_tool_checks;
pub mod extdeps;
pub mod features;
pub mod fluent_alphabetical;
pub(crate) mod iter_header;
pub mod mir_opt_tests;
pub mod pal;
pub mod rustdoc_css_themes;
Expand All @@ -73,6 +74,7 @@ pub mod style;
pub mod target_policy;
pub mod target_specific_tests;
pub mod tests_placement;
pub mod tests_revision_unpaired_stdout_stderr;
pub mod ui_tests;
pub mod unit_tests;
pub mod unstable_book;
Expand Down
1 change: 1 addition & 0 deletions src/tools/tidy/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ fn main() {

// Checks over tests.
check!(tests_placement, &root_path);
check!(tests_revision_unpaired_stdout_stderr, &tests_path);
check!(debug_artifacts, &tests_path);
check!(ui_tests, &tests_path, bless);
check!(mir_opt_tests, &tests_path, bless);
Expand Down
28 changes: 4 additions & 24 deletions src/tools/tidy/src/target_specific_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,32 +4,12 @@
use std::collections::BTreeMap;
use std::path::Path;

use crate::iter_header::{iter_header, HeaderLine};
use crate::walk::filter_not_rust;

const COMMENT: &str = "//@";
const LLVM_COMPONENTS_HEADER: &str = "needs-llvm-components:";
const COMPILE_FLAGS_HEADER: &str = "compile-flags:";

/// Iterate through compiletest headers in a test contents.
///
/// Adjusted from compiletest/src/header.rs.
fn iter_header<'a>(contents: &'a str, it: &mut dyn FnMut(Option<&'a str>, &'a str)) {
for ln in contents.lines() {
let ln = ln.trim();
if ln.starts_with(COMMENT) && ln[COMMENT.len()..].trim_start().starts_with('[') {
if let Some(close_brace) = ln.find(']') {
let open_brace = ln.find('[').unwrap();
let lncfg = &ln[open_brace + 1..close_brace];
it(Some(lncfg), ln[(close_brace + 1)..].trim_start());
} else {
panic!("malformed condition directive: expected `//[foo]`, found `{ln}`")
}
} else if ln.starts_with(COMMENT) {
it(None, ln[COMMENT.len()..].trim_start());
}
}
}

#[derive(Default, Debug)]
struct RevisionInfo<'a> {
target_arch: Option<&'a str>,
Expand All @@ -40,9 +20,9 @@ pub fn check(path: &Path, bad: &mut bool) {
crate::walk::walk(path, |path, _is_dir| filter_not_rust(path), &mut |entry, content| {
let file = entry.path().display();
let mut header_map = BTreeMap::new();
iter_header(content, &mut |cfg, directive| {
iter_header(content, &mut |HeaderLine { revision, directive }| {
if let Some(value) = directive.strip_prefix(LLVM_COMPONENTS_HEADER) {
let info = header_map.entry(cfg).or_insert(RevisionInfo::default());
let info = header_map.entry(revision).or_insert(RevisionInfo::default());
let comp_vec = info.llvm_components.get_or_insert(Vec::new());
for component in value.split(' ') {
let component = component.trim();
Expand All @@ -56,7 +36,7 @@ pub fn check(path: &Path, bad: &mut bool) {
if let Some((arch, _)) =
v.trim_start_matches(|c| c == ' ' || c == '=').split_once("-")
{
let info = header_map.entry(cfg).or_insert(RevisionInfo::default());
let info = header_map.entry(revision).or_insert(RevisionInfo::default());
info.target_arch.replace(arch);
} else {
eprintln!("{file}: seems to have a malformed --target value");
Expand Down
146 changes: 146 additions & 0 deletions src/tools/tidy/src/tests_revision_unpaired_stdout_stderr.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,146 @@
//! Checks that there are no unpaired `.stderr` or `.stdout` for a test with and without revisions.
use std::collections::{BTreeMap, BTreeSet};
use std::ffi::OsStr;
use std::path::Path;

use crate::iter_header::*;
use crate::walk::*;

// Should be kept in sync with `CompareMode` in `src/tools/compiletest/src/common.rs`,
// as well as `run`.
const IGNORES: &[&str] = &[
"polonius",
"chalk",
"split-dwarf",
"split-dwarf-single",
"next-solver-coherence",
"next-solver",
"run",
];
const EXTENSIONS: &[&str] = &["stdout", "stderr"];
const SPECIAL_TEST: &str = "tests/ui/command/need-crate-arg-ignore-tidy.x.rs";

pub fn check(tests_path: impl AsRef<Path>, bad: &mut bool) {
// Recurse over subdirectories under `tests/`
walk_dir(tests_path.as_ref(), filter, &mut |entry| {
// We are inspecting a folder. Collect the paths to interesting files `.rs`, `.stderr`,
// `.stdout` under the current folder (shallow).
let mut files_under_inspection = BTreeSet::new();
for sibling in std::fs::read_dir(entry.path()).unwrap() {
let Ok(sibling) = sibling else {
continue;
};

if sibling.path().is_dir() {
continue;
}

let sibling_path = sibling.path();

let Some(ext) = sibling_path.extension().map(OsStr::to_str).flatten() else {
continue;
};

if ext == "rs" || EXTENSIONS.contains(&ext) {
files_under_inspection.insert(sibling_path);
}
}

let mut test_info = BTreeMap::new();

for test in
files_under_inspection.iter().filter(|f| f.extension().is_some_and(|ext| ext == "rs"))
{
if test.ends_with(SPECIAL_TEST) {
continue;
}

let mut expected_revisions = BTreeSet::new();

let contents = std::fs::read_to_string(test).unwrap();

// Collect directives.
iter_header(&contents, &mut |HeaderLine { revision, directive }| {
// We're trying to *find* `//@ revision: xxx` directives themselves, not revisioned
// directives.
if revision.is_some() {
return;
}

let directive = directive.trim();

if directive.starts_with("revisions") {
let Some((name, value)) = directive.split_once([':', ' ']) else {
return;
};

if name == "revisions" {
let revs = value.split(' ');
for rev in revs {
expected_revisions.insert(rev.to_owned());
}
}
}
});

let Some((test_name, _)) = test.to_str().map(|s| s.split_once('.')).flatten() else {
continue;
};

test_info.insert(test_name.to_string(), (test, expected_revisions));
}

// Our test file `foo.rs` has specified no revisions. There should not be any
// `foo.rev{.stderr,.stdout}` files. rustc-dev-guide says test output files can have names
// of the form: `test-name.revision.compare_mode.extension`, but our only concern is
// `test-name.revision` and `extension`.
for sibling in files_under_inspection.iter().filter(|f| {
f.extension().map(OsStr::to_str).flatten().is_some_and(|ext| EXTENSIONS.contains(&ext))
}) {
let filename_components = sibling.to_str().unwrap().split('.').collect::<Vec<_>>();
let file_prefix = filename_components[0];

let Some((test_path, expected_revisions)) = test_info.get(file_prefix) else {
continue;
};

match filename_components[..] {
// Cannot have a revision component, skip.
[] | [_] => return,
[_, _] if !expected_revisions.is_empty() => {
// Found unrevisioned output files for a revisioned test.
tidy_error!(
bad,
"found unrevisioned output file `{}` for a revisioned test `{}`",
sibling.display(),
test_path.display(),
);
}
[_, _] => return,
[_, found_revision, .., extension] => {
if !IGNORES.contains(&found_revision)
&& !expected_revisions.contains(found_revision)
// This is from `//@ stderr-per-bitwidth`
&& !(extension == "stderr" && ["32bit", "64bit"].contains(&found_revision))
{
// Found some unexpected revision-esque component that is not a known
// compare-mode or expected revision.
tidy_error!(
bad,
"found output file `{}` for unexpected revision `{}` of test `{}`",
sibling.display(),
found_revision,
test_path.display()
);
}
}
}
}
});
}

fn filter(path: &Path) -> bool {
filter_dirs(path) // ignore certain dirs
|| (path.file_name().is_some_and(|name| name == "auxiliary")) // ignore auxiliary folder
}
17 changes: 17 additions & 0 deletions src/tools/tidy/src/walk.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,3 +86,20 @@ pub(crate) fn walk_no_read(
}
}
}

// Walk through directories and skip symlinks.
pub(crate) fn walk_dir(
path: &Path,
skip: impl Send + Sync + 'static + Fn(&Path) -> bool,
f: &mut dyn FnMut(&DirEntry),
) {
let mut walker = ignore::WalkBuilder::new(path);
let walker = walker.filter_entry(move |e| !skip(e.path()));
for entry in walker.build() {
if let Ok(entry) = entry {
if entry.path().is_dir() {
f(&entry);
}
}
}
}

0 comments on commit f11713b

Please sign in to comment.