-
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
Libtest json output #46450
Libtest json output #46450
Conversation
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @TimNN (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
cc @rust-lang/dev-tools -- we talked a bit about this two weeks ago. One of the things we kinda settled on was that we want the test system to be extensible, so people can experiment with alternative test runners, etc. This does look like a nice addition to the default testing tool, though. I think @matklad might be one of the first production user (of anything that allows easily parsing the test results in IntelliJ), so I'm especially interested in whether this is something he can use. |
+:100:, having a machine readable output out of the box is hugely important for tool interoperability, and implementing it is much easier then generic "custom test runners" thing. The main comment I have is that we absolutely need the docs which describe, in detail, the machine readable format. We probably also want to put this format through the RFC process? Regarding the implementation, I think we should replace newlines in strings with
It's also not clear what happens with stdout and stderr of the tests, but I've kinda convinced myself that the problem of associating output with a particular test is in general unsolvable, so I am fine if the output is just interleaved with JSON messages. |
Yeah, probably. I'm kinda hoping to have this be a limited/focussed RFC that skips a whole bunch of edge cases (i.e., literally only do what this PR does), or land this as an unstable feature. Otherwise, we're probably opening the door to discuss any and all features we could ever want in this format…
Oh yes, I totally forgot to comment on this. JSON does not allow lines breaks in strings, they have to be escaped. |
They're all great ideas, I'll try to escape \n today and I'll try to work on the RFC by the end of the weekend |
I think it'd be a good idea to use an actual JSON library to write these. The compiler has its own rustc_serialize, but this format looks like a very good fit for serde's tagged enum types. Not how easy/desirable it is to depend on serde{_json, _derive,} here, but it's already used in the bootstrap scripts and cargo. |
TBH, I tried adding serde as a dep of libtest and had some obscure errors. (didn't investigate) It seemed simple enough to just roll-my-own, but I agree it's probably better to use it now that I've realized escaping is not that simple. |
RFC here: |
This PR appears to be blocked on rust-lang/rfcs#2234 |
I see now that there's a desire to add multiple new formats (TAP, etc.). I can split this PR to merge only the refactoring needed for multiple output-formats, and have JSON as a separate PR. |
☔ The latest upstream changes (presumably #46620) made this pull request unmergeable. Please resolve the merge conflicts. |
dcd49b9
to
266f26d
Compare
We discussed this at the dev-tools meeting yesterday. We concluded that we don't need to wait for an RFC and we should land this now as a way to experiment and iterate on machine-readable test output. Everything should be behind an unstable feature flag. The long-term solution is going to be deeply intertwined with pluggable test runners. |
src/libtest/formatters.rs
Outdated
|
||
Ok(()) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing trailing newline
I've made a few fixes suggested in the RFC. The only issue left is how to mark it as unstable/nightly only. @alexcrichton , maybe you have an idea? Edit: |
Unfortunately we're not really set up right now to have unstable flags in libtest, so the infrastructure would have to be added here as well. In general though the way our unstable flags look like is:
|
Cool, and how should libtest obtain the stability/instability status? |
This is how it's done in libsyntax, and unfortunately that probably would just need to be duplicated for libtest. |
93e347a
to
debe263
Compare
r? @TimNN (am I doing this right?) |
src/libtest/lib.rs
Outdated
@@ -519,7 +530,13 @@ pub fn parse_opts(args: &[String]) -> Option<OptRes> { | |||
None if quiet => OutputFormat::Terse, | |||
Some("pretty") | None => OutputFormat::Pretty, | |||
Some("terse") => OutputFormat::Terse, | |||
Some("json") => OutputFormat::Json, | |||
Some("json") => { | |||
if !allow_unstable { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not quite what @alexcrichton mentioned in #46450 (comment): There should be a -Z unstable-options
flag, which may only be passed on nightly
(the allow_unstable()
function above), and only if -Z unstable-options
is passed should json output be allowed (in other words: the user needs to explicitly enable unstable flags, before being allowed to pass the json flag).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ohh, got it.
json is behind -Z now |
ab10869
to
100ead3
Compare
@kennytm All problems should be solved™ |
@bors r=nrc |
📌 Commit 100ead3 has been approved by |
Libtest json output A revisit to my [last PR](#45923). Events are now more atomic, printed in a flat hierarchy. For the normal test output: ``` running 1 test test f ... FAILED failures: ---- f stdout ---- thread 'f' panicked at 'assertion failed: `(left == right)` left: `3`, right: `4`', f.rs:3:1 note: Run with `RUST_BACKTRACE=1` for a backtrace. failures: f test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out ``` The JSON equivalent is: ``` { "type": "suite", "event": "started", "test_count": "1" } { "type": "test", "event": "started", "name": "f" } { "type": "test", "event": "failed", "name": "f" } { "type": "suite", "event": "failed", "passed": 0, "failed": 1, "allowed_fail": 0, "ignored": 0, "measured": 0, "filtered_out": "0" } { "type": "test_output", "name": "f", "output": "thread 'f' panicked at 'assertion failed: `(left == right)` left: `3`, right: `4`', f.rs:3:1 note: Run with `RUST_BACKTRACE=1` for a backtrace. " } ```
💔 Test failed - status-appveyor |
$(RUSTC) --test f.rs | ||
$(call RUN,f) -Z unstable-options --test-threads=1 --format=json > $(OUTPUT_FILE) || true | ||
|
||
cat $(OUTPUT_FILE) | $(PYTHON) validate_json.py |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please quote the environment variable like "$(PYTHON)"
. On Windows the path is C:\Python27\python.exe
, and shell strips the backslashes without the quotes.
[02:04:23] ---- [run-make] run-make\libtest-json stdout ----
[02:04:23]
[02:04:23] error: make failed
[02:04:23] status: exit code: 2
[02:04:23] command: "make"
[02:04:23] stdout:
[02:04:23] ------------------------------------------
[02:04:23] PATH="/c/projects/rust/build/x86_64-pc-windows-gnu/test/run-make/libtest-json.stage2-x86_64-pc-windows-gnu:C:\projects\rust\build\x86_64-pc-windows-gnu\stage2\bin:/c/projects/rust/build/x86_64-pc-windows-gnu/stage0-tools/x86_64-pc-windows-gnu/release/deps:/c/projects/rust/build/x86_64-pc-windows-gnu/stage0-sysroot/lib/rustlib/x86_64-pc-windows-gnu/lib:/c/Program Files (x86)/Inno Setup 5:/c/Python27:/c/projects/rust/mingw64/bin:/usr/bin:/c/Perl/site/bin:/c/Perl/bin:/c/Windows/system32:/c/Windows:/c/Windows/System32/Wbem:/c/Windows/System32/WindowsPowerShell/v1.0:/c/Program Files/7-Zip:/c/Program Files/Microsoft/Web Platform Installer:/c/Tools/GitVersion:/c/Tools/PsTools:/c/Program Files/Git LFS:/c/Program Files (x86)/Subversion/bin:/c/Program Files/Microsoft SQL Server/120/Tools/Binn:/c/Program Files/Microsoft SQL Server/Client SDK/ODBC/110/Tools/Binn:/c/Program Files (x86)/Microsoft SQL Server/120/Tools/Binn:/c/Program Files/Microsoft SQL Server/120/DTS/Binn:/c/Program Files (x86)/Microsoft SQL Server/120/Tools/Binn/ManagementStudio:/c/Tools/WebDriver:/c/Program Files (x86)/Microsoft SDKs/TypeScript/1.4:/c/Program Files (x86)/Microsoft Visual Studio 12.0/Common7/IDE/PrivateAssemblies:/c/Program Files (x86)/Microsoft SDKs/Azure/CLI/wbin:/c/Ruby193/bin:/c/Tools/NUnit/bin:/c/Tools/xUnit:/c/Tools/MSpec:/c/Tools/Coverity/bin:/c/Program Files (x86)/CMake/bin:/c/go/bin:/c/Program Files/Java/jdk1.8.0/bin:/c/Python27:/c/Program Files/nodejs:/c/Program Files (x86)/iojs:/c/Program Files/iojs:/c/Users/appveyor/AppData/Roaming/npm:/c/Program Files/Microsoft SQL Server/130/Tools/Binn:/c/Program Files (x86)/MSBuild/14.0/Bin:/c/Tools/NuGet:/c/Program Files (x86)/Microsoft Visual Studio 14.0/Common7/IDE/CommonExtensions/Microsoft/TestWindow:/c/Program Files/Microsoft DNX/Dnvm:/c/Program Files/Microsoft SQL Server/Client SDK/ODBC/130/Tools/Binn:/c/Program Files (x86)/Microsoft SQL Server/130/Tools/Binn:/c/Program Files (x86)/Microsoft SQL Server/130/DTS/Binn:/c/Program Files/Microsoft SQL Server/130/DTS/Binn:/c/Program Files (x86)/Microsoft SQL Server/110/DTS/Binn:/c/Program Files (x86)/Microsoft SQL Server/120/DTS/Binn:/c/Program Files (x86)/Apache/Maven/bin:/c/Python27/Scripts:/c/Tools/NUnit3:/c/Program Files/Mercurial:/c/Program Files/LLVM/bin:/c/Program Files/dotnet:/c/Tools/curl/bin:/c/Program Files/Amazon/AWSCLI:/c/Program Files (x86)/Microsoft SQL Server/140/DTS/Binn:/c/Program Files (x86)/Microsoft Visual Studio 14.0/Common7/IDE/Extensions/Microsoft/SQLDB/DAC/140:/c/Program Files/Git/cmd:/c/Program Files/Git/usr/bin:/c/Tools/vcpkg:/c/Program Files/Microsoft Service Fabric/bin/Fabric/Fabric.Code:/c/Program Files/Microsoft SDKs/Service Fabric/Tools/ServiceFabricLocalClusterManager:/c/Program Files (x86)/Yarn/bin:/c/Program Files (x86)/Microsoft SQL Server/140/Tools/Binn:/c/Program Files/Microsoft SQL Server/140/Tools/Binn:/c/Program Files/Microsoft SQL Server/140/DTS/Binn:/c/Program Files (x86)/nodejs:/c/ProgramData/chocolatey/bin:/c/Program Files/PowerShell/6.0.0:/c/Program Files/erl9.2/bin:/c/Users/appveyor/AppData/Local/Yarn/bin:/c/Users/appveyor/AppData/Roaming/npm:/c/Program Files/AppVeyor/BuildAgent:/c/projects/rust:/c/projects/rust/handle" 'C:\projects\rust\build\x86_64-pc-windows-gnu\stage2\bin\rustc.exe' --out-dir /c/projects/rust/build/x86_64-pc-windows-gnu/test/run-make/libtest-json.stage2-x86_64-pc-windows-gnu -L /c/projects/rust/build/x86_64-pc-windows-gnu/test/run-make/libtest-json.stage2-x86_64-pc-windows-gnu --test f.rs
[02:04:23] PATH="/c/projects/rust/build/x86_64-pc-windows-gnu/stage0-tools/x86_64-pc-windows-gnu/release/deps:/c/projects/rust/build/x86_64-pc-windows-gnu/stage0-sysroot/lib/rustlib/x86_64-pc-windows-gnu/lib:/c/Program Files (x86)/Inno Setup 5:/c/Python27:/c/projects/rust/mingw64/bin:/usr/bin:/c/Perl/site/bin:/c/Perl/bin:/c/Windows/system32:/c/Windows:/c/Windows/System32/Wbem:/c/Windows/System32/WindowsPowerShell/v1.0:/c/Program Files/7-Zip:/c/Program Files/Microsoft/Web Platform Installer:/c/Tools/GitVersion:/c/Tools/PsTools:/c/Program Files/Git LFS:/c/Program Files (x86)/Subversion/bin:/c/Program Files/Microsoft SQL Server/120/Tools/Binn:/c/Program Files/Microsoft SQL Server/Client SDK/ODBC/110/Tools/Binn:/c/Program Files (x86)/Microsoft SQL Server/120/Tools/Binn:/c/Program Files/Microsoft SQL Server/120/DTS/Binn:/c/Program Files (x86)/Microsoft SQL Server/120/Tools/Binn/ManagementStudio:/c/Tools/WebDriver:/c/Program Files (x86)/Microsoft SDKs/TypeScript/1.4:/c/Program Files (x86)/Microsoft Visual Studio 12.0/Common7/IDE/PrivateAssemblies:/c/Program Files (x86)/Microsoft SDKs/Azure/CLI/wbin:/c/Ruby193/bin:/c/Tools/NUnit/bin:/c/Tools/xUnit:/c/Tools/MSpec:/c/Tools/Coverity/bin:/c/Program Files (x86)/CMake/bin:/c/go/bin:/c/Program Files/Java/jdk1.8.0/bin:/c/Python27:/c/Program Files/nodejs:/c/Program Files (x86)/iojs:/c/Program Files/iojs:/c/Users/appveyor/AppData/Roaming/npm:/c/Program Files/Microsoft SQL Server/130/Tools/Binn:/c/Program Files (x86)/MSBuild/14.0/Bin:/c/Tools/NuGet:/c/Program Files (x86)/Microsoft Visual Studio 14.0/Common7/IDE/CommonExtensions/Microsoft/TestWindow:/c/Program Files/Microsoft DNX/Dnvm:/c/Program Files/Microsoft SQL Server/Client SDK/ODBC/130/Tools/Binn:/c/Program Files (x86)/Microsoft SQL Server/130/Tools/Binn:/c/Program Files (x86)/Microsoft SQL Server/130/DTS/Binn:/c/Program Files/Microsoft SQL Server/130/DTS/Binn:/c/Program Files (x86)/Microsoft SQL Server/110/DTS/Binn:/c/Program Files (x86)/Microsoft SQL Server/120/DTS/Binn:/c/Program Files (x86)/Apache/Maven/bin:/c/Python27/Scripts:/c/Tools/NUnit3:/c/Program Files/Mercurial:/c/Program Files/LLVM/bin:/c/Program Files/dotnet:/c/Tools/curl/bin:/c/Program Files/Amazon/AWSCLI:/c/Program Files (x86)/Microsoft SQL Server/140/DTS/Binn:/c/Program Files (x86)/Microsoft Visual Studio 14.0/Common7/IDE/Extensions/Microsoft/SQLDB/DAC/140:/c/Program Files/Git/cmd:/c/Program Files/Git/usr/bin:/c/Tools/vcpkg:/c/Program Files/Microsoft Service Fabric/bin/Fabric/Fabric.Code:/c/Program Files/Microsoft SDKs/Service Fabric/Tools/ServiceFabricLocalClusterManager:/c/Program Files (x86)/Yarn/bin:/c/Program Files (x86)/Microsoft SQL Server/140/Tools/Binn:/c/Program Files/Microsoft SQL Server/140/Tools/Binn:/c/Program Files/Microsoft SQL Server/140/DTS/Binn:/c/Program Files (x86)/nodejs:/c/ProgramData/chocolatey/bin:/c/Program Files/PowerShell/6.0.0:/c/Program Files/erl9.2/bin:/c/Users/appveyor/AppData/Local/Yarn/bin:/c/Users/appveyor/AppData/Roaming/npm:/c/Program Files/AppVeyor/BuildAgent:/c/projects/rust:/c/projects/rust/handle:C:\projects\rust\build\x86_64-pc-windows-gnu\stage2\lib\rustlib\x86_64-pc-windows-gnu\lib" /c/projects/rust/build/x86_64-pc-windows-gnu/test/run-make/libtest-json.stage2-x86_64-pc-windows-gnu/f -Z unstable-options --test-threads=1 --format=json > /c/projects/rust/build/x86_64-pc-windows-gnu/test/run-make/libtest-json.stage2-x86_64-pc-windows-gnu/libtest-json-output.json || true
[02:04:23] cat /c/projects/rust/build/x86_64-pc-windows-gnu/test/run-make/libtest-json.stage2-x86_64-pc-windows-gnu/libtest-json-output.json | C:\Python27\python.exe validate_json.py
[02:04:23]
[02:04:23] ------------------------------------------
[02:04:23] stderr:
[02:04:23] ------------------------------------------
[02:04:23] /bin/sh: C:Python27python.exe: command not found
[02:04:23] make: *** [Makefile:10: all] Error 127
[02:04:23]
[02:04:23] ------------------------------------------
[02:04:23]
[02:04:23] thread '[run-make] run-make\libtest-json' panicked at 'explicit panic', tools\compiletest\src\runtest.rs:2883:9
[02:04:23] note: Run with `RUST_BACKTRACE=1` for a backtrace.
[02:04:23]
[02:04:23]
[02:04:23] failures:
[02:04:23] [run-make] run-make\libtest-json
[02:04:23]
[02:04:23] test result: FAILED. 172 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out
[02:04:23]
[02:04:23] thread 'main' panicked at 'Some tests failed', tools\compiletest\src\main.rs:476:22
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it enough? Does it work in msys2 whatever? It seems like it wants "/c/python27/...."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Gilnaa should be enough, msys2 can recognize Windows paths.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what a time to be alive
3804e0a
to
8b7f1d0
Compare
@bors r=nrc |
📌 Commit 8b7f1d0 has been approved by |
Libtest json output A revisit to my [last PR](#45923). Events are now more atomic, printed in a flat hierarchy. For the normal test output: ``` running 1 test test f ... FAILED failures: ---- f stdout ---- thread 'f' panicked at 'assertion failed: `(left == right)` left: `3`, right: `4`', f.rs:3:1 note: Run with `RUST_BACKTRACE=1` for a backtrace. failures: f test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out ``` The JSON equivalent is: ``` { "type": "suite", "event": "started", "test_count": "1" } { "type": "test", "event": "started", "name": "f" } { "type": "test", "event": "failed", "name": "f" } { "type": "suite", "event": "failed", "passed": 0, "failed": 1, "allowed_fail": 0, "ignored": 0, "measured": 0, "filtered_out": "0" } { "type": "test_output", "name": "f", "output": "thread 'f' panicked at 'assertion failed: `(left == right)` left: `3`, right: `4`', f.rs:3:1 note: Run with `RUST_BACKTRACE=1` for a backtrace. " } ```
☀️ Test successful - status-appveyor, status-travis |
Is there any support in cargo for this feature? |
@farodin91 on nightly, Not sure if that is |
Is there an active RFC or a tracking issue for this feature? I know there's been discussion about overhauling testing in general, but I'm curious when/if this particular feature may become stable. |
A revisit to my last PR.
Events are now more atomic, printed in a flat hierarchy.
For the normal test output:
The JSON equivalent is: