Stream: t-compiler

Topic: #52531-error-pattern-in-ui-tests


nikomatsakis (Aug 08 2018 at 09:48, on Zulip):

So @davidtwco points out #52531, filed by @Vadim Petrochenkov ...

nikomatsakis (Aug 08 2018 at 09:48, on Zulip):

ok so this code exists

nikomatsakis (Aug 08 2018 at 09:49, on Zulip):

I think the comments are backward, to start

nikomatsakis (Aug 08 2018 at 09:49, on Zulip):

e.g., this handles //~ ERROR this line

nikomatsakis (Aug 08 2018 at 09:50, on Zulip):

so if indeed they do not work (have we tested that?) the question is why this fn doesn't work:

https://github.com/rust-lang/rust/blob/master/src/tools/compiletest/src/runtest.rs#L1128

davidtwco (Aug 08 2018 at 09:50, on Zulip):

They don't work.

nikomatsakis (Aug 08 2018 at 09:51, on Zulip):

maybe to start I would dump what stderr field contains here:

https://github.com/rust-lang/rust/blob/master/src/tools/compiletest/src/runtest.rs#L2677

davidtwco (Aug 08 2018 at 09:52, on Zulip):

So, simply swapping those two lines fixes a bunch, the number of failures I'm seeing decreases a bunch - there are now only four no compare-mode failures - all where the error pattern is not found. And a bunch of nll compare-mode failures where compilation succeeds.

nikomatsakis (Aug 08 2018 at 09:52, on Zulip):

wait what

nikomatsakis (Aug 08 2018 at 09:53, on Zulip):

what is diff exactly?

davidtwco (Aug 08 2018 at 09:53, on Zulip):

That's down from a few hundred failures.

davidtwco (Aug 08 2018 at 09:53, on Zulip):

Swapping the branches of the if statement there.

nikomatsakis (Aug 08 2018 at 09:53, on Zulip):

like, the order of them?

nikomatsakis (Aug 08 2018 at 09:53, on Zulip):

can you paste the diff?

davidtwco (Aug 08 2018 at 09:53, on Zulip):
diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs
index c8f3956415..7ea1443c68 100644
--- a/src/tools/compiletest/src/runtest.rs
+++ b/src/tools/compiletest/src/runtest.rs
@@ -2670,11 +2670,11 @@ impl<'test> TestCx<'test> {
         }
         if !explicit && self.config.compare_mode.is_none() {
             if !expected_errors.is_empty() || !proc_res.status.success() {
-                // "// error-pattern" comments
-                self.check_expected_errors(expected_errors, &proc_res);
-            } else if !self.props.error_patterns.is_empty() || !proc_res.status.success() {
                 // "//~ERROR comments"
                 self.check_error_patterns(&proc_res.stderr, &proc_res);
+            } else if !self.props.error_patterns.is_empty() || !proc_res.status.success() {
+                // "// error-pattern" comments
+                self.check_expected_errors(expected_errors, &proc_res);
             }
         }
nikomatsakis (Aug 08 2018 at 09:54, on Zulip):

ok... that...seems wrong though

nikomatsakis (Aug 08 2018 at 09:54, on Zulip):

I think just the comments were wrong

nikomatsakis (Aug 08 2018 at 09:54, on Zulip):

now we are saying: "if the list of expected errors is non-empty, check the error patterns"

nikomatsakis (Aug 08 2018 at 09:54, on Zulip):

"expected errors" are the //~ ERROR annotations

nikomatsakis (Aug 08 2018 at 09:54, on Zulip):

I think we want to say "if the list of expected errors is non-empty, check them"

nikomatsakis (Aug 08 2018 at 09:54, on Zulip):

as we were saying before

nikomatsakis (Aug 08 2018 at 09:54, on Zulip):

though the if is quite weird

nikomatsakis (Aug 08 2018 at 09:55, on Zulip):

e.g., the || !proc_res.status.success()...

nikomatsakis (Aug 08 2018 at 09:55, on Zulip):

that part confuses me

davidtwco (Aug 08 2018 at 09:55, on Zulip):

Oh, yeah, oops.

davidtwco (Aug 08 2018 at 09:55, on Zulip):

:face_palm:

davidtwco (Aug 08 2018 at 09:56, on Zulip):

Not sure what I was quite thinking there.

davidtwco (Aug 08 2018 at 09:56, on Zulip):

Well, I'm fairly confused because I'm still seeing less errors.

nikomatsakis (Aug 08 2018 at 09:56, on Zulip):

one thing that always bothers me aout compiletest

nikomatsakis (Aug 08 2018 at 09:56, on Zulip):

is that there are no unit tests for it

nikomatsakis (Aug 08 2018 at 09:56, on Zulip):

I think you were seeing fewer errors because

nikomatsakis (Aug 08 2018 at 09:56, on Zulip):

we were checking for error patterns

nikomatsakis (Aug 08 2018 at 09:56, on Zulip):

and there weren't any

nikomatsakis (Aug 08 2018 at 09:56, on Zulip):

or..something

davidtwco (Aug 08 2018 at 09:57, on Zulip):

Yes, but after fixing that, I'm still seeing fewer errors.

davidtwco (Aug 08 2018 at 09:57, on Zulip):

4 fails in normal compare-mode, 49 in NLL compare-mode for the copied compile-fail tests.

davidtwco (Aug 08 2018 at 09:58, on Zulip):

Almost all of the 49 nll mode ones are successful compilations where that isn't expected.

nikomatsakis (Aug 08 2018 at 09:59, on Zulip):

Yes, but after fixing that, I'm still seeing fewer errors.

oh, like, no diff ...?

davidtwco (Aug 08 2018 at 09:59, on Zulip):

Well, only the comments.

davidtwco (Aug 08 2018 at 10:00, on Zulip):

I was definitely seeing errors that seemed to be related to error-patterns not being considered, but now I'm not. Maybe compiletest hasn't rebuilt?

nikomatsakis (Aug 08 2018 at 10:00, on Zulip):

are you using --keep-stage 1? :)

nikomatsakis (Aug 08 2018 at 10:00, on Zulip):

maybe that does something weird?

davidtwco (Aug 08 2018 at 10:01, on Zulip):

I did a clean, build and now test src/test/ui --keep-stage 0.

davidtwco (Aug 08 2018 at 10:02, on Zulip):

(and that command repeated a bunch)

nikomatsakis (Aug 08 2018 at 10:05, on Zulip):

I have no idea what --keep-stage 0 does here

nikomatsakis (Aug 08 2018 at 10:05, on Zulip):

but if you are tinkering with compiletest

nikomatsakis (Aug 08 2018 at 10:05, on Zulip):

I think you should not need it

nikomatsakis (Aug 08 2018 at 10:05, on Zulip):

it won't rebuild compiler or libstd

nikomatsakis (Aug 08 2018 at 10:05, on Zulip):

anyway maybe that is at fault?

nikomatsakis (Aug 08 2018 at 10:05, on Zulip):

you can always add some panic!() and see what happens :)

davidtwco (Aug 08 2018 at 10:54, on Zulip):

Had to run for a moment there, sorry about that.

davidtwco (Aug 08 2018 at 14:53, on Zulip):

#53196 fixes this.

Last update: Nov 22 2019 at 04:30UTC