Check //~ERROR comments in ui tests #46116

oli-obk · 2017-11-20T15:26:50Z

cc #44844 @Phlosioneer @estebank @petrochenkov

this depends on #46052 getting merged first (the commits are included in here)

The relevant changes of this PR are c2f0af7 and 979269b

nikomatsakis · 2017-11-20T17:54:09Z

I'm very excited to see this PR!

One minor thing is that I think there was some minor advantage in trying to capture the exact output we gave to the user (versus relying on JSON). But I think having just a test or two that checks that would be good enough -- and really we weren't checking colors anyhow.

zackmdavis · 2017-11-20T20:16:20Z

src/librustc/session/mod.rs

-                let id_span_message = (lint_id, span, message.to_owned());
-                let fresh = self.one_time_diagnostics.borrow_mut().insert(id_span_message);
-                if fresh {
-                    do_method()


The motivation for defining the do_method closure was to avoid repeating the match method code for the JSON and human-friendly cases: if we're going to start treating these the same, we can delete lines 362–371 and put the match here.

zackmdavis · 2017-11-20T20:19:17Z

src/test/ui/check_match/issue-43253.stderr


 warning: unreachable pattern
  --> $DIR/issue-43253.rs:49:9
   |
 49 |         9...9 => {},
   |         ^^^^^
+   |
+note: lint level defined here


This looks like a regression in one-time diagnostics (the motivation for which was avoiding too many "lint level defined here"s), but it's not immediately clear why that would happen ...

I think this is just a mixup that happened due to the update-all script not actually updating everything after it produced the expected output once. I need to touch all files in order to have them regenerate their stderr files.

Phlosioneer · 2017-11-20T22:27:14Z

I'm glad someone's taken over for me. Thanks.

bors · 2017-11-21T09:14:28Z

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

oli-obk · 2017-11-21T09:18:08Z

But I think having just a test or two that checks that would be good enough -- and really we weren't checking colors anyhow.

I added one. It's just a matter of adding // compile-flags: --error-format=human to the file. In that case the //~ ERROR comments won't work anymore

oli-obk · 2017-11-21T10:10:45Z

Travis should pass now. At least both ui and ui-fulldeps run through locally on my machine.

bors · 2017-11-21T20:06:43Z

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

nikomatsakis

r=me

I'm sure this is prone to bitrot, so p=1 as well.

nikomatsakis · 2017-11-22T09:05:24Z

src/libsyntax/json.rs

@@ -170,6 +171,27 @@ impl Diagnostic {
                rendered: None,
            }
        });
+
+        // generate regular command line output and store it in the json


As an aside, one thing I have long wanted is precisely this: to have the full formatted output in the JSON. The only thing missing here are the colors, but I guess we could add that in a side "attribute" table saying like "the text from this column to that column is this color".

cc @estebank

nikomatsakis · 2017-11-22T09:26:19Z

Travis result:

failures:
[00:49:26]     [compile-fail] compile-fail/issue-31221.rs
[00:49:26]     [compile-fail] compile-fail/lint-output-format-2.rs
[00:49:26]     [compile-fail] compile-fail/lint-unconditional-recursion.rs

oli-obk · 2017-11-22T14:58:29Z

tests pass locally now

kennytm · 2017-11-22T16:40:40Z

@bors r=nikomatsakis p=1

bors · 2017-11-22T16:40:41Z

📌 Commit 5a6da19 has been approved by nikomatsakis

bors · 2017-11-22T18:54:31Z

⌛ Testing commit 5a6da1969c0e5f74866ca6d0cec62aeca8948c95 with merge 8b0d1bb4012f71191363fdf3d024eb72454ab84c...

bors · 2017-11-22T19:38:14Z

💔 Test failed - status-travis

kennytm · 2017-11-22T19:49:05Z

The UI tests need to be updated to account for the recent merges. Please rebase on latest master.

[00:38:49] failures:
[00:38:49] 
[00:38:49] ---- [ui] ui/fmt/send-sync.rs stdout ----
[00:38:49] 	
[00:38:49] error: /checkout/src/test/ui/fmt/send-sync.rs:18: unexpected error: '18:5: 18:9: the trait bound `*mut std::ops::Fn() + 'static: std::marker::Sync` is not satisfied in `[std::fmt::ArgumentV1<'_>]` [E0277]'
[00:38:49] 
[00:38:49] error: /checkout/src/test/ui/fmt/send-sync.rs:19: unexpected error: '19:5: 19:9: the trait bound `*mut std::ops::Fn() + 'static: std::marker::Sync` is not satisfied in `std::fmt::Arguments<'_>` [E0277]'
[00:38:49] 
[00:38:49] error: 2 unexpected errors found, 0 expected errors not found
[00:38:49] status: exit code: 101
[00:38:49] command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/src/test/ui/fmt/send-sync.rs" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui" "--target=x86_64-unknown-linux-musl" "--error-format" "json" "-C" "prefer-dynamic" "-o" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/fmt/send-sync.stage2-x86_64-unknown-linux-musl" "-Crpath" "-O" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-musl/native/rust-test-helpers" "-Clinker=/musl-x86_64/bin/musl-gcc" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/fmt/send-sync.stage2-x86_64-unknown-linux-musl.aux" "-A" "unused"
[00:38:49] unexpected errors (from JSON output): [
[00:38:49]     Error {
[00:38:49]         line_num: 18,
[00:38:49]         kind: Some(
[00:38:49]             Error
[00:38:49]         ),
[00:38:49]         msg: "18:5: 18:9: the trait bound `*mut std::ops::Fn() + \'static: std::marker::Sync` is not satisfied in `[std::fmt::ArgumentV1<\'_>]` [E0277]"
[00:38:49]     },
[00:38:49]     Error {
[00:38:49]         line_num: 19,
[00:38:49]         kind: Some(
[00:38:49]             Error
[00:38:49]         ),
[00:38:49]         msg: "19:5: 19:9: the trait bound `*mut std::ops::Fn() + \'static: std::marker::Sync` is not satisfied in `std::fmt::Arguments<\'_>` [E0277]"
[00:38:49]     }
[00:38:49] ]
[00:38:49] 
[00:38:49] thread '[ui] ui/fmt/send-sync.rs' panicked at 'explicit panic', /checkout/src/tools/compiletest/src/runtest.rs:1086:12
[00:38:49] note: Run with `RUST_BACKTRACE=1` for a backtrace.
[00:38:49] 
[00:38:49] ---- [ui] ui/impl-trait/no-method-suggested-traits.rs stdout ----
[00:38:49] 	
[00:38:49] error: /checkout/src/test/ui/impl-trait/no-method-suggested-traits.rs:14: unexpected help message: '14:1: 14:1: the following traits are implemented but not in scope, perhaps add a `use` for one of them:'
[00:38:49] 
[00:38:49] error: /checkout/src/test/ui/impl-trait/no-method-suggested-traits.rs:14: unexpected help message: '14:1: 14:1: the following traits are implemented but not in scope, perhaps add a `use` for one of them:'
[00:38:49] 
[00:38:49] error: /checkout/src/test/ui/impl-trait/no-method-suggested-traits.rs:14: unexpected help message: '14:1: 14:1: the following trait is implemented but not in scope, perhaps add a `use` for it:'
[00:38:49] 
[00:38:49] error: /checkout/src/test/ui/impl-trait/no-method-suggested-traits.rs:14: unexpected help message: '14:1: 14:1: the following trait is implemented but not in scope, perhaps add a `use` for it:'
[00:38:49] 
[00:38:49] error: /checkout/src/test/ui/impl-trait/no-method-suggested-traits.rs:14: unexpected help message: '14:1: 14:1: the following trait is implemented but not in scope, perhaps add a `use` for it:'
[00:38:49] 
[00:38:49] error: /checkout/src/test/ui/impl-trait/no-method-suggested-traits.rs:14: unexpected help message: '14:1: 14:1: the following trait is implemented but not in scope, perhaps add a `use` for it:'
[00:38:49] 
[00:38:49] error: 6 unexpected errors found, 0 expected errors not found
[00:38:49] status: exit code: 101
[00:38:49] command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/src/test/ui/impl-trait/no-method-suggested-traits.rs" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui" "--target=x86_64-unknown-linux-musl" "--error-format" "json" "-C" "prefer-dynamic" "-o" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/impl-trait/no-method-suggested-traits.stage2-x86_64-unknown-linux-musl" "-Crpath" "-O" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-musl/native/rust-test-helpers" "-Clinker=/musl-x86_64/bin/musl-gcc" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/impl-trait/no-method-suggested-traits.stage2-x86_64-unknown-linux-musl.aux" "-A" "unused"
[00:38:49] unexpected errors (from JSON output): [
[00:38:49]     Error {
[00:38:49]         line_num: 14,
[00:38:49]         kind: Some(
[00:38:49]             Help
[00:38:49]         ),
[00:38:49]         msg: "14:1: 14:1: the following traits are implemented but not in scope, perhaps add a `use` for one of them:"
[00:38:49]     },
[00:38:49]     Error {
[00:38:49]         line_num: 14,
[00:38:49]         kind: Some(
[00:38:49]             Help
[00:38:49]         ),
[00:38:49]         msg: "14:1: 14:1: the following traits are implemented but not in scope, perhaps add a `use` for one of them:"
[00:38:49]     },
[00:38:49]     Error {
[00:38:49]         line_num: 14,
[00:38:49]         kind: Some(
[00:38:49]             Help
[00:38:49]         ),
[00:38:49]         msg: "14:1: 14:1: the following trait is implemented but not in scope, perhaps add a `use` for it:"
[00:38:49]     },
[00:38:49]     Error {
[00:38:49]         line_num: 14,
[00:38:49]         kind: Some(
[00:38:49]             Help
[00:38:49]         ),
[00:38:49]         msg: "14:1: 14:1: the following trait is implemented but not in scope, perhaps add a `use` for it:"
[00:38:49]     },
[00:38:49]     Error {
[00:38:49]         line_num: 14,
[00:38:49]         kind: Some(
[00:38:49]             Help
[00:38:49]         ),
[00:38:49]         msg: "14:1: 14:1: the following trait is implemented but not in scope, perhaps add a `use` for it:"
[00:38:49]     },
[00:38:49]     Error {
[00:38:49]         line_num: 14,
[00:38:49]         kind: Some(
[00:38:49]             Help
[00:38:49]         ),
[00:38:49]         msg: "14:1: 14:1: the following trait is implemented but not in scope, perhaps add a `use` for it:"
[00:38:49]     }
[00:38:49] ]
[00:38:49] 
[00:38:49] thread '[ui] ui/impl-trait/no-method-suggested-traits.rs' panicked at 'explicit panic', /checkout/src/tools/compiletest/src/runtest.rs:1086:12
[00:38:49] 
[00:38:49] ---- [ui] ui/issue-35976.rs stdout ----
[00:38:49] 	
[00:38:49] error: /checkout/src/test/ui/issue-35976.rs:24: expected message not found: another candidate was found in the following trait, perhaps add a `use` for it:
[00:38:49] 
[00:38:49] error: 0 unexpected errors found, 1 expected errors not found
[00:38:49] status: exit code: 101
[00:38:49] command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/src/test/ui/issue-35976.rs" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui" "--target=x86_64-unknown-linux-musl" "--error-format" "json" "-C" "prefer-dynamic" "-o" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/issue-35976.stage2-x86_64-unknown-linux-musl" "-Crpath" "-O" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-musl/native/rust-test-helpers" "-Clinker=/musl-x86_64/bin/musl-gcc" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/issue-35976.stage2-x86_64-unknown-linux-musl.aux" "-A" "unused"
[00:38:49] not found errors (from test file): [
[00:38:49]     Error {
[00:38:49]         line_num: 24,
[00:38:49]         kind: None,
[00:38:49]         msg: "another candidate was found in the following trait, perhaps add a `use` for it:"
[00:38:49]     }
[00:38:49] ]
[00:38:49] 
[00:38:49] thread '[ui] ui/issue-35976.rs' panicked at 'explicit panic', /checkout/src/tools/compiletest/src/runtest.rs:1086:12
[00:38:49] 
[00:38:49] 
[00:38:49] failures:
[00:38:49]     [ui] ui/fmt/send-sync.rs
[00:38:49]     [ui] ui/impl-trait/no-method-suggested-traits.rs
[00:38:49]     [ui] ui/issue-35976.rs
[00:38:49] 
[00:38:49] test result: FAILED. 462 passed; 3 failed; 1 ignored; 0 measured; 0 filtered out

oli-obk · 2017-11-23T07:59:30Z

updated

bors · 2017-11-23T08:21:24Z

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

oli-obk · 2017-11-23T09:13:37Z

rebased again

oli-obk · 2017-11-23T09:17:21Z

The PR in the queue (#46051) will break this again.

The next one after that (#46054) should be fine though.

oli-obk · 2017-11-23T11:16:03Z

I have not been able to fix it in time (gotta run, and rebase + test is taking too long). Will try again tomorrow.

Can I have bors delegation rights so I can immediately put it in the queue when I fix it?

petrochenkov · 2017-11-23T11:29:18Z

@bors delegate+

bors · 2017-11-23T11:29:19Z

✌️ @oli-obk can now approve this pull request

oli-obk · 2017-11-24T08:01:54Z

@bors r=nikomatsakis p=1

bors · 2017-11-24T08:01:54Z

📌 Commit 8c1f9ef has been approved by nikomatsakis

bors · 2017-11-24T09:05:16Z

⌛ Testing commit 8c1f9ef with merge cc8ceb0...

@nikomatsakis

Check //~ERROR comments in ui tests r? @nikomatsakis cc #44844 @Phlosioneer @estebank @petrochenkov this depends on #46052 getting merged first (the commits are included in here) The relevant changes of this PR are c2f0af7 and 979269b

bors · 2017-11-24T10:06:06Z

💔 Test failed - status-appveyor

oli-obk · 2017-11-24T10:09:45Z

legit:

-$DIR/short-error-format.rs:16:9 - error[E0308]: mismatched types
-$DIR/short-error-format.rs:18:7 - error[E0599]: no method named `salut` found for type `u32` in the current scope
+C:/projects/rust/src/test/ui/short-error-format.rs:16:9 - error[E0308]: mismatched types
+C:/projects/rust/src/test/ui/short-error-format.rs:18:7 - error[E0599]: no method named `salut` found for type `u32` in the current scope

oli-obk · 2017-11-24T10:35:31Z

I wasn't able to test on windows, but I undid the change that lead to the problem (replacing \ with \\ whenever --error-format was passed to a ui test, when it should only be done if the format is json)

@bors r=nikomatsakis p=1

bors · 2017-11-24T10:35:32Z

📌 Commit 8937d6a has been approved by nikomatsakis

bors · 2017-11-24T12:38:17Z

⌛ Testing commit 8937d6a with merge 71da1c2...

@nikomatsakis

Check //~ERROR comments in ui tests r? @nikomatsakis cc #44844 @Phlosioneer @estebank @petrochenkov this depends on #46052 getting merged first (the commits are included in here) The relevant changes of this PR are c2f0af7 and 979269b

bors · 2017-11-24T15:11:06Z

☀️ Test successful - status-appveyor, status-travis
Approved by: nikomatsakis
Pushing 71da1c2 to master...

rust-highfive assigned nikomatsakis Nov 20, 2017

kennytm added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 20, 2017

zackmdavis reviewed Nov 20, 2017

View reviewed changes

oli-obk force-pushed the json_ui branch from 979269b to 6dca0ef Compare November 21, 2017 09:17

oli-obk force-pushed the json_ui branch 2 times, most recently from a30ef3f to f40b805 Compare November 21, 2017 10:01

nikomatsakis approved these changes Nov 22, 2017

View reviewed changes

oli-obk force-pushed the json_ui branch 2 times, most recently from f4cc0f6 to 5a6da19 Compare November 22, 2017 14:57

kennytm added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 22, 2017

oli-obk force-pushed the json_ui branch from 5a6da19 to bee17d9 Compare November 23, 2017 07:59

oli-obk force-pushed the json_ui branch from bee17d9 to 021f834 Compare November 23, 2017 09:13

oli-obk force-pushed the json_ui branch from 021f834 to 6b7e5be Compare November 23, 2017 11:14

oli-obk added 3 commits November 24, 2017 08:06

Simplify an Iterator::fold to Iterator::any

7e07fd3

This method of once-diagnostics doesn't allow nesting

f7361a8

UI tests extract the regular output from the 'rendered' field in json

36066d8

oli-obk force-pushed the json_ui branch from 6b7e5be to 8c1f9ef Compare November 24, 2017 08:01

Merge cfail and ui tests into ui tests

8937d6a

oli-obk force-pushed the json_ui branch from 8c1f9ef to 8937d6a Compare November 24, 2017 10:32

bors merged commit 8937d6a into rust-lang:master Nov 24, 2017

This was referenced Nov 24, 2017

MIR: Fix value moved diagnose messages #46231

Merged

Add nested groups in imports #45846

Merged

Be more obvious when suggesting dereference #45947

Merged

Display \t in diagnostics code as four spaces #45953

Merged

oli-obk deleted the json_ui branch November 24, 2017 18:11

petrochenkov mentioned this pull request Feb 27, 2018

Remove unneeded error check in ui tests #48562

Closed

Check //~ERROR comments in ui tests #46116

Check //~ERROR comments in ui tests #46116

Conversation

oli-obk commented Nov 20, 2017 • edited Loading

nikomatsakis commented Nov 20, 2017

zackmdavis Nov 20, 2017

Choose a reason for hiding this comment

oli-obk Nov 21, 2017

Choose a reason for hiding this comment

zackmdavis Nov 20, 2017

Choose a reason for hiding this comment

oli-obk Nov 21, 2017

Choose a reason for hiding this comment

Phlosioneer commented Nov 20, 2017

bors commented Nov 21, 2017

oli-obk commented Nov 21, 2017

oli-obk commented Nov 21, 2017

bors commented Nov 21, 2017

nikomatsakis left a comment

Choose a reason for hiding this comment

nikomatsakis Nov 22, 2017

Choose a reason for hiding this comment

nikomatsakis commented Nov 22, 2017

oli-obk commented Nov 22, 2017

kennytm commented Nov 22, 2017

bors commented Nov 22, 2017

bors commented Nov 22, 2017

bors commented Nov 22, 2017

kennytm commented Nov 22, 2017

oli-obk commented Nov 23, 2017

bors commented Nov 23, 2017

oli-obk commented Nov 23, 2017

oli-obk commented Nov 23, 2017

oli-obk commented Nov 23, 2017

petrochenkov commented Nov 23, 2017

bors commented Nov 23, 2017

oli-obk commented Nov 24, 2017

bors commented Nov 24, 2017

bors commented Nov 24, 2017

bors commented Nov 24, 2017

oli-obk commented Nov 24, 2017

oli-obk commented Nov 24, 2017

bors commented Nov 24, 2017

bors commented Nov 24, 2017

bors commented Nov 24, 2017

oli-obk commented Nov 20, 2017 •

edited

Loading