Join GitHub today
GitHub is home to over 31 million developers working together to host and review code, manage projects, and build software together.
Sign upMachine-readable output of tests #1284
Conversation
skade
reviewed
Sep 17, 2015
|
|
||
| ### Structure | ||
|
|
||
| Output is stream of lines containing JSON objects separated by new line. Each |
This comment has been minimized.
This comment has been minimized.
skade
Sep 17, 2015
Contributor
It would make sense to agree on something like http://dataprotocols.org/ndjson/ or http://jsonlines.org/ for that or is that up to TAP-J to define?
This comment has been minimized.
This comment has been minimized.
hauleth
Sep 17, 2015
Author
It could also make usage of \1E ASCII code which refers to "Record Separator". This is still under consideration, but 1 record per line is simple enough.
This comment has been minimized.
This comment has been minimized.
skade
Sep 17, 2015
Contributor
I'd take '\n' or '\r\n', it's a pretty common format in the NoSQL-world (e.g. Elasticsearch bulk uses it) and JSON can be written newline-free very well.
This comment has been minimized.
This comment has been minimized.
hauleth
Sep 17, 2015
Author
But is inconsistent between platform as Windows still uses \r\n as a line separator and I believe that println! macro respect that. Although this needs reconsideration as I chosen new line as it was simple enough to implement in first draft of specs.
This comment has been minimized.
This comment has been minimized.
skade
Sep 17, 2015
Contributor
That inconsistency doesn't matter in that case as '{"test": "foo"}\r' is the same JSON value as '{"test": "foo"}'. Even if your IO isn't prepared for that, this is not an issue.
skade
reviewed
Sep 17, 2015
| # Drawbacks | ||
|
|
||
| This is breaking change in tooling and will require new tool that will provide | ||
| current functionality for compatibility reasons, but IMHO small pain for big gain. |
This comment has been minimized.
This comment has been minimized.
skade
Sep 17, 2015
Contributor
Is the test output a committed API? Using the test API directly requires #[feature(test)], so it is not breaking a committed API.
This comment has been minimized.
This comment has been minimized.
hauleth
Sep 17, 2015
Author
No, test isn't commited. But testing is available in stable Rust and change in that matter will be visible to non-compiler-devs also so I marked it as a breaking change.
This comment has been minimized.
This comment has been minimized.
masklinn
Sep 17, 2015
Why not make the machine-readable output an option and keep the human-readable output the default? Machine-readable output by default makes for a dreadful interactive experience.
FWIW pytest outputs a human-readable format to stdout and optionally a machine-readable one to a specified file.
The ability to output both human-readable and machine-readable is actually convenient in CI, you can send the human-readable output to a human-readable log and the machine-readable output to the CI's parser.
This comment has been minimized.
This comment has been minimized.
hauleth
Sep 17, 2015
Author
That's the point of drawbacks. That this should be written, but IMHO isn't point of this RFC. I can rewrite it to provide flag that will print out machine-readable style.
About writing to file machine-readable and on stdout human-readable then I think that one should use Unix tools:
cargo test | tee output.log | formatter
Where formatter is tool that change machine-readable input into desired human-readable output.
killercup
reviewed
Sep 17, 2015
| # Summary | ||
|
|
||
| Replace current test output with machine-readable one and add thin compatibility | ||
| layer on top of that. |
This comment has been minimized.
This comment has been minimized.
killercup
Sep 17, 2015
Member
and add thin compatibility layer on top of that.
What do you mean by "compatibility layer"? The JSON-based output (instead of binary Rust structs)? In Drawbacks you write that new tooling will be required for compatibility but this sentence sounds to me like that tooling is actually part of the RFC.
This comment has been minimized.
This comment has been minimized.
hauleth
Sep 17, 2015
Author
I mean display. I don't know how tooling should be done. There should be external program that will parse output and display it in the same way it is. Maybe it should be included into rustc but maybe into cargo. I have no idea and this need to be resolved.
This comment has been minimized.
This comment has been minimized.
|
I really like this proposal. Thanks for writing this! |
alexcrichton
added
the
T-dev-tools
label
Sep 17, 2015
This comment has been minimized.
This comment has been minimized.
|
cc @rust-lang/tools, definitely our territory! Thanks for the RFC @hauleth! Some thoughts of mine:
I personally always find it a good exercise to implement features like this ahead of time to get a good feeling about what's needed to implement the current functionality we have today. Along those lines it'd certainly help weed out what's needed for maintaining the same output in terms of benchmarks and tests, but it may also be a pretty significant chunk of work! |
This comment has been minimized.
This comment has been minimized.
|
About first point. I assumed that About second. It is completely possible to do so. I've just added that to be sure that we are running valid binary. About third. Of course it could be solution, but I don't see reason behind providing timestamp of beginning and ending of test, duration will do as well (maybe it's only me, but I doesn't care when my test started, I want to know how long it has been running). About last two it would be recommended to do so. It could simplify calls. About implementing that: I had that in mind, but currently I hadn't enough time to dig in |
This comment has been minimized.
This comment has been minimized.
jgraham
commented
Sep 17, 2015
|
For comparison we designed a similar format for logging browser test results at Mozilla. You probably don't need everything in that format, and may want some different things, but it's another point in the possible design space to consider. One of the driving considerations there was when you have tests from a third party source (e.g. because you are implementing some specification) there can be tests that you expect to fail but which you don't wish to edit. If that isn't a case you care about here you are likely to make differnt design choices. |
This comment has been minimized.
This comment has been minimized.
|
In general I'm a big fan of anything TAP based, and this JSON encoding removes most of problems with TAP. Human-readable output should definitely be the default, though. Perhaps an environment variable such as RUST_MACHINE_OUTPUT or a universally understood command-line to enable machine output. These tools are primarily for people, not machines. Changing Cargo to consume the machine-readable format is not an issue. Looking forward to seeing something like this develop! |
This comment has been minimized.
This comment has been minimized.
diwic
commented
Sep 18, 2015
|
|
This comment has been minimized.
This comment has been minimized.
erickt
commented
Sep 19, 2015
|
This is a great start! I think it could also be useful to capture the rustc command line arguments so that we can observe optimization levels, enabled feature flags, and etc. For reference, a bunch of other test libraries produce the xUnit XML File Format. |
This comment has been minimized.
This comment has been minimized.
I don't see why it should be an environment variable; environment variables are much harder to reason about than command line flags (it's a bit like dynamic scope vs. lexical scope), and it's not like we need to adjust some behavior deep within a stack of executing programs. A flag on the compiled test runner ( |
This comment has been minimized.
This comment has been minimized.
Ah yeah I meant in addition to the suite object there'd also be an object for "this test has started to run". That gives consumers a notion of what tests are currently running, e.g. those you've seen start records for and haven't seen end records for. Additionally if a consumer of the output wants to provide a progress bar this would be useful information perhaps.
Ah just in the sense that we don't have a lot of timestamp support in the standard library just yet, so it'd be difficult to implement this in-tree (e.g. whereas it'd be pretty easy to do it out-of-tree), so for ease of implementation we may want to avoid this for now.
Ah yeah no worries! You're not on the hook to implement this or anything like that, just some musings from me! |
This comment has been minimized.
This comment has been minimized.
|
What are the limitations on TAP that warrant the invention of another format? To my understanding this format would be Though being slightly inconvenient to parse, I think that using TAP yields more benefits in terms of tooling, adoption, familiarity and interoperability than creating something new. It's quite annoying if every language comes with their own custom way of formatting test output. E.g. I don't think Rust should fall into the same trap Golang fell into with their test output:
|
This comment has been minimized.
This comment has been minimized.
|
@yoshuawuyts the only problem with TAP is that this is primitive format, that doesn't provide way to message a lot of things (like type of test, performance, etc.) in uniform way. RusTAP is created to fit into Rust testing framework as it has some quirks that original TAP-J doesn't cover (i.e. benches). |
This comment has been minimized.
This comment has been minimized.
IanConnolly
commented
Oct 27, 2015
|
I'd be happy to help with moving this along, as I've been wanting this myself recently. |
This comment has been minimized.
This comment has been minimized.
|
My quick comments:
|
This comment has been minimized.
This comment has been minimized.
|
Closing as I want to rewrite it for TAP13 protocol (wider usage, and already existing tools). |
hauleth
closed this
Jan 8, 2016
This comment has been minimized.
This comment has been minimized.
tj
commented
Jan 9, 2016
|
IMO the problem with Go's is that you can't replace the output generation. I know the Go team has an issue open for considering JSON output, but to me It would be so much nicer if you could just: import (
_ "my/fancy/test/output" // register a replacement hook
)Then you don't have to fight about JSON, TAP, etc, just use whatever you like. Maybe Rust could go that route instead of a base format? |
This comment has been minimized.
This comment has been minimized.
andrew-d
commented
Jan 9, 2016
|
|
This comment has been minimized.
This comment has been minimized.
jonastepe
commented
Jan 9, 2016
|
Then there still has to be a sensible default or fallback for those that do not provide such a plugin. |
This comment has been minimized.
This comment has been minimized.
jonastepe
commented
Jan 9, 2016
|
However, I am also in favor of this plugin approach. |
This comment has been minimized.
This comment has been minimized.
|
I am in the progress of rewriting Łukasz Jan Niemier Dnia 9 sty 2016 o godz. 09:55 Jonas Tepe notifications@github.com napisał(a):
|
This comment has been minimized.
This comment has been minimized.
Do you need any help with this? I'd very much like testing to get better in Rust! |
nagisa
referenced this pull request
Mar 30, 2016
Closed
Allow more-easily-parsable output format for libtest benchmark harness #32595
This comment has been minimized.
This comment has been minimized.
milgner
commented
Oct 9, 2016
|
Is this still being worked on? I think Rust would greatly benefit from this as it makes integration of tests into CI systems much more elegant. TAP 13 seems like a good format, too, even if it doesn't explicitly contain a differentiation between regular tests and benchmarks. But I guess a |
This comment has been minimized.
This comment has been minimized.
denniscollective
commented
Sep 3, 2017
|
Can you reopen this? |
hauleth commentedSep 17, 2015
Rendered