Add project name to junit.xml output #261

nikolaigut · 2022-06-27T13:44:25Z

Add attributes name, timestamp, tests, failures, errors, skipepd, time, hostname to the testsuites xml node, according to the schema: https://github.com/windyroad/JUnit-Schema/blob/master/JUnit.xsd.

dnephin

Thank you for the PR! Sorry it took me some time to read over it.

One of the challenges of Junit.xml is that there is no official spec (as far as I've seen). Some people have published their version of a spec, but I don't think the tools and systems that read the file all agree.

So far the junit.xml output of gotestsum has mostly been limited to what common CI systems will use. It doesn't make any effort to add other fields unless they will be used by something.

Can you elaborate on your use case? What tool or system is receiving the junit.xml, and how does it make use of these fields? Understanding that will help ensure we are building the right thing.

I think it would be fine to add some more fields. I've left some suggestions an questions below as well.

.gitignore

cmd/main.go

dnephin · 2022-07-16T20:01:36Z

internal/junitxml/report.go

+	Name      string   `xml:"name,attr,omitempty"`
+	Timestamp string   `xml:"timestamp,attr"`
+	Tests     int      `xml:"tests,attr"`
+	Failures  int      `xml:"failures,attr"`
+	Errors    int      `xml:"errors,attr"`
+	Skipped   int      `xml:"skipped,attr"`
+	Time      float64  `xml:"time,attr"`
+	Hostname  string   `xml:"hostname,attr,omitempty"`


I'm not sure this is correct. From my read of the XSD file, these should be properties of JUnitTestSuite not JUnitTestSuites. Am I missing something? Can you explain more about why they would be on JUnitTestSuites ?

JUnitTestSuite already has many of these fields, but we could add the ones that are missing.

This is a little tricky. In some definitions are these properties available for JUnitTestSuites (e.g., https://github.com/junit-team/junit5/blob/main/platform-tests/src/test/resources/jenkins-junit.xsd) and for others not.
I am using this parser to generate a user-friendly view of the tests https://lotterfriends.github.io/online-junit-parser/ and there these properties are used for the title.

Even in that jenkins schema, the testsuites only has name, time, tests, failures, disabled, and errors. It doesn't have hostname, timestamp, or skipped. Those are all properties of testsuite.

Looking at the source of the tool you linked,

https://github.com/lotterfriends/online-junit-parser/blob/d279015fac02c402ec12ce033f1a433508914b35/index.html#L122-L132

I don't see hostname in there either. Hostname is on the testsuite (not testsuites).

So something still doesn't look right here.

I looked for title in that source, and I guess that's the <h1> here, is that right?

https://github.com/lotterfriends/online-junit-parser/blob/d279015fac02c402ec12ce033f1a433508914b35/index.html#L174

That seems to only use the name. Are you sure these other properties are required?

internal/junitxml/report_test.go

dnephin · 2022-07-16T20:08:27Z

testjson/execution.go

@@ -471,6 +472,17 @@ func (e *Execution) Elapsed() time.Duration {
 	return clock.Now().Sub(e.started)
 }

+func (e *Execution) ElapsedOnWrite() time.Duration {


What's the purpose of this? Does Elapsed() not give the correct value for some reason?

I'm still curious about this. I think we can find a better way to solve this than introducing two new methods on Execution.

nikolaigut · 2022-07-19T12:57:44Z

Thank you for the PR! Sorry it took me some time to read over it.

One of the challenges of Junit.xml is that there is no official spec (as far as I've seen). Some people have published their version of a spec, but I don't think the tools and systems that read the file all agree.

So far the junit.xml output of gotestsum has mostly been limited to what common CI systems will use. It doesn't make any effort to add other fields unless they will be used by something.

Can you elaborate on your use case? What tool or system is receiving the junit.xml, and how does it make use of these fields? Understanding that will help ensure we are building the right thing.

I think it would be fine to add some more fields. I've left some suggestions an questions below as well.

Thank you very much for your review.

I need these changes to generate a more user-friendly report with the tool https://lotterfriends.github.io/online-junit-parser/, which uses some of these properties in the title.

dnephin

Thanks! The test is looking good. Just a few more questions to make sure I understand the purpose of some of these additions.

dnephin · 2022-07-23T17:52:00Z

internal/junitxml/report.go

+	Name      string   `xml:"name,attr,omitempty"`
+	Timestamp string   `xml:"timestamp,attr"`
+	Tests     int      `xml:"tests,attr"`
+	Failures  int      `xml:"failures,attr"`
+	Errors    int      `xml:"errors,attr"`
+	Skipped   int      `xml:"skipped,attr"`
+	Time      float64  `xml:"time,attr"`
+	Hostname  string   `xml:"hostname,attr,omitempty"`


Even in that jenkins schema, the testsuites only has name, time, tests, failures, disabled, and errors. It doesn't have hostname, timestamp, or skipped. Those are all properties of testsuite.

Looking at the source of the tool you linked,

https://github.com/lotterfriends/online-junit-parser/blob/d279015fac02c402ec12ce033f1a433508914b35/index.html#L122-L132

I don't see hostname in there either. Hostname is on the testsuite (not testsuites).

So something still doesn't look right here.

I looked for title in that source, and I guess that's the <h1> here, is that right?

https://github.com/lotterfriends/online-junit-parser/blob/d279015fac02c402ec12ce033f1a433508914b35/index.html#L174

That seems to only use the name. Are you sure these other properties are required?

dnephin · 2022-07-23T17:53:26Z

testjson/execution.go

@@ -471,6 +472,17 @@ func (e *Execution) Elapsed() time.Duration {
 	return clock.Now().Sub(e.started)
 }

+func (e *Execution) ElapsedOnWrite() time.Duration {


I'm still curious about this. I think we can find a better way to solve this than introducing two new methods on Execution.

nikolaigut · 2022-08-05T07:30:34Z

Thanks! The test is looking good. Just a few more questions to make sure I understand the purpose of some of these additions.

Thanks for this input. You are totally right. I removed the unused properties.

Co-authored-by: Daniel Nephin <dnephin@gmail.com>

dnephin

Looks good! Thank you for the PR!

I rebased to resolve the conflicts, squashed the commits, and made a couple small changes

renamed the flag to match the other junitfile flags, and restored the previous default of no name
changed the field type of Elapsed to string to match the other time field

Bumps gotest.tools/gotestsum from 1.7.0 to 1.8.2. Release notes Sourced from gotest.tools/gotestsum's releases. v1.8.2 What's Changed Show shuffle seed by @dnephin in gotestyourself/gotestsum#256 Update tests, and cleanup formats by @dnephin in gotestyourself/gotestsum#262 Remove one dependency, update the rest by @dnephin in gotestyourself/gotestsum#263 Test against go1.19, remove go1.15 by @dnephin in gotestyourself/gotestsum#269 Add project name to junit.xml output by @nikolaigut in gotestyourself/gotestsum#261 Adding in support for s390x and ppc64le by @james-crowley in gotestyourself/gotestsum#270 New Contributors @nikolaigut made their first contribution in gotestyourself/gotestsum#261 @james-crowley made their first contribution in gotestyourself/gotestsum#270 Full Changelog: gotestyourself/gotestsum@v1.8.1...v1.8.2 v1.8.1 Changelog e209ec9 Merge pull request #249 from dnephin/add-update-flag 849289b Add to readme 57ba68e Add some tests for watch events 14592d7 watch: add u key for updates f1fdcbd Merge pull request #246 from dnephin/warn-watch-and-paths ddb4125 Document --watch with extra package paths 938480e Merge pull request #245 from dnephin/aix 6b46741 Fix the build on aix v1.8.0 Changelog 69fcbcb Merge pull request #243 from dnephin/enable-color-on-github-actions 9e8433c Enable color by default when run from github actions a8b4e27 Merge pull request #242 from gotestyourself/docs-post-run 4cc96fb Improve the docs for --post-run-command cc0c1a1 Merge pull request #234 from uhthomas/exec-stdin ac4bc6f Improve the test for input from stdin ce158b4 Pass stdin to go test 0fa88f3 Merge pull request #241 from gotestyourself/mkdir-path d93af71 create any missing directories for {json,junit}file 528a3fd Merge pull request #235 from ashanbrown/asb/ignore-warnings bba51ac Fix e2e test failure 90a5a10 Ensure we still rerun tests when there are warnings d9cd222 Merge pull request #237 from gotestyourself/ci-go1.18 99a7f30 fix goreleaser afec7a6 ci: go1.18 661b091 Merge pull request #226 from g-gaston/compile-test2json-without-go 42e50c2 Add instructions to compile test2json without the need of a local go installation 08a7689 Merge pull request #224 from greut/fix/golangci-lint 2a49979 fix: remove deprecated linters from golanci-lint 87d6387 Merge pull request #223 from dmitris/patch-1 98a1847 update 'go get' idirective to 'go install' e5de87c Merge pull request #216 from dnephin/pkg-elapsed ... (truncated) Commits f863151 Merge pull request #270 from james-crowley/alt_arch_support 85a7f9e feat: Adding in support for s390x and ppc64le 9fea242 Merge pull request #261 from nikolaigut/main 20f51e0 Add project name option for junit xml de44b55 Merge pull request #267 from dnephin/update-readme c54abf9 Set golangci-lint config path in CI 5a46af5 Demo image url 9bab80b .project directory e8ca43d Consolidate 9abe001 Move log to internal Additional commits viewable in compare view Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting @dependabot rebase. Dependabot commands and options You can trigger Dependabot actions by commenting on this PR: @dependabot rebase will rebase this PR @dependabot recreate will recreate this PR, overwriting any edits that have been made to it @dependabot merge will merge this PR after your CI passes on it @dependabot squash and merge will squash and merge this PR after your CI passes on it @dependabot cancel merge will cancel a previously requested merge and block automerging @dependabot reopen will reopen this PR if it is closed @dependabot close will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually @dependabot ignore this major version will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) @dependabot ignore this minor version will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) @dependabot ignore this dependency will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Kevin Robayna <me@kevinrobayna.com>

dnephin reviewed Jul 16, 2022

View reviewed changes

dnephin reviewed Jul 23, 2022

View reviewed changes

Add project name option for junit xml

20f51e0

Co-authored-by: Daniel Nephin <dnephin@gmail.com>

dnephin changed the title ~~Add attributes name, timestamp, tests, failures, errors, `s…~~ Add project name to junit.xml output Aug 6, 2022

dnephin approved these changes Aug 6, 2022

View reviewed changes

dnephin merged commit 9fea242 into gotestyourself:main Aug 6, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add project name to junit.xml output #261

Add project name to junit.xml output #261

nikolaigut commented Jun 27, 2022

dnephin left a comment

dnephin Jul 16, 2022

nikolaigut Jul 19, 2022

dnephin Jul 23, 2022

dnephin Jul 16, 2022

dnephin Jul 23, 2022

nikolaigut commented Jul 19, 2022

dnephin left a comment

dnephin Jul 23, 2022

dnephin Jul 23, 2022

nikolaigut commented Aug 5, 2022

dnephin left a comment

Add project name to junit.xml output #261

Add project name to junit.xml output #261

Conversation

nikolaigut commented Jun 27, 2022

dnephin left a comment

Choose a reason for hiding this comment

dnephin Jul 16, 2022

Choose a reason for hiding this comment

nikolaigut Jul 19, 2022

Choose a reason for hiding this comment

dnephin Jul 23, 2022

Choose a reason for hiding this comment

dnephin Jul 16, 2022

Choose a reason for hiding this comment

dnephin Jul 23, 2022

Choose a reason for hiding this comment

nikolaigut commented Jul 19, 2022

dnephin left a comment

Choose a reason for hiding this comment

dnephin Jul 23, 2022

Choose a reason for hiding this comment

dnephin Jul 23, 2022

Choose a reason for hiding this comment

nikolaigut commented Aug 5, 2022

dnephin left a comment

Choose a reason for hiding this comment