Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Parser gojson incompatibility with go test's -coverprofile flag #134

Closed
forsaken628 opened this issue Jun 11, 2022 · 6 comments
Closed

Parser gojson incompatibility with go test's -coverprofile flag #134

forsaken628 opened this issue Jun 11, 2022 · 6 comments
Assignees
@jstemmer
Labels
bug

Comments

@forsaken628
Copy link

forsaken628 commented Jun 11, 2022 •
edited
Loading

When handle
yy.txt

Generated report contain incorrect content like this

<testcase name="TestSrv_ServiceMap" classname="example.com/foo/pkg/labels" time="0.000">
	<error message="No test result found"></error>
</testcase>

command:
go test -json -race -coverpkg=./... -covermode=atomic -coverprofile=out/cover.out.tmp ./...

go version:
go 1.18
@jstemmer jstemmer added the bug label Jun 11, 2022
@jstemmer
Copy link
Owner

Thanks. It looks like the testcase gets attributed to the wrong package, possibly because of the order in which it appears in the output.

I don't think I've seen this before, it would be helpful if I could reproduce this output somehow. Can you share a bit more about the environment and the tests that you ran, e.g. what version of Go did you use and did you run any tests in parallel? Did you use any additional commandline options when running go test?

@medzin
Copy link

medzin commented Jul 15, 2022

I can confirm that I have a similar issue regardless of whether I'm using a test coverage reporting or not. After disabling the test report in JSON, the tests run stably.

@jstemmer
Copy link
Owner

I managed to reproduce by testing multiple packages at a time and enabling the race detector (-race). For non-JSON output, test events are always grouped by package. When enabling JSON output this appears to not always be the case, test events from different packages may be mixed occasionally which triggers the bug you observed. Fortunately, each event in the JSON output contains the package name so this is fixable but it will require some refactoring.

Until this has been fixed you can work around this problem by disabling the race detector, or not use the JSON test output.

@jstemmer jstemmer self-assigned this Jul 20, 2022
jstemmer added a commit that referenced this issue Aug 15, 2022
@jstemmer
parser/gotest: Add support for processing events for different packages
562bb27 
The reportBuilder assumed we were always processing events for a single
package at a time. This is not true however when running `go test -json
-race` for example. In order to properly support processing events from
different packages we now have packageBuilders per package name.

Fixes #134
jstemmer added a commit that referenced this issue Aug 15, 2022
@jstemmer
Add testdata for go test -json -race tests.
7917c9d 
Refs #134
jstemmer added a commit that referenced this issue Aug 15, 2022
@jstemmer
Add testdata for go test -json -race tests.
4d0ed8b 
Refs #134
@karelbilek
Copy link

I have been hit by the same bug now. Yes this is fixable by looking at the JSON tag.

I can have a look at this and make a PR

@karelbilek
Copy link

Ah, seems like we were using stable 2.0.0 version, which doesn't have this fixed, that's the reason. Nevermind!

@altego371
Copy link

FYI
If you are using a version without this fix you can use the next trick:
go test -json [...] | jq -c '{k: .Package, v: .}' | sort | jq -c '.v' | go-junit-report [...] where [...] are yours flags
It sorts go test output before passing it to the go-junit-report.
jq https://stedolan.github.io/ is a cli-tool for json.

bendrucker added a commit to observeinc/terraform-provider-observe that referenced this issue Jun 22, 2023
@bendrucker
fix: fix tests
bf95245 
For a brief period, we had tests failing in Docker due to git ownership errors but the JUnit parser swallowed this error. This is a batch change fixing all test errors, since individual atomic changes would not be verified by Jenkins.

Changes are summarized below:

* make: set git safe directory for all docker + go targets
* make(testacc): exit 1 when `go test` fails
* chore: fix name conflict in layered settings test
  * Two layered settings, one targeting a datastream and one targeting a dataset, had the same name.
* fix: omit unimplemented accelerationDisabled field
  * Introduced in https://gerrit.observeinc.com/c/terraform-provider-observe/+/33177
  * Until this is implemented by the provider, it must be omitted. The server will not accept a null value for this field.
* app: update OpenWeather
  * v0.1.0 is no longer available and causing tests to fail
* fix: marshal floats as numbers
  * Previously, the API would fail on a JSON number representing a float, and required numeric strings. Now, that behavior is reversed, and submitting a string fails.
* monitor: remove `reminder_frequency` removal from test
  * The API currently does not support un-setting a reminder frequency. Remove that.
  * See https://gerrit.observeinc.com/c/terraform-provider-observe/+/32961
* poller: disable GCP tests
  * These now validate the private key among other OAuth inputs and the tests are now broken
  * They can be restored when validation can be disabled
* make: fix sweeper pattern (escape)
  * Previously \d became d without escaping, resulting in a broken regex
* deps: update github.com/jstemmer/go-junit-report/v2
  * We seem to be hitting a bug where test results are reported in the wrong package (client), resulting in errors because the test result is empty.
  * jstemmer/go-junit-report#134
  * This is fixed in the main branch but hasn't had a tagged release:
  * jstemmer/go-junit-report@9357c18
  * This updates the dependency version to the latest commit
    * go get github.com/jstemmer/go-junit-report/v2@7933520

Change-Id: I51d31c1073b58ebbe28fef698cdc20f693f6e582
@burgesQ burgesQ mentioned this issue Jul 4, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jstemmer jstemmer

Labels
bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@jstemmer @karelbilek @medzin @forsaken628 @altego371