Skip to content

Commit

Permalink
Auto merge of #121992 - jieyouxu:fix-tidy-unpaired-revision, r=onur-o…
Browse files Browse the repository at this point in the history
…zkan

tidy: split dots in filename not the entire path when checking for stray stdout/stderr files

I committed a path crime by splitting the entire path on `.`, when I meant to split on the filename. This means that any parent folders which contain `.` will cause tidy failure. Added a regression test so that doesn't happen again.

### Follow-up

- [ ] Adjust rustc-dev-guide to document assert on test name not containing dots. rust-lang/rustc-dev-guide#1927

Fixes #121986.
  • Loading branch information
bors committed Mar 5, 2024
2 parents bdde2a8 + 247a080 commit c7beecf
Show file tree
Hide file tree
Showing 5 changed files with 27 additions and 8 deletions.
26 changes: 19 additions & 7 deletions src/tools/tidy/src/tests_revision_unpaired_stdout_stderr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,10 +84,16 @@ pub fn check(tests_path: impl AsRef<Path>, bad: &mut bool) {
}
});

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

assert!(
!test_name.contains('.'),
"test name cannot contain dots '.': `{}`",
test.display()
);

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

Expand All @@ -98,14 +104,20 @@ pub fn check(tests_path: impl AsRef<Path>, bad: &mut bool) {
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(filename) = sibling.file_name().map(OsStr::to_str).flatten() else {
continue;
};

let filename_components = filename.split('.').collect::<Vec<_>>();
let [file_prefix, ..] = &filename_components[..] else {
continue;
};

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

match filename_components[..] {
match &filename_components[..] {
// Cannot have a revision component, skip.
[] | [_] => return,
[_, _] if !expected_revisions.is_empty() => {
Expand All @@ -120,9 +132,9 @@ pub fn check(tests_path: impl AsRef<Path>, bad: &mut bool) {
[_, _] => return,
[_, found_revision, .., extension] => {
if !IGNORES.contains(&found_revision)
&& !expected_revisions.contains(found_revision)
&& !expected_revisions.contains(*found_revision)
// This is from `//@ stderr-per-bitwidth`
&& !(extension == "stderr" && ["32bit", "64bit"].contains(&found_revision))
&& !(*extension == "stderr" && ["32bit", "64bit"].contains(&found_revision))
{
// Found some unexpected revision-esque component that is not a known
// compare-mode or expected revision.
Expand Down
File renamed without changes.
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
error: invalid character `'.'` in crate name: `need_crate_arg_ignore_tidy.x`
error: invalid character `'$'` in crate name: `need_crate_arg_ignore_tidy$x`
|
= help: you can either pass `--crate-name` on the command line or add `#![crate_name="…"]` to set the crate name

Expand Down
7 changes: 7 additions & 0 deletions tests/ui/meta/dir.with.dots/test.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
// Regression test for <https://github.com/rust-lang/rust/issues/121986>.
// Check that `tests_revision_unpaired_stdout_stderr` don't accidentally get confused by
// paths containing periods.

//@ check-pass

fn main() {}

0 comments on commit c7beecf

Please sign in to comment.