Check //~ERROR comments in ui tests#46116
Conversation
|
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. |
src/librustc/session/mod.rs
Outdated
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 ...
There was a problem hiding this comment.
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.
|
I'm glad someone's taken over for me. Thanks. |
|
☔ The latest upstream changes (presumably #45545) made this pull request unmergeable. Please resolve the merge conflicts. |
I added one. It's just a matter of adding |
a30ef3f to
f40b805
Compare
|
Travis should pass now. At least both |
|
☔ The latest upstream changes (presumably #46166) made this pull request unmergeable. Please resolve the merge conflicts. |
nikomatsakis
left a comment
There was a problem hiding this comment.
r=me
I'm sure this is prone to bitrot, so p=1 as well.
src/libsyntax/json.rs
Outdated
There was a problem hiding this comment.
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
|
Travis result: |
f4cc0f6 to
5a6da19
Compare
|
tests pass locally now |
|
@bors r=nikomatsakis p=1 |
|
📌 Commit 5a6da19 has been approved by |
|
⌛ Testing commit 5a6da1969c0e5f74866ca6d0cec62aeca8948c95 with merge 8b0d1bb4012f71191363fdf3d024eb72454ab84c... |
|
💔 Test failed - status-travis |
|
The UI tests need to be updated to account for the recent merges. Please rebase on latest master. Details |
|
updated |
|
☔ The latest upstream changes (presumably #46024) made this pull request unmergeable. Please resolve the merge conflicts. |
|
rebased again |
|
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? |
|
@bors delegate+ |
|
✌️ @oli-obk can now approve this pull request |
|
@bors r=nikomatsakis p=1 |
|
📌 Commit 8c1f9ef has been approved by |
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
|
💔 Test failed - status-appveyor |
|
legit: |
|
I wasn't able to test on windows, but I undid the change that lead to the problem (replacing @bors r=nikomatsakis p=1 |
|
📌 Commit 8937d6a has been approved by |
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
|
☀️ Test successful - status-appveyor, status-travis |
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