-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Add JSON reporter #2706
Add JSON reporter #2706
Conversation
@horenmar this PR is still in the drafting phase but could you have a look at the API for |
12bc1cf
to
fb0d4ad
Compare
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.
I need some context for the review. Do you want me to review the JSON writer code or the JSON output format as well?
I looked at the code locally and left some quick notes directly in the code.
More high level notes:
- Please don't touch
catch_amalgamated.*
- The output should always contain a "version" field, for now set to 1.
- Listings can be combined and the output format should support this, because something like
./SelfTest --list-tests --list-tags
is a valid invocation and the output needs to work. (This currently doesn't work with the XML reporter, but that's a bug that will need fixing at some point.)
Both now actually, the whole thing is already functional so it would be great if you look at it end to end.
Sorry about that, the tests were failing and I didn't know what to do. I'll address the other issues later. |
About output format:
Taken together, this means that the top level object should look like this {
"version": 1,
"metadata" : { ... },
"listing": { ... },
"test-results": { ... }
} listingsTags are easy, we just copy the xml reporter's output, adjusted for JSON. I think something like this is fine "tags": [
{"aliases": ["Foo", "foo"], "count": 2},
{"aliases": ["decomposition", "Decomposition"], "count": 5},
...
] Same goes for listing reporters, there is very little data in there "reporters": [
{ "name": "JSON", "description": "..." },
{ "name": "XML", "description": "..." },
] and listeners are analogous, except their top level key should be "listeners" obviously. Tests are more interesting because they have fields that can be effectively empty, e.g. tags, class name (almost always empty). After thinking about it, I think the JSON reporter should do the same thing XML reporter does; always provide the key, and an empty string/list/other type as appropriate. Otherwise we can reuse the format from XML reporter (with the obvious adjustments for XML vs JSON). test runThere is a bug in the output right now, observe this "sections": [
{
"name": "Function pointer",
"filename": "/mnt/c/ubuntu/Catch2-uyha/tests/SelfTest/UsageTests/Matchers.tests.cpp",
"line": 599,
"assertions": {
"passed": 2,
"failed": 0,
"fail-but-ok": 0,
"skipped": 0
}
},
{
"name": "Arbitrary predicate matcher",
"filename": "/mnt/c/ubuntu/Catch2-uyha/tests/SelfTest/UsageTests/Matchers.tests.cpp",
"line": 598,
"assertions": {
"passed": 2,
"failed": 0,
"fail-but-ok": 0,
"skipped": 0
}
},
{
"name": "Lambdas + different type",
"filename": "/mnt/c/ubuntu/Catch2-uyha/tests/SelfTest/UsageTests/Matchers.tests.cpp",
"line": 603,
"assertions": {
"passed": 2,
"failed": 0,
"fail-but-ok": 0,
"skipped": 0
}
},
{
"name": "Arbitrary predicate matcher",
"filename": "/mnt/c/ubuntu/Catch2-uyha/tests/SelfTest/UsageTests/Matchers.tests.cpp",
"line": 598,
"assertions": {
"passed": 2,
"failed": 0,
"fail-but-ok": 0,
"skipped": 0
}
}
], "Arbitrary predicate matcher" is the name of the test, and is reported multiple times, and in weird order. The first is due to the fact that every The weird order is because the reporter writes the section metadata in TEST_CASE("TEST NESTED SECTIONS") {
SECTION( "First" ) {
SECTION( "Second" ) {}
}
} gives this output
Past this, notes on the output. test case metadata "test-cases": [
{
"test-info": {
"name": "Arbitrary predicate matcher",
"tags": [
"generic",
"matchers"
],
"source-location": {
"filename": "/mnt/c/ubuntu/Catch2-uyha/tests/SelfTest/UsageTests/Matchers.tests.cpp",
"line": 598,
}
"properties": ["is-hidden", "ok-to-fail", ...]
}
"sections": [
...
section results "sections": [
{
"name": "Function pointer",
"source-location": {
"filename": "/mnt/c/ubuntu/Catch2-uyha/tests/SelfTest/UsageTests/Matchers.tests.cpp",
"line": 599,
},
"assertions-stats": {
"passed": 2,
"failed": 0,
"failed-but-ok": 0
},
"succeeded": true,
"skipped": false
"sections": [
{ ... nested sections go here ... }
]
},
...
]
assertion results & benchmarks Having all assertions output by default might get very noisy, very quickly. On the other hand, the machine readable reporters are the ones that should provide all possible information XML reporter currently distinguishes this according to the |
81c7a56
to
1b95cd1
Compare
I don't think |
There is no event for listing. I am not sure if one should be added, but the JSON reporter can keep track of whether it has opened the listing object already, and if not open it in the called listing function. Listings and test runs cannot be combined, so test run reporting does not have to deal with the potentially opened object. |
…rBase` This commit is a POC to check if the current structure is ok or not, more details need to be filled in still
@horenmar I've rewritten implementation for the JSON reporter with cumulative instead since it feels impossible, or I'm just too dumb to figure it out, to implement JSON with the streaming approach. Could you have a look at the current structure? Also, in the points you pointed out, there are |
That seems odd, but fine for now.
A section can only be skipped once, so it makes more sense to write TEST_CASE( "nested sections can be skipped dynamically at runtime",
"[skipping]" ) {
SECTION( "B" ) {
SECTION( "B1" ) { std::cout << "b1"; }
SECTION( "B2" ) { SKIP(); }
SECTION( "B3" ) { SKIP(); }
SECTION( "B4" ) { std::cout << "b4"; }
}
std::cout << "!\n";
} should section B report skipped zero times, one time or twice? So nevermind that for now. However, I found some bugs in the output of the test case above, which I will have to look into. Since this started primarily for the listings, let's get the output format for that done, and I can fix this up, while you adapt the CMake test registration script. I should have time to work on this during the weekend. |
The problem is figuring out when to close the
Then should I split this into 2 PRs? one for the JSON writer and listing only and one for the full JSON reporter. The listing part is already taken of in this PR. |
* Nest the listing outputs inside an object. * print versions and other metadata during listing * output finishes with a newline
m_startedListing = true; | ||
} | ||
|
||
// TODO: Why? This should just assert. If the object is not there, there is a bug. |
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.
I wasn't sure what to do when writing this, since I saw something similar to this in the XML reporter, I followed it
You can branch off this branch for the CMake script changes, I'll finish this PR and the second one should get retargeted automatically. |
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## devel #2706 +/- ##
==========================================
+ Coverage 91.36% 91.45% +0.09%
==========================================
Files 190 194 +4
Lines 7855 8176 +321
==========================================
+ Hits 7176 7477 +301
- Misses 679 699 +20 |
Okay, so I redid the json reporter to go back to streaming and made a skeleton implementation that shows how it could look. I didn't finish handling all the different assertion states/etc because it is a lot of work that I am not interested in doing at this time, but I want to unblock #2690 , so I will squash merge this into main, and finish the full reporter eventually. |
Description
This PR adds a JSON reporter.
GitHub Issues
#2690 (comment)