Add the file/line to the output of Junit XML #470

asfaltboy · 2025-02-27T08:45:34Z

Implement the proposed solution in #264 (comment) to parse the text output of a failure (which contains a useful trace) add then file/line attributes to the junit xml output.

This is useful when parsing the XML output e.g in CI, in order to link to the file (e.g Github annotations, as done by mikepenz/action-junit-report).

I tested it on a local project, and can confirm the relative path to the file and the line was added as attributes when I ran gotestsum in the root of my module. Here is some anonymised output example excerpt:

<?xml version="1.0" encoding="UTF-8"?>
<testsuites tests="30" failures="4" errors="0" time="17.950271">
	<testsuite tests="30" failures="4" time="14.719000" name="github.com/mycool/package" timestamp="2025-02-27T18:26:48Z">
		<properties>
			<property name="go.version" value="go1.23.2 darwin/arm64"></property>
		</properties>
		<testcase classname="github.com/mycool/package" name="TestSomeService_ProcessFoo_Completed" time="1.170000" file="mycool/package/foo_test.go" line="353">
			<failure message="Failed" type="">=== RUN   TestSomeService_ProcessFoo_Completed ... some long output ... /workspace/mycool/package/foo_test.go:56&#xA;        &#x9;Error:      &#x9;Not equal: &#xA;        &#x9;            &#x9;expected: &#34;ended&#34;&#xA;        &#x9;            &#x9;actual  : &#34;active&#34;&#xA;        &#x9;            &#x9;&#xA;        &#x9;            &#x9;Diff:&#xA;        &#x9;            &#x9;--- Expected&#xA;        &#x9;            &#x9;+++ Actual&#xA;        &#x9;            &#x9;@@ -1,2 +1,2 @@&#xA;        &#x9;            &#x9;-(model.FooStatus) (len=5) &#34;ended&#34;&#xA;        &#x9;            &#x9;+(model.FooStatus) (len=6) &#34;active&#34;&#xA;        &#x9;            &#x9; &#xA;        &#x9;Test:       &#x9;TestSomeService_ProcessFoo_Completed&#xA;--- FAIL: TestSomeService_ProcessFoo_Completed (1.17s)&#xA;</failure>
		</testcase>
		<testcase classname="github.com/mycool/package" name="TestSomeService_CancelFoo_Ok" time="1.070000"></testcase>
		<testcase classname="github.com/mycool/package" name="TestSomeService_CancelFoo_CannotCancel/completed" time="0.020000"></testcase>
	</testsuite>
</testsuites>

Merging this would be a proper fix to #264

TODO

correct the relative path (use ~~goModuleFilePath~~ current working dir and clean it up, hard-code in tests), as it's currently a bit too low down the tree

asfaltboy · 2025-03-03T10:25:40Z

@dnephin kindly review when you get the chance 🙏 happy to address any concerns

dnephin

Thank you for the PR! If you pull in the latest changes from main that should fix the test failures.

In general I think the approach of parsing the file:line from the output is ok, but I don't think it's safe to expect the "Error Trace" prefix.

We may want to do this parsing on the event before we join all the lines. I think that will make it easier to find the relevant file:line.

There's also this proposal to add an OutputType field: golang/go#62728 (comment)

I think that field would make it much easier to find the appropriate file:line in the output. We could parse it directly from only the events that have an OutputType: error.

internal/junitxml/report.go

cmd/tool/parser/parser.go

dnephin · 2025-03-13T02:08:55Z

cmd/tool/parser/parser.go

+		// Usually the failure would contain a line like this:
+		// Error Trace:	/Users/user/proje/path/to/package/some_test.go:42
+		// where the full path to the file is in the same line as "Error Trace:"
+		if strings.Contains(outputLine, "Error Trace") {


I don't think this I've ever see an "Error Trace" line like this before. Is this from a specific test helper library?

Most test failures I see look like this:

fails_test.go:50: failed sub

I don't think we can rely on this prefix.

My bad! I misunderstood where 'Error Trace' it's coming from, and wrongly assumed it's from Go's test tooling. But, in fact this comes from the testify package that I often use for constructing assertions.

Here's how they define the value for an 'error trace': https://github.com/stretchr/testify/blob/7127b6099902025b069e842d93bcdd6e8462eb5c/assert/assertions.go#L207-L286

I picked this option because it was a more correct file:line value in some cases. But clearly we cannot rely on users to been using testify, and there's no way for us to mimic this behaviour without some code being invoked in the error...

I think it's best that we switch to the common output for now, as you recommend. We can discuss separately if it's worth to add optional "support for testify-style error output", if we find it valuable.

cmd/tool/parser/parser.go

samloop · 2025-05-06T18:17:59Z

Hi, bumping this PR - also a user of gotestsum, and interested in a file attribute for tests. @asfaltboy are you planning on coming back to this?

lucas-anchorage · 2025-06-05T13:06:09Z

Hi! Bumping this as well as it's a feature that'd be highly useful for me. thanks!

match filename.go:1 instead as per discussion & comments: gotestyourself#470 (review)

asfaltboy · 2025-06-07T11:27:41Z

Thank you for the PR! If you pull in the latest changes from main that should fix the test failures.

In general I think the approach of parsing the file:line from the output is ok, but I don't think it's safe to expect the "Error Trace" prefix.

We may want to do this parsing on the event before we join all the lines. I think that will make it easier to find the relevant file:line.

There's also this proposal to add an OutputType field: golang/go#62728 (comment)

I think that field would make it much easier to find the appropriate file:line in the output. We could parse it directly from only the events that have an OutputType: error.

Pardon for the delay, and thanks a lot for the review @dnephin , you were right to block the merging I messed up with the 'Error Trace' assumption. I just realise that a big benefit of that testify feature is that it returns a full path to the file, which they obtain from the stack trace.

I think I addressed the comments as requested, and I'd rather merge it as-is as if we can, and come back to this to iterate for other features like getting the full path (requires understanding where the package lives in the path, and where our file is in that package).

lucas-anchorage · 2025-06-16T14:09:38Z

@dnephin ping. Is this something we could expect merged in the next couple of weeks?

dnephin

Thanks for reworking this!

A couple more test cases and I think this is good.

dnephin · 2025-06-19T03:39:20Z

cmd/tool/parser/parser.go

+// ParseFailure parses the output of the `go test` for a test failure  and
+// returns the file and line number of the failed test case.
+func ParseFailure(output string) (file string, line int, err error) {
+	re, err := regexp.Compile(`^\s*([_\w]+\.go):(\d+):`)


I think there are many other valid filename characters, space, dash, dots, and maybe even unicode? I assume other characters like + could also be valid?

I'm not sure if regex is the best tool here. https://pkg.go.dev/strings#Cut or strings.Index maybe?

dnephin · 2025-06-19T03:41:06Z

cmd/tool/parser/parser_test.go

+	"gotest.tools/v3/assert"
+)
+
+func TestParseFailure_Ok(t *testing.T) {


Some negative tests would also be great. Ex:

What if the file:line appears in the middle of other text?
What if there is no match?

asfaltboy force-pushed the feat/add-file-line-to-xml branch 3 times, most recently from 4aa0ab1 to ab484fc Compare February 27, 2025 12:51

asfaltboy marked this pull request as ready for review February 27, 2025 13:29

dnephin reviewed Mar 13, 2025

View reviewed changes

asfaltboy and others added 4 commits June 7, 2025 11:09

Add the file/line to the Junit XML output

cf4c6aa

fixup! Add the file/line to the Junit XML output

937e59c

fixup! Add the file/line to the Junit XML output

aed3904

do not rely on 'Error Trace'

c3a3dad

match filename.go:1 instead as per discussion & comments: gotestyourself#470 (review)

asfaltboy force-pushed the feat/add-file-line-to-xml branch from ab484fc to c3a3dad Compare June 7, 2025 11:20

dnephin reviewed Jun 19, 2025

View reviewed changes

Add the file/line to the output of Junit XML #470

Are you sure you want to change the base?

Add the file/line to the output of Junit XML #470

Uh oh!

Conversation

asfaltboy commented Feb 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

asfaltboy commented Mar 3, 2025

Uh oh!

dnephin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

dnephin Mar 13, 2025

Choose a reason for hiding this comment

Uh oh!

asfaltboy Jun 7, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

samloop commented May 6, 2025

Uh oh!

lucas-anchorage commented Jun 5, 2025

Uh oh!

asfaltboy commented Jun 7, 2025

Uh oh!

lucas-anchorage commented Jun 16, 2025

Uh oh!

dnephin left a comment

Choose a reason for hiding this comment

Uh oh!

dnephin Jun 19, 2025

Choose a reason for hiding this comment

Uh oh!

dnephin Jun 19, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

asfaltboy commented Feb 27, 2025 •

edited

Loading