-
-
Notifications
You must be signed in to change notification settings - Fork 102
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
Machine-readable output for cargo nextest run #20
Comments
This is something that we'd still like to do but it's just the work that needs to be done. Help wanted: If you're interested in tackling this project, here's the general set of things we need to do:
I'd estimate this to be around 80 hours of work for a newcomer to the project -- it's a chunky, nontrivial project but totally doable for someone new and interested. |
We're interested in doing this work so that we can feed structured test results to this. (Note that while it supports JUnit which nextest can already produce that option has limitations that make it less appealing). Before I started work on this, I wanted to run the idea I have for the initial implementation by you first before I do work that then gets rejected. Basically the idea would be to have a This adds a maintenance burden as breaking changes to the nightly output would need to be reflected in a new format-version in nextest, but I haven't actually looked at how often the format has changed historically to know if this is a major concern or not. This is mitigated by the ability to pin to a specific version so IMO it's not a huge deal, especially if the code is done in the right way, and thus hopefully fairly easy for contributors who need the latest and greatest format output to add support for them without too much trouble. This would also allow nextest to define its own format separately that would be unaffected by libtest decisions. |
Yay, super excited about this! Could you document the limitations of JUnit? It wasn't entirely clear to me what they were from reading the documentation. For reference, buildkite uses this test collector.
The idea of supporting libtest JSON is definitely interesting. Is this an accurate read of the format?
So what this suggests is that there have to be changes made to the buildkite collector, and honestly to anything else that consumes libtest JSON events, to support nextest. But at that point why don't we just define our own format?
Some other thoughts if we go down the libtest-json emulation path:
|
I think before we decide on the right path here, I'd like to see a more fleshed out proposal with how to handle retries and timeouts. I'd like to understand the limits of what can be supported within the libtest format. |
From here:
Yes, this is the output from
You are correct, my guess is the libtest output is more focused on a single crate use case but obviously loses relevant information inside a large workspace with hundreds or thousands of tests spread across multiple crates with n test binaries. That being said, I think the libtest output can still be replicated, just that the
Also correct, I think this could be fine by adding a "nextest" subobject in the event for this kind of additional information outside of the scope of libtest so that consumers can either ignore it if they don't understand it, or take advantage of it they do. That being said, in the retries case that might need to be handled explicitly by consumers if they have logic that will fail if it encounters the same test multiple times (though I suppose if nextest already does mutations on the test name by adding the crate name/test binary as stated above then having something like
My reason for suggesting having libtest compatible output was more so that projects that use other tooling that used to (or still does) depend on that format, as it was usable in stable rust for years before 1.70.0 made it unstable, can seamlessly (hopefully) drop in cargo-nextest without too much disruption, but I don't know how many people would actually find that useful. We will write our own buildkite test collector regardless, its support for the libtest output was just what made me think of other tooling in the wild that might be able to get that seamless interop with cargo-nextest if it supported the same output.
I think the easiest thing for me to understand all of this would be just to do a quick and dirty implementation (+ tests) in both nextest as well as a consumer to actually have a concrete thing to talk about, as I find that much easier, if that's ok? |
Hmm, I might be misunderstanding but how would you get the span information out of a Rust test? It seems like to do that you'd have to have tests produce the span information in a way that nextest can understand. This is certainly possible to do in principle but the protocol for it doesn't exist yet.
I think this is fine, we could use a separator like
I like having the
I'm curious how buildkite supports retries. They have a section on flaky tests but I didn't see anything in the JSON output which indicated support for that.
Sure, that sounds good. |
So I've got this mostly done, but I've hit a rather fundamental issue with how nextest-runner captures output, namely that it is split into 2 separate buffers for stdout and stderr. This makes it impossible to properly write the captured output to the json object in the same way that libtest does. Concretely, lets say we have this test. #[test]
fn fails() {
println!("this is stdout");
eprintln!("this is stderr");
panic!("fiddlesticks");
} The libtest output with { "type": "test", "name": "fails", "event": "failed", "exec_time": 0.009125365, "stdout": "this is stdout\nthis is stderr\nthread 'fails' panicked at src/lib.rs:34:5:\nfiddlesticks\nstack backtrace:\n 0: rust_begin_unwind\n at /rustc/aa1a71e9e90f6eb3aed8cf79fc80bea304c17ecb/library/std/src/panicking.rs:597:5\n 1: core::panicking::panic_fmt\n at /rustc/aa1a71e9e90f6eb3aed8cf79fc80bea304c17ecb/library/core/src/panicking.rs:72:14\n 2: myplayground::fails\n at ./src/lib.rs:34:5\n 3: myplayground::fails::{{closure}}\n at ./src/lib.rs:30:11\n 4: core::ops::function::FnOnce::call_once\n at /rustc/aa1a71e9e90f6eb3aed8cf79fc80bea304c17ecb/library/core/src/ops/function.rs:250:5\n 5: core::ops::function::FnOnce::call_once\n at /rustc/aa1a71e9e90f6eb3aed8cf79fc80bea304c17ecb/library/core/src/ops/function.rs:250:5\nnote: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.\n" } The I think the proper way to fix this would be to have a capture structure in nextest-runner that keeps a single buffer containing the combination of both stdout and stderr, and then an array that contains the ranges of bytes for each output block, the stream it came from, and the timestamp (or duration from start of capture), so that the output can be captured faithfully for this case (and others?), and then simple helpers that can reconstruct the individual streams as they are currently used in nextest, which shouldn't be much of a problem since the streams are currently just byte buffers, and the cases where they are interpreted as utf-8 strings already take the hit of (potentially) allocating by using the I can submit what I have now in a PR first, or I could do the described work in a separate PR first? Let me know how you want me to proceed. |
Perhaps you can use a pipe and then set stdout and stderr to both write into that pipe instead, when json output is required? That will be much simpler. |
I've opened #1086 as a draft, will create a helper for doing combined output in the same branch but can split it out into a separate PR if needed later. |
I've opened #1088 which adds support for interleaved test output that can have each stream recovered separately to satisfy all of the current code. |
Thanks Jake! Experimental support for this will be part of nextest 0.9.65, which I'll release tomorrow (in around 18 hours). https://nexte.st/book/run-machine-readable.html has some basic documentation, and the remaining issues are tracked in #1152 (help getting them done would be greatly appreciated). |
nextest 0.9.66 is now out (0.9.65 had a build issue) and it includes basic support. However, it is missing a few important bits like #1088. |
Apparently the last mentioned PR has landed as #1212. So, is this still incomplete? 🤔 |
@torokati44 Yes, because the libtest output isn't stable yet (see #1152). Besides, the libtest JSON format has some limitations, and we'll probably want to support at least one nextest-native format as well. Nextest is mostly a spare-time project for me these days, and I only add new features to it based on my employer's needs. If you'd like to see this happen, please get involved! Contributions would be really welcome. #1152 has a few tasks that are good first issues. |
The output should be similar to what cargo itself provides, complete with
--message-format json
and--message-format json-render-status
options.The text was updated successfully, but these errors were encountered: