Skip to content

compiletest: Read the whole test file before parsing directives #147431

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

compiletest: Read the whole test file before parsing directives #147431

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
37 changes: 13 additions & 24 deletions src/tools/compiletest/src/directives.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
use std::collections::HashSet;
use std::env;
use std::fs::File;
use std::io::BufReader;
use std::io::prelude::*;
use std::process::Command;
use std::{env, fs};

use camino::{Utf8Path, Utf8PathBuf};
use semver::Version;
Expand Down Expand Up @@ -54,18 +51,19 @@ pub struct EarlyProps {

impl EarlyProps {
pub fn from_file(config: &Config, testfile: &Utf8Path) -> Self {
let file = File::open(testfile.as_std_path()).expect("open test file to parse earlyprops");
Self::from_reader(config, testfile, file)
let file_contents =
fs::read_to_string(testfile).expect("read test file to parse earlyprops");
Self::from_file_contents(config, testfile, &file_contents)
}

pub fn from_reader<R: Read>(config: &Config, testfile: &Utf8Path, rdr: R) -> Self {
pub fn from_file_contents(config: &Config, testfile: &Utf8Path, file_contents: &str) -> Self {
let mut props = EarlyProps::default();
let mut poisoned = false;
iter_directives(
config.mode,
&mut poisoned,
testfile,
rdr,
file_contents,
// (dummy comment to force args into vertical layout)
&mut |ref ln: DirectiveLine<'_>| {
parse_and_update_aux(config, ln, testfile, &mut props.aux);
Expand Down Expand Up @@ -362,15 +360,15 @@ impl TestProps {
fn load_from(&mut self, testfile: &Utf8Path, test_revision: Option<&str>, config: &Config) {
let mut has_edition = false;
if !testfile.is_dir() {
let file = File::open(testfile.as_std_path()).unwrap();
let file_contents = fs::read_to_string(testfile).unwrap();
Copy link
Member

@jieyouxu jieyouxu Oct 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remark: (not for this PR) I've been meaning to actually handle these errors (or rather, not naively panic everywhere), but again, never got around to it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, sensible error handling in compiletest is on my mental list of things to do someday.


let mut poisoned = false;

iter_directives(
config.mode,
&mut poisoned,
testfile,
file,
&file_contents,
&mut |ref ln: DirectiveLine<'_>| {
if !ln.applies_to_test_revision(test_revision) {
return;
Expand Down Expand Up @@ -859,7 +857,7 @@ fn iter_directives(
mode: TestMode,
poisoned: &mut bool,
testfile: &Utf8Path,
rdr: impl Read,
file_contents: &str,
it: &mut dyn FnMut(DirectiveLine<'_>),
) {
if testfile.is_dir() {
Expand All @@ -886,16 +884,7 @@ fn iter_directives(
}
}

let mut rdr = BufReader::with_capacity(1024, rdr);
let mut ln = String::new();
let mut line_number = 0;

loop {
line_number += 1;
ln.clear();
if rdr.read_line(&mut ln).unwrap() == 0 {
break;
}
for (line_number, ln) in (1..).zip(file_contents.lines()) {
let ln = ln.trim();

let Some(directive_line) = line_directive(line_number, ln) else {
Expand Down Expand Up @@ -1359,13 +1348,13 @@ where
Some((min, max))
}

pub(crate) fn make_test_description<R: Read>(
pub(crate) fn make_test_description(
config: &Config,
cache: &DirectivesCache,
name: String,
path: &Utf8Path,
filterable_path: &Utf8Path,
src: R,
file_contents: &str,
test_revision: Option<&str>,
poisoned: &mut bool,
) -> CollectedTestDesc {
Expand All @@ -1380,7 +1369,7 @@ pub(crate) fn make_test_description<R: Read>(
config.mode,
&mut local_poisoned,
path,
src,
file_contents,
&mut |ref ln @ DirectiveLine { line_number, .. }| {
if !ln.applies_to_test_revision(test_revision) {
return;
Expand Down
36 changes: 16 additions & 20 deletions src/tools/compiletest/src/directives/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
use std::io::Read;

use camino::Utf8Path;
use semver::Version;

Expand All @@ -10,12 +8,12 @@ use crate::directives::{
};
use crate::executor::{CollectedTestDesc, ShouldPanic};

fn make_test_description<R: Read>(
fn make_test_description(
config: &Config,
name: String,
path: &Utf8Path,
filterable_path: &Utf8Path,
src: R,
file_contents: &str,
revision: Option<&str>,
) -> CollectedTestDesc {
let cache = DirectivesCache::load(config);
Expand All @@ -26,7 +24,7 @@ fn make_test_description<R: Read>(
name,
path,
filterable_path,
src,
file_contents,
revision,
&mut poisoned,
);
Expand Down Expand Up @@ -226,14 +224,13 @@ fn cfg() -> ConfigBuilder {
}

fn parse_rs(config: &Config, contents: &str) -> EarlyProps {
let bytes = contents.as_bytes();
EarlyProps::from_reader(config, Utf8Path::new("a.rs"), bytes)
EarlyProps::from_file_contents(config, Utf8Path::new("a.rs"), contents)
}

fn check_ignore(config: &Config, contents: &str) -> bool {
let tn = String::new();
let p = Utf8Path::new("a.rs");
let d = make_test_description(&config, tn, p, p, std::io::Cursor::new(contents), None);
let d = make_test_description(&config, tn, p, p, contents, None);
d.ignore
}

Expand All @@ -243,9 +240,9 @@ fn should_fail() {
let tn = String::new();
let p = Utf8Path::new("a.rs");

let d = make_test_description(&config, tn.clone(), p, p, std::io::Cursor::new(""), None);
let d = make_test_description(&config, tn.clone(), p, p, "", None);
assert_eq!(d.should_panic, ShouldPanic::No);
let d = make_test_description(&config, tn, p, p, std::io::Cursor::new("//@ should-fail"), None);
let d = make_test_description(&config, tn, p, p, "//@ should-fail", None);
assert_eq!(d.should_panic, ShouldPanic::Yes);
}

Expand Down Expand Up @@ -778,9 +775,8 @@ fn threads_support() {
}
}

fn run_path(poisoned: &mut bool, path: &Utf8Path, buf: &[u8]) {
let rdr = std::io::Cursor::new(&buf);
iter_directives(TestMode::Ui, poisoned, path, rdr, &mut |_| {});
fn run_path(poisoned: &mut bool, path: &Utf8Path, file_contents: &str) {
iter_directives(TestMode::Ui, poisoned, path, file_contents, &mut |_| {});
}

#[test]
Expand All @@ -789,7 +785,7 @@ fn test_unknown_directive_check() {
run_path(
&mut poisoned,
Utf8Path::new("a.rs"),
include_bytes!("./test-auxillary/unknown_directive.rs"),
include_str!("./test-auxillary/unknown_directive.rs"),
);
assert!(poisoned);
}
Expand All @@ -800,7 +796,7 @@ fn test_known_directive_check_no_error() {
run_path(
&mut poisoned,
Utf8Path::new("a.rs"),
include_bytes!("./test-auxillary/known_directive.rs"),
include_str!("./test-auxillary/known_directive.rs"),
);
assert!(!poisoned);
}
Expand All @@ -811,7 +807,7 @@ fn test_error_annotation_no_error() {
run_path(
&mut poisoned,
Utf8Path::new("a.rs"),
include_bytes!("./test-auxillary/error_annotation.rs"),
include_str!("./test-auxillary/error_annotation.rs"),
);
assert!(!poisoned);
}
Expand All @@ -822,29 +818,29 @@ fn test_non_rs_unknown_directive_not_checked() {
run_path(
&mut poisoned,
Utf8Path::new("a.Makefile"),
include_bytes!("./test-auxillary/not_rs.Makefile"),
include_str!("./test-auxillary/not_rs.Makefile"),
);
assert!(!poisoned);
}

#[test]
fn test_trailing_directive() {
let mut poisoned = false;
run_path(&mut poisoned, Utf8Path::new("a.rs"), b"//@ only-x86 only-arm");
run_path(&mut poisoned, Utf8Path::new("a.rs"), "//@ only-x86 only-arm");
assert!(poisoned);
}

#[test]
fn test_trailing_directive_with_comment() {
let mut poisoned = false;
run_path(&mut poisoned, Utf8Path::new("a.rs"), b"//@ only-x86 only-arm with comment");
run_path(&mut poisoned, Utf8Path::new("a.rs"), "//@ only-x86 only-arm with comment");
assert!(poisoned);
}

#[test]
fn test_not_trailing_directive() {
let mut poisoned = false;
run_path(&mut poisoned, Utf8Path::new("a.rs"), b"//@ revisions: incremental");
run_path(&mut poisoned, Utf8Path::new("a.rs"), "//@ revisions: incremental");
assert!(!poisoned);
}

Expand Down
5 changes: 3 additions & 2 deletions src/tools/compiletest/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -892,7 +892,8 @@ fn make_test(cx: &TestCollectorCx, collector: &mut TestCollector, testpaths: &Te
// `CollectedTest` that can be handed over to the test executor.
collector.tests.extend(revisions.into_iter().map(|revision| {
// Create a test name and description to hand over to the executor.
let src_file = fs::File::open(&test_path).expect("open test file to parse ignores");
let file_contents =
fs::read_to_string(&test_path).expect("read test file to parse ignores");
let (test_name, filterable_path) =
make_test_name_and_filterable_path(&cx.config, testpaths, revision);
// Create a description struct for the test/revision.
Expand All @@ -904,7 +905,7 @@ fn make_test(cx: &TestCollectorCx, collector: &mut TestCollector, testpaths: &Te
test_name,
&test_path,
&filterable_path,
src_file,
&file_contents,
revision,
&mut collector.poisoned,
);
Expand Down
Loading