-
Notifications
You must be signed in to change notification settings - Fork 7
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
Implement json2junit #212
base: main
Are you sure you want to change the base?
Implement json2junit #212
Conversation
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.
Nice. Overall, I wonder if it would be useful to also expose it as a library? Then it could be used directly in _util
without an extra pipeline step (and extra file to manage explicitly). (Then it could also potentially be streamed.)
Good call. It makes sense. Done 😺. This is the new API: func Convert(w io.Writer, r io.Reader) error
func ConvertFile(out, in string) error
type Converter struct {
// Has unexported fields.
}
func NewConverter(w io.Writer) *Converter
func (c *Converter) Close() error
func (c *Converter) Write(b []byte) (int, error) |
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.
Some suggestions, but still looking good.
@@ -0,0 +1,7 @@ | |||
<?xml version="1.0" encoding="UTF-8"?> | |||
<testsuites> | |||
<testsuite name="cmd/go" tests="2" failures="0" skipped="0" time="0.011" timestamp="2025-02-07T14:43:16+01:00"> |
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.
It might be worth adding a tiny happy path test json that's easy for a person to read in full (without truncation by default) (maybe also easier to debug into). chatty.json
is the smallest one, but it's still intentionally not short.
Here's one: 828f05c
@@ -0,0 +1,100 @@ | |||
{"Time":"2025-02-07T14:43:16.4746636+01:00","Action":"start","Package":"cmd/go"} | |||
{"Time":"2025-02-07T14:43:16.4751944+01:00","Action":"run","Package":"cmd/go","Test":"TestScript"} |
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.
.jsonl
will make VS Code's syntax checker happy, by the way. ("JSON Lines".)
|
||
func (s junitTestSuite) testCase(name string) *junitTestCase { | ||
for _, tc := range s.TestCases { | ||
if tc.Name == name { |
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.
Hmm, it seems like testCase
and suite
are called in a decently tight loop, and I wonder if maps are actually still worth the trouble...
return fmt.Errorf("no start entry for %v", entry.Package) | ||
} | ||
testCase := suite.testCase(entry.Test) | ||
if testCase == nil { |
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.
Why no error if there's a duplicate test start, like the error for duplicate suite start?
Co-authored-by: Davis Goodin <[email protected]>
This PR implements a simpler converted from Go JSON output to JUnit XML files.
The implementation is less that 300 lines. I don't expect it to grow in the future, it is feature completetm. The remaining additions in this PR are just test files.
The alternatives are:
All in all, I think it worth having some tailored parser to cover our limited use-case.