-
Notifications
You must be signed in to change notification settings - Fork 12.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
compiletest failures are close to unreadable because of JSON #36516
Comments
@nikomatsakis suggests that the compile-fail output is better and json is only used for that case, so compiletest could be modified to only use json mode there. |
I've started a patch. |
Simply disabling json errors for rpass isn't sufficient because there are rpass errors that use the new precise error checking to check warnings. |
@brson example? Perhaps those tests should be made into ui tests? I forget if those work for run-pass tests or not. In general, as discussed on this thread, I'd like to move over to ui testing more and more. One of the things I had hoped to do was to generalize the concept of "run-pass" vs "compile-fail" test so that we just had "tests", where you could specify in the test how it was expected to behave (compile successfully, compile with errors, etc) and then use a reference file to check the compiler's output. I don't claim to have this vision 100% worked out yet though. |
I don't think I'll get to fixing this after all because of the aforementioned issue. |
@nikomatsakis here are the rpass failures that check warnings:
|
@brson do you have an update on this, just hit this again with run-pass-fulldeps tests unfortunately :( @nikomatsakis I think the problem here is that the tests aren't UI tests, just standard run-pass tests. Everything is json errors now which makes it harder to diagnose failures. |
@alexcrichton my point is that I think we could switch run-pass to not use JSON. If the sticking point is those 5 tests, we could either fix them somehow, or convert those 5 into ui tests instead. |
@alexcrichton No, I'm not working on this. |
@nikomatsakis agreed! (that we can switch) |
…ust-lang#36516. The 'run-pass' header cause a 'ui' test to execute the result. It is used to test the lint output, at the same time ensure those lints won't cause the source code to become compile-fail. 12 run-pass/run-pass-fulldeps tests gained the header and are moved to ui/ui-fulldeps. After this move, no run-pass/run-pass-fulldeps tests should rely on the compiler's JSON message. This allows us to stop passing `--error-format json` in run-pass tests, thus fixing rust-lang#36516.
…tput-36516, r=nikomatsakis Introduce 'run-pass' header to 'ui' tests in compiletest. Fix issue rust-lang#36516. <del>`ui-run` test is a combination of `ui` test and `run-pass` test. It is used to test lint output.</del> Added support of `// run-pass` header to `ui` tests. The compiler message of each test must match the corresponding `*.stderr` file like the traditional `ui` tests. Additionally, the compiled output must be executed successfully like the `run-pass` test. 12 `run-pass`/`run-pass-fulldeps` tests are moved to `ui`/`ui-fulldeps` plus the headers. After this move, no `run-pass`/`run-pass-fulldeps` tests should rely on the compiler's JSON message. This allows us to stop passing `--error-format json` in run-pass tests, thus fixing rust-lang#36516.
Is this still an issue? I seem to get much nicer failures now. |
…ust-lang#36516. The 'run-pass' header cause a 'ui' test to execute the result. It is used to test the lint output, at the same time ensure those lints won't cause the source code to become compile-fail. 12 run-pass/run-pass-fulldeps tests gained the header and are moved to ui/ui-fulldeps. After this move, no run-pass/run-pass-fulldeps tests should rely on the compiler's JSON message. This allows us to stop passing `--error-format json` in run-pass tests, thus fixing rust-lang#36516.
… r=nikomatsakis Introduce 'run-pass' header to 'ui' tests in compiletest. Fix issue #36516. <del>`ui-run` test is a combination of `ui` test and `run-pass` test. It is used to test lint output.</del> Added support of `// run-pass` header to `ui` tests. The compiler message of each test must match the corresponding `*.stderr` file like the traditional `ui` tests. Additionally, the compiled output must be executed successfully like the `run-pass` test. 12 `run-pass`/`run-pass-fulldeps` tests are moved to `ui`/`ui-fulldeps` plus the headers. After this move, no `run-pass`/`run-pass-fulldeps` tests should rely on the compiler's JSON message. This allows us to stop passing `--error-format json` in run-pass tests, thus fixing #36516.
We've changed |
Here's one I hit recently:
compiletest uses the --error-format=json switch to capture more precise error information then dumps that out for humans to read.
Instead we might either: have the compiler output the json to a side file; reformat the JSON into something that makes more sense.
The text was updated successfully, but these errors were encountered: