Run all pass-dep tests natively, too #3948

oli-obk · 2024-10-07T13:25:06Z

fixes #3810

tests/pass/intptrcast.rs

oli-obk · 2024-10-07T17:27:36Z

unknown unstable option: miri-provenance-gc

lol oops, dependency building on linux does different stuff I guess

RalfJung · 2024-10-07T17:37:56Z

Yeah we set that flag here

miri/ci/ci.sh

Line 53 in 6e83c99

time MIRIFLAGS="${MIRIFLAGS-} -Zmiri-provenance-gc=1" ./miri test $TARGET_FLAG

GC_STRESS is disabled on Windows since the runner is so slow.^^

RalfJung

This looks great!

I wonder if it'd be worth to first only land this for pass-dep, also to keep the diff smaller. Then we can iron out the kinks for the large number of pass tests in a next step.

RalfJung · 2024-10-07T17:40:17Z

tests/ui.rs

+            exit_code: 0,
+            output_conflict_handling: Some(|path, actual, errors, config| {
+                let path = path.with_file_name(
+                    path.file_name().unwrap().to_str().unwrap().replace(".run.", "."),


Where does this .run. come from?

This is so you get foo.run.stderr that does not conflict with foo.stderr, which is for stderr during compilation

But this removes .run.

Is this to make sure that the output of the "run" is compared with the normal reference file for Miri?

RalfJung · 2024-10-07T17:40:41Z

tests/ui.rs

+                revision: &str,
+            ) -> bool {
+                for rev in comments.for_revision(revision) {
+                    for arg in &rev.compile_flags {


Shouldn't we just strip everything that starts with -Zmiri?

This is filtering out tests. Otherwise I need to add only-miri a lot

Ah... "strip" is a very confusing name then.

Hm, not sure how I feel about that. Feels a bit too implicit. This reaffirms my position that we should, at least in the first iteration, only do this for pass-dep tests.

Not sure what that changes. I would still have to do all the same changes in ui.rs

I will do that to reduce the scope of this PR and future unrelated PRs that may be affected, but the impl is just as complex

Well the logic for implicitly making tests only-miri when they use these particular flag could be removed, I think?

RalfJung · 2024-10-07T17:41:07Z

tests/ui.rs

+        });
+        config.program.envs.push(("MIRI_BE_RUSTC".into(), Some("host".into())));
+
+        #[derive(Debug)]


Suggested change

#[derive(Debug)]

/// A ui_test filter that strips `-Zmiri` flags.

#[derive(Debug)]

I guess the comment is wrong then. Please replace it by something that is correct :)

tests/ui.rs

tests/pass-dep/libc/libc-mem.rs

tests/pass/miri_start.rs

tests/pass/ptr_int_casts.rs

tests/pass/ptr_int_from_exposed.rs

tests/pass/tls/win_tls_callback.rs

tests/pass/zero-sized-accesses-and-offsets.rs

oli-obk · 2024-10-07T18:11:20Z

tests/ui.rs

+                    c.current_dir(dir);
+                }
+                c.args(
+                    cmd.get_args().filter(|arg| !arg.as_encoded_bytes().starts_with(b"-Zmiri-")),


This is the arg filter

More comments would help :)

RalfJung · 2024-10-08T10:33:32Z

tests/ui.rs

+        config.comment_defaults.base().add_custom("run", Run {
+            exit_code: 0,
+            output_conflict_handling: Some(|path, actual, errors, config| {
+                let path = path.with_file_name(


Suggested change

let path = path.with_file_name(

// ui_test would compare the output with `test.run.stdout`, but we want to

// compare with `test.stdout`, so adjust the filename.

let path = path.with_file_name(

At least I think that's what happens?

RalfJung · 2024-10-08T10:34:50Z

tests/ui.rs

+        if !config.host_matches_target() {
+            return Ok(());
+        }
+        config.output_conflict_handling = ui_test::ignore_output_conflict;


Suggested change

config.output_conflict_handling = ui_test::ignore_output_conflict;

// This is about the output during compilation. We want to ignore that.

// Above in `add_custom("run", ...)`, we set things up so that the output

// from *running* the program is compared as it should.

config.output_conflict_handling = ui_test::ignore_output_conflict;

RalfJung · 2024-10-08T18:13:17Z

tests/ui.rs

@@ -312,21 +440,30 @@ fn main() -> Result<()> {
        }
    }

-    ui(Mode::Pass, "tests/pass", &target, WithoutDependencies, tmpdir.path())?;
-    ui(Mode::Pass, "tests/pass-dep", &target, WithDependencies, tmpdir.path())?;
+    ui(Mode::Pass { rustc: false }, "tests/pass", &target, WithoutDependencies, tmpdir.path())?;


Can you also adjust the eprintln in fn ui to add something like "on rustc" or so, so that one can see which way the tests are being run?

bors · 2024-10-10T17:44:38Z

☔ The latest upstream changes (presumably #3950) made this pull request unmergeable. Please resolve the merge conflicts.

oli-obk added 3 commits October 6, 2024 10:58

Avoid a bool and use an Option of ZST instead

2253941

Remove a miri-specific note from test output

4bc0e25

Bump ui_test

d28fb80

oli-obk force-pushed the rustc_doublecheck branch 2 times, most recently from de41d01 to 1426114 Compare October 7, 2024 14:24

oli-obk commented Oct 7, 2024

View reviewed changes

tests/pass/intptrcast.rs Outdated Show resolved Hide resolved

RalfJung reviewed Oct 7, 2024

View reviewed changes

oli-obk commented Oct 7, 2024

View reviewed changes

RalfJung reviewed Oct 8, 2024

View reviewed changes

Run all run(-dep) tests natively, too

ca3fd79

oli-obk force-pushed the rustc_doublecheck branch from ad79e8f to ca3fd79 Compare October 8, 2024 16:50

oli-obk changed the title ~~Run all run(-dep)-pass tests natively, too~~ Run all pass-dep tests natively, too Oct 8, 2024

oli-obk added the S-waiting-on-author Status: Waiting for the PR author to address review comments label Oct 8, 2024

RalfJung reviewed Oct 8, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Run all pass-dep tests natively, too #3948

Run all pass-dep tests natively, too #3948

oli-obk commented Oct 7, 2024

oli-obk commented Oct 7, 2024

RalfJung commented Oct 7, 2024

RalfJung left a comment

RalfJung Oct 7, 2024

oli-obk Oct 7, 2024

RalfJung Oct 8, 2024

oli-obk Oct 8, 2024

RalfJung Oct 7, 2024

oli-obk Oct 7, 2024

RalfJung Oct 8, 2024

oli-obk Oct 8, 2024

RalfJung Oct 8, 2024

RalfJung Oct 7, 2024

RalfJung Oct 8, 2024

oli-obk Oct 7, 2024

RalfJung Oct 8, 2024

RalfJung Oct 8, 2024

RalfJung Oct 8, 2024

RalfJung Oct 8, 2024

bors commented Oct 10, 2024

	#[derive(Debug)]
	/// A ui_test filter that strips `-Zmiri` flags.
	#[derive(Debug)]

-        config.output_conflict_handling = ui_test::ignore_output_conflict;
+        // This is about the output during compilation. We want to ignore that.
+        // Above in `add_custom("run", ...)`, we set things up so that the output
+        // from *running* the program is compared as it should.
+        config.output_conflict_handling = ui_test::ignore_output_conflict;

Run all pass-dep tests natively, too #3948

Are you sure you want to change the base?

Run all pass-dep tests natively, too #3948

Conversation

oli-obk commented Oct 7, 2024

oli-obk commented Oct 7, 2024

RalfJung commented Oct 7, 2024

RalfJung left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bors commented Oct 10, 2024