Repository navigation
Most of UI tests are now compiled twice, once with --error-format json and once with --error-format human #48550
Description
The change was made in #48337
cc @GuillaumeGomez
This is not helpful for our testing times, especially given that number of UI tests will grow due to other test categories being converted to UI.
Activity
GuillaumeGomez commentedon Feb 26, 2018
Yep, sorry about that. However, if I didn't do this, the output was broken (because we'd have multiple useless "rustc --explain" sentences). If anyone has a better idea, it'd be very appreciated!
GuillaumeGomez commentedon Feb 26, 2018
Actually, for the moment we only rely on the json error output. Add a new check over the "normal" error output would allow us to greatly reduce the ui tests duration.
zackmdavis commentedon Feb 28, 2018
@GuillaumeGomez
Can you be more specific about what the problem was exactly? Intuitively, I wouldn't expect printing one or two extra lines after the errors to require any changes to the UI test framework itself.
zackmdavis commentedon Feb 28, 2018
I have a branch which reverts 1dc2015. And indeed, when outputting for humans, we see the message about
--explainjust once, but in JSON mode, it appears once per error. It looks like this is because the message is being printed inside ofEmitterWriter's.drop()method (!), and the JSON emitter itself instantiates anEmitterWriterwhen creating aDiagnosticfrom aDiagnosticBuilder.zackmdavis commentedon Feb 28, 2018
I need to sleep now, but I'd like to put together a PR tomorrow
zackmdavis commentedon Feb 28, 2018
I'm pretty confident that we want to revert #48337: printing the
--explainusage message inside ofEmitterWriter.dropwas not an adequate solution (because it's counterintuitive to be doing non-cleanup work in.drop, the unintended additional messages for every diagnostic in the JSON output were undesirable, and changing the test framework in response to that unintended undesirable thing is also undesirable).I was hoping that, in order to avoid another enormous UI test output diff (which could potentially slow down the rest of the queue), it would be easy to reimplement the
--explainusage message near where we issue the "aborting after n errors" message, without changing the output: then we would only have one more enormous UI test output diff when we edit the wording (#48559). But outputting the--explainusage message along with the "aborting after n errors" message in theHandleris looking somewhat less trivial that it sounds, becauseHandlerdoesn't seem to have direct access to an output stream where we can write some arbitrarily lines; it just has anEmittertrait object whose sole method (emit) can only take aDiagnosticBuilder. No doubt we could formulate the--explainmessage that we want in the form of a diagnostic (or come up with some alternative solution), but I wouldn't want to block on that.revert "Update tools code"
revert "Add rustc --explain back"
update UI tests after reverting `--explain` note and double-testing
petrochenkov commentedon Mar 16, 2018
Fixed in #48684