Move test suite to ui tests and bless stderr output #2032

oli-obk · 2022-03-17T13:50:06Z

fixes #2027

//~ annotations should check that the error happens on their line
document all the things

RalfJung · 2022-03-17T14:06:49Z

Nice, thanks!

I was not sure yet whether we'd want to merge the passing and failing tests... it is quite nice to be able to immediately see which kind of test this is, and this requires a lot more reorganization. But I guess it makes sense to follow rustc here.

oli-obk · 2022-03-17T14:42:57Z

Yea I was a bit surprised how the run-pass tests don't need any flags (like build-pass)... so maybe we should indeed keep them separate until that happens..

Anyway, right now we have a problem: paths to the sysroot are flaky, as they depend on where your sysroot is. Gotta figure out how rustc handles this.

oli-obk · 2022-03-17T15:33:36Z

I reverted the directory moves for now

oli-obk · 2022-03-17T17:13:52Z

hmm... gotta figure out how to handle the optimized stderr

RalfJung · 2022-03-17T17:52:16Z

Yea I was a bit surprised how the run-pass tests don't need any flags (like build-pass)... so maybe we should indeed keep them separate until that happens..

Yeah compiletest lags behind the rustc code by a lot by now, and fails to check the exist status most of them time. Plus things like Manishearth/compiletest-rs#123.

Maybe we should use https://crates.io/crates/trybuild instead?^^ (Not sure if that's actually possible.) EDIT: Looking at its API -- no that is probably not possible.

cargo-miri/bin.rs

tests/compiletest.rs

tests/run-pass/backtrace-api.stderr

miri

RalfJung · 2022-03-17T18:02:38Z

hmm... gotta figure out how to handle the optimized stderr

Ah, yeah, that has been a problem sometimes even with compile-fail and is a lot worse with ui.
Can we somehow tell compiletest to check the error annotations but not the reference files? Worst case, tell it to bless and un-do the blessing afterwards...

RalfJung · 2022-03-17T18:11:43Z

We'll also have to normalize Alloc IDs and SB Tags in many places. Is there a way to add a "default normalizer" to compiletest so we don't have to add the same normalize-stderr to dozens of tests?

oli-obk · 2022-03-17T19:40:05Z

We'll also have to normalize Alloc IDs and SB Tags in many places. Is there a way to add a "default normalizer" to compiletest so we don't have to add the same normalize-stderr to dozens of tests?

Yea, I haven't figured this out yet.. maybe I can add something to the miri binary instead that filters all the stderr output

RalfJung · 2022-03-17T19:47:28Z

Is patching compiletest an option? Shouldn't be too hard to add a new field to the config struct and using that to initialize TestProps.

bjorn3 · 2022-03-17T19:47:49Z

Anyway, right now we have a problem: paths to the sysroot are flaky, as they depend on where your sysroot is. Gotta figure out how rustc handles this.

You can set the CFG_VIRTUAL_RUST_SOURCE_BASE_DIR env var to the sysroot source path. Compiletest will then replace virtual_rust_source_base_dir.join("library") with the literal $SRC_DIR. At least that is the case with the compiletest version of rustc. I'm not sure if it also works with the version from crates.io. Maybe it needs a different way to configure this.

bors · 2022-03-20T02:59:16Z

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

RalfJung · 2022-05-17T11:49:44Z

@oli-obk what are your plans for this? I would love to see this happen. :)

oli-obk · 2022-05-17T12:07:37Z

I talked with pnkfelix about it yesterday. I'm going to add default filters to compiletest, and once that is merged, bump the crates.io version and then we can set some default filters here. That'll probably take a bit tho

RalfJung · 2022-05-17T12:22:14Z

Sounds like a great plan. :)

compiletest-rs is quite far behind the rustc version, FWIW. As in, many years. Doing a bump might be more like re-doing the extraction from scratch rather than updating the existing thing...

oli-obk · 2022-05-17T15:43:28Z

compiletest-rs is quite far behind the rustc version, FWIW. As in, many years. Doing a bump might be more like re-doing the extraction from scratch rather than updating the existing thing...

oof yea, I looked at this. And remembered I tried this before... Not sure what the best way forward is, but I think this is a larger project. I don't want to do the change just in the crates.io version, but re-doing a sync when it's just going to get out of sync again is also not great.

RalfJung · 2022-05-17T15:45:58Z

Yeah, just doing the change on crates.io is where I would have started. It's terrible but I gave up my hopes for this getting back in sync.^^

Or we go the low-tech way and simply have a common prelude in all our test files, and some ad-hoc hacky scripts to update it when needed...

oli-obk · 2022-05-17T21:20:10Z

Now I just gotta get back //~ ERROR annotations and we're good to go..

oli-obk · 2022-05-18T10:12:05Z

@RalfJung once I get all the targets working, I would like to merge this. While there are still minor bugs that I want to fix before merging, there is one major missing feature that I need to fix: //~ annotations do not check that the diagnostic happened on the line, just that it happened. Fixing this is not a problem, but it still requires a bit of work. Merging now will avoid forcing me to keep maintaining and rebasing this branch until everything works.

Opinions?

oli-obk · 2022-05-18T15:17:08Z

🚀 CI is passing.

I'll see if it still works if we run it from within the rustc repo

tests/compile-fail/extern_static.stderr

tests/run-fail/panic/unsupported_syscall.rs

tests/run-pass/stacked-borrows/generators-self-referential.rs

RalfJung · 2022-05-19T16:41:34Z

This is what I do.

The changes are just some minor things where we now show spans in libstd because the paths are available. I know we also have them here, so I don't know why that makes a difference

Ah, so that will be hard to normalize as well.
Spans in libstd are weird, yeah... but seems fine now to do as you suggest. 👍

CONTRIBUTING.md

cargo-miri/bin.rs

RalfJung · 2022-05-20T14:02:44Z

tests/compiletest.rs

-    ui(Mode::UB, "tests/compile-fail", &target);
+    ui(Mode::Pass, "tests/run-pass");
+    ui(Mode::Panic, "tests/run-fail");
+    ui(Mode::UB, "tests/compile-fail");


Looking at the names of the folders, maybe we should call the variants RunFail and CompileFail?

Though "compile-fail" is not a great name and we should probably rename the folder instead...

I didn't want to cause the folder renaming churn.

I want run-pass, run-panic and run-fail

I didn't want to cause the folder renaming churn.

Fair, we can leave that to a separate PR.

I want run-pass, run-panic and run-fail

The "run" part looks rather redundant in these names...

yeaaa... we don't really have compile-fail tests, so leaving out the run part makes sense

tests/compiletest.rs

ui_test/src/lib.rs

RalfJung · 2022-05-20T14:05:31Z

tests/compiletest.rs

@@ -123,5 +128,5 @@ fn main() {

    ui(Mode::Pass, "tests/run-pass");
    ui(Mode::Panic, "tests/run-fail");
-    ui(Mode::UB, "tests/compile-fail");
+    ui(Mode::Fail, "tests/compile-fail");


So what about "pass", "panic", and "fail" for the folder names?

oli-obk · 2022-05-24T11:11:42Z

I addressed all review comments

tests/compiletest.rs

RalfJung

Generally looks great! I left a bunch of comments/nits on the ui test library. I didn't re-check the test output itself; that part is probably easier to tweak post-landing. I also didn't check the details of how the library prints test failures, and the comment parsing. It seems to work well enough for now. :)

ui_test/README.md

CONTRIBUTING.md

ui_test/src/lib.rs

ui_test/src/comments.rs

ui_test/README.md

oli-obk · 2022-05-25T09:52:43Z

Addressed all comments and rebased (no squashing or other history editing)

ui_test/src/lib.rs

ui_test/README.md

RalfJung · 2022-05-25T13:05:16Z

Apart from these last two nits, LGTM. Please squash and land. :-)

oli-obk · 2022-05-25T13:38:38Z

@bors r+

oli-obk · 2022-05-25T13:41:09Z

@bors ping

bors · 2022-05-25T13:41:11Z

😪 I'm awake I'm awake

oli-obk · 2022-05-25T13:41:21Z

@bors r+

bors · 2022-05-25T13:41:23Z

📌 Commit f1756c3 has been approved by oli-obk

bors · 2022-05-25T13:41:31Z

⌛ Testing commit f1756c3 with merge 0a4279f...

bors · 2022-05-25T14:04:01Z

☀️ Test successful - checks-actions
Approved by: oli-obk
Pushing 0a4279f to master...

oli-obk force-pushed the gesundheit branch from afcd687 to 08d4fdd Compare March 17, 2022 13:51

oli-obk force-pushed the gesundheit branch from d6dd8a9 to b70464d Compare March 17, 2022 15:32

oli-obk force-pushed the gesundheit branch from cf52f0e to 5e3be82 Compare March 17, 2022 16:49

RalfJung reviewed Mar 17, 2022

View reviewed changes

cargo-miri/bin.rs Show resolved Hide resolved

RalfJung reviewed Mar 17, 2022

View reviewed changes

tests/compiletest.rs Outdated Show resolved Hide resolved

RalfJung reviewed Mar 17, 2022

View reviewed changes

tests/run-pass/backtrace-api.stderr Outdated Show resolved Hide resolved

RalfJung reviewed Mar 17, 2022

View reviewed changes

miri Show resolved Hide resolved

RalfJung mentioned this pull request Apr 13, 2022

Print spans where tags are created and invalidated #2030

Merged

oli-obk force-pushed the gesundheit branch 2 times, most recently from ab2648f to fe38f1b Compare May 17, 2022 21:19

oli-obk force-pushed the gesundheit branch from cad9b39 to d800b3f Compare May 18, 2022 10:09

RalfJung reviewed May 19, 2022

View reviewed changes

tests/compile-fail/extern_static.stderr Outdated Show resolved Hide resolved

RalfJung reviewed May 19, 2022

View reviewed changes

tests/run-fail/panic/unsupported_syscall.rs Show resolved Hide resolved

RalfJung reviewed May 19, 2022

View reviewed changes

tests/run-pass/stacked-borrows/generators-self-referential.rs Outdated Show resolved Hide resolved

oli-obk mentioned this pull request May 20, 2022

UI tests should check that //~ annotations actually match an error on that line #2131

Closed

2 tasks

RalfJung reviewed May 20, 2022

View reviewed changes

ui_test/src/lib.rs Outdated Show resolved Hide resolved

RalfJung reviewed May 20, 2022

View reviewed changes

RalfJung added the S-waiting-on-author Status: Waiting for the PR author to address review comments label May 23, 2022

oli-obk force-pushed the gesundheit branch from 6e8ebba to 13520cf Compare May 24, 2022 08:53

RalfJung reviewed May 24, 2022

View reviewed changes

tests/compiletest.rs Outdated Show resolved Hide resolved

RalfJung reviewed May 24, 2022

View reviewed changes

ui_test/src/comments.rs Outdated Show resolved Hide resolved

ui_test/README.md Outdated Show resolved Hide resolved

oli-obk force-pushed the gesundheit branch from c5b4459 to ddaa818 Compare May 25, 2022 09:51

RalfJung reviewed May 25, 2022

View reviewed changes

ui_test/src/lib.rs Show resolved Hide resolved

RalfJung reviewed May 25, 2022

View reviewed changes

ui_test/README.md Outdated Show resolved Hide resolved

Add a custom ui test runner and move all tests to it

f1756c3

oli-obk force-pushed the gesundheit branch from fcfa174 to f1756c3 Compare May 25, 2022 13:34

bors merged commit 0a4279f into master May 25, 2022

bors mentioned this pull request May 25, 2022

Optimizing Stacked Borrows (part 1?): Cache locations of Tags in a Borrow Stack #1935

Merged

Move test suite to ui tests and bless stderr output #2032

Move test suite to ui tests and bless stderr output #2032

Conversation

oli-obk commented Mar 17, 2022 • edited Loading

RalfJung commented Mar 17, 2022

oli-obk commented Mar 17, 2022

oli-obk commented Mar 17, 2022

oli-obk commented Mar 17, 2022

RalfJung commented Mar 17, 2022 • edited Loading

RalfJung commented Mar 17, 2022

RalfJung commented Mar 17, 2022

oli-obk commented Mar 17, 2022

RalfJung commented Mar 17, 2022

bjorn3 commented Mar 17, 2022

bors commented Mar 20, 2022

RalfJung commented May 17, 2022

oli-obk commented May 17, 2022

RalfJung commented May 17, 2022

oli-obk commented May 17, 2022

RalfJung commented May 17, 2022 • edited Loading

oli-obk commented May 17, 2022

oli-obk commented May 18, 2022

oli-obk commented May 18, 2022

RalfJung commented May 19, 2022

RalfJung May 20, 2022

Choose a reason for hiding this comment

oli-obk May 20, 2022

Choose a reason for hiding this comment

RalfJung May 20, 2022

Choose a reason for hiding this comment

oli-obk May 20, 2022

Choose a reason for hiding this comment

RalfJung May 20, 2022

Choose a reason for hiding this comment

oli-obk commented May 24, 2022

RalfJung left a comment

Choose a reason for hiding this comment

oli-obk commented May 25, 2022

RalfJung commented May 25, 2022

oli-obk commented May 25, 2022

oli-obk commented May 25, 2022

bors commented May 25, 2022

oli-obk commented May 25, 2022

bors commented May 25, 2022

bors commented May 25, 2022

bors commented May 25, 2022

oli-obk commented Mar 17, 2022 •

edited

Loading

RalfJung commented Mar 17, 2022 •

edited

Loading

RalfJung commented May 17, 2022 •

edited

Loading