Add libtest JSON emitter #316

kirtchev-adacore · 2025-04-24T09:11:56Z

This PR adds a status emitter which outputs the libtest JSON format, as specified in library/test/src/formatters/json.rs.

fixes #315

TODO (check if already done)

Add tests
Add CHANGELOG.md entry

src/status_emitter.rs

kirtchev-adacore · 2025-04-24T09:14:23Z

The PR is currently broken as I am trying to figure out how to integrate the emitter in lib.rs. A colleague will have a look.

There are also no tests at the moment.

src/status_emitter.rs

src/status_emitter/text.rs

src/config/args.rs

src/status_emitter/libtest_json.rs

kirtchev-adacore · 2025-05-14T11:45:28Z

Oli, could you advise on how to write tests for this PR? I was looking into tests, and here is what I understood (which is most likely wrong or incomplete):

tests/integration.rs is the only place which mentions output formatting, so I assume that this is the testing entry point. The Args returned by Args::test() are parsed from the command line, so I assume I would need to pass --format=json, but where?

tests/integration.rs then calls run_tests_generic() on what I assume is the tests/integrations directory as a testing root.

And from here on, I have no idea what is going on. I don't seem to find any expected output or oracles...

kirtchev-adacore · 2025-05-15T13:47:44Z

I struggled with testing and ended up copying and adapting integrations/basic and integrations/basic-fail to work with the JSON emitter. I noticed a couple of issues:

The testing framework appears to be either running or reporting on (the more likely) a single test several times:

{ "type": "test", "event": "ok", "name": "tests/actual_tests/bad_pattern.rs" }
{ "type": "test", "event": "failed", "name": "tests/actual_tests/bad_pattern.rs", . . .}
. . .
{ "type": "test", "event": "ok", "name": "tests/actual_tests/executable.rs" }
{ "type": "test", "event": "ok", "name": "tests/actual_tests/executable.rs" }

serde_json::to_string() does not properly escape { and }. If the stderr or stdout contain these characters, the JSON ends up being malformed. I will have to check what EscapedString does with these characters.

src/status_emitter/json.rs

tests/integration.rs

oli-obk · 2025-05-15T19:31:57Z

tests/integrations/json-basic/tests/ui_tests.rs

+            ui_test::error_on_output_conflict
+        },
+        bless_command: Some("cargo test".to_string()),
+        ..Config::rustc("tests/actual_tests")


instead of copying the basic test suite, use a path here pointing to the already existing test suite

How did I not think of this ... will do!

Done.

There are however issues with building the auxiliary files in basic and basic-fail. I still need to investigate.

That may just be my whacky test setup. So if there is too much trouble I'll check out your PR locally and fix it myself.

I wasn't able to figure out what the source of the auxiliary file build failure is, so I could use your help Oli.

I also noticed something else - the diffing portion of the testing seems to have a character limit. This was cutting off the terminating JSON brace among other things, and I thought that there was a general issue with the escaping. Here is what I observed:

Diff report:

Same "pure" output:

How do you want me to handle this?

hmmm I see it. Maybe this is sth similar to #308 At least we have a test now 😆

tests/integrations/json-basic-fail/tests/ui_tests.rs

src/status_emitter/json.rs

oli-obk · 2025-05-15T19:41:31Z

please run rustfmt and clippy (cargo clippy --fix should work for most changes)

I think the reason you see some output twice is that e.g. for //@run tests you get one message for the test being compiled and one message for the compiled artifact being executed.

src/status_emitter/json.rs

oli-obk · 2025-05-15T19:45:24Z

src/status_emitter/json.rs

+            String::new()
+        };
+
+        emit_test_end(&self.name, &status, &error_output);


the revision and the path are not part of the json, so you are getting duplicate messages. I'm ok with putting all of this into the name like format!("{name}({revision}): {path}") or sth. Usually when the revision is an empty string we don't print it at all (like not even empty parentheses).

My preferred solution would be extra fields in the json, but I understand that this is suboptimal due to none of the json tooling consuming revision and path information yet.

Note that I treat the path of the test as the name of the test. In addition, when creating a TestStatus, the revision is an empty string.

Is there a better way of obtaining the name and revision of a test?

You are already tracking it correctly in for_revision and for_path. So I think you just need to print it as part of the name

oli-obk · 2025-05-18T07:48:01Z

I struggled with testing and ended up copying and adapting integrations/basic and integrations/basic-fail to work with the JSON emitter. I noticed a couple of issues:

bonus points if you don't need any duplication but can make it work by adding #@revisions default json to the Cargo.toml. It's gonna be icky telling the test about json, probably needs sth like #@[json] rustc-env: USE_JSON=1 or going via compile-flags with like -- --emit=json

oli-obk · 2025-05-19T07:52:28Z

bonus points if you don't need any duplication but can make it work by adding #@revisions default json to the Cargo.toml. It's gonna be icky

This is just a random idea that I don't know if it will work. Just adjusting the path but duplicating the test is fine by me

src/status_emitter/json.rs

kirtchev-adacore · 2025-05-19T11:44:09Z

Oli, I think the PR is in good shape. There are a couple of outstanding issues which break testing:

The diffing is slicing off some of the actual output of two tests from the basic-fail tests.
The auxiliary file build failure in basic is registered as a failure even though the outputs satisfy the oracles.

This PR adds a status emitter which outputs the libtest JSON format, as specified in `library/test/src/formatters/json.rs`. Issue: oli-obk#315

oli-obk · 2025-05-19T15:00:06Z

The diffing is slicing off some of the actual output of two tests from the basic-fail tests.

I think we can ignore that. cargo test -- -- --bless produces the "right" (matching) diff output, so while the diff output is broken, we can just keep it as is and I'll worry about it on the main branch.

The auxiliary file build failure in basic is registered as a failure even though the outputs satisfy the oracles.

yea there was some weird stuff going on. I just merged the json tests into the regular test suite and that seems to have done the trick

oli-obk · 2025-05-19T15:01:14Z

windows path escaping again 💀 I'll merge the PR without windows and fix it on main on my windows machine

oli-obk · 2025-05-19T15:02:54Z

Thanks for bearing with me through all the change requests! I wanted a json backend for a while.

On that note, I assume bogus (unknown to you) json messages or extraneous json fields don't matter to you, right? So I can just add stuff even if you are an active consumer that will suddenly see more json than they need?

kirtchev-adacore · 2025-05-20T07:42:08Z

Thanks for bearing with me through all the change requests! I wanted a json backend for a while.

Are you kidding, it was a pleasure! I learned a lot in the process, in fact, thank you very much for keeping with my Rust inadequacies!

On that note, I assume bogus (unknown to you) json messages or extraneous json fields don't matter to you, right? So I can just add stuff even if you are an active consumer that will suddenly see more json than they need?

I think we should be fine. 👌