run-pass tests with non-zero exit code is considered passing

[RalfJung]

Currently, run-pass tests are considered successful even if the binary they run exits with a non-zero exit code. This is a critical bug, after all, checking if things ran successfully is the entire point of this crate!

This has been fixed upstream 2 years ago.

[Munksgaard]

I dream about doing an entire merge from upstream, you can see my progress in #201. However, this will remove the run-pass test mode. It seems like ui is used for those tests nowadays, would that work for you?

I don't know when #201 will be finished, so perhaps we could merge the upstream changes you're talking about in the meantime.

[RalfJung]

I dream about doing an entire merge from upstream, you can see my progress in #201

That's awesome ❤️

However, this will remove the run-pass test mode.

That'll be interesting, but I expected this to happen at some point when rustc started to remove run-pass. @oli-obk do you think we can use ui for Miri? The main problem will be making sure it doesn't try to parse the output as JSON.

@Munksgaard can we test this with the ui tests that compiletest already supports? IIRC a lot changed there upstream so I am not sure if that is a representative test.

[Munksgaard]

@Munksgaard can we test this with the ui tests that compiletest already supports? IIRC a lot changed there upstream so I am not sure if that is a representative test.

I haven't gotten a good overview of what's changed with regards to the particular testing modes yet, so I'm afraid I don't know. I also only have sporadic time to work on #201 at the moment, hence my disclaimer in the first comment :-)

[oli-obk]

The main problem will be making sure it doesn't try to parse the output as JSON.

UI test differentiate between stderr and stdout. stdout is dumped directly while stderr is parsed as json. This should not be a problem, because for ui tests we can make miri emit json diagnostics instead of user rendered diagnostics.

[jimblandy]

@RalfJung You don't happen to be running these tests in Emacs, do you?

I noticed that if the TERM environment variable is set to dumb, then I don't get a non-zero exit status, but if it's set to ansi, I do. Emacs, of course, sets TERM to dumb in *compilation* buffers.

For a while I worked around this by running TERM=ansi cargo test, but eventually I just put this in my runtests.rs file:

    // Work around https://github.com/laumann/compiletest-rs/issues/210
    // Emacs sets `TERM` to `dumb` in `*compilation*` buffers, and this
    // seems to impede compiletest's ability to detect failures.
    if env::var("TERM") == Ok("dumb".to_string()) {
        // Maybe this is not the correct value for `TERM`, but getting garbage
        // on your screen is better than getting failures not reported.
        env::set_var("TERM", "ansi");
    }

[RalfJung]

No, I am running them directly in my console. Also I have no idea how the TERM env var could have any effect here, that's really strange.

But also Miri moved to "ui" tests since then, which might behave differently wrt status code checking.