move mir-opt miri tests to CI logic - #160422
Conversation
rustbot
added
A-CI
|
rustbot has assigned @Mark-Simulacrum. Use Why was this reviewer chosen?The reviewer was selected based on:
|
|
In general, I like to make bootstrap steps more composable and remove similar special cases from them. On the other hand, adding bespoke test commands into the CI bash scripts runs into the risk of making it less obvious to people what failed on CI, and how to reproduce it locally. Especially since this is configured via an environment variable, and bootstrap will not print it after a failure, unlike the command-line args. That being said, we already have a bunch of similar commands on CI, so no big deal. Just a remark. |
|
Hm actually strangely this does not seem to run any mir-opt tests any more. In the logs I just see |
Yeah that is a fair point. I think ideally we'd also print these env vars if the command fails but I am not sure how to best do that. Is there a shell mode like That said I think in this case the bespoke special case in |
|
We already use
👍
Hmm, strange indeed. It works for me locally. |
But that prints the command before it runs it. Ideally we'd have a mode that prints the command after it got run, but only if it failed. That's a separate discussion though. |
|
It looks like it's not actually doing expansion of the Oh, this uses |
891d7c1 to
d60de5c
Compare
|
Now it finished in 52min?^^ |
|
Well anyway the main point is it actually ran the mir-opt tests this time, so |
075edf5 to
548894b
Compare
| MIRIFLAGS=-Zmiri-provenance-gc=1 python3 "$X_PY" test --stage 2 miri miri/cargo-miri | ||
| else | ||
| python3 "$X_PY" test --stage 2 src/tools/miri src/tools/miri/cargo-miri | ||
| python3 "$X_PY" test --stage 2 miri miri/cargo-miri |
There was a problem hiding this comment.
Did you inline #160502 into this one? Btw it should be just cargo-miri, rather than miri/cargo-miri.
There was a problem hiding this comment.
Yeah I kind of it, as I was editing the file anyway I wanted tog et rid of the cargo-miri runs for looking at CI times here.
Btw it should be just cargo-miri, rather than miri/cargo-miri.
👍
548894b to
2d91ed5
Compare
also refactor check-miri a bit to make it easier to read
2d91ed5 to
c8e4034
Compare
rust-bors
Bot
added
S-waiting-on-bors
rust-bors
Bot
added
S-waiting-on-author
|
This pull request was unapproved. |
4 participants
It is useful to run Miri tests with mir-opts as that sometimes finds miscompilations.
However, doing so on every
./x test miriis annoying as it makes that take a lot longer, and it is inconsistent with./miri testin the Miri repo. So remove this logic there, and instead have the check-miri script in CI invoke the test suite a 2nd time with mir-opts enabled.Cc @rust-lang/miri