-
Notifications
You must be signed in to change notification settings - Fork 39
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
Dump junit.xml #650
Dump junit.xml #650
Conversation
9830519
to
25f1485
Compare
.github/workflows/ci.yaml
Outdated
- name: Test | ||
run: go test ./... | ||
run: go test ./... 2>&1 | tee >(go-junit-report -set-exit-code > junit-xml/${{matrix.os}}.xml) |
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.
In the server we use https://gotest.tools/gotestsum
as a test runner, which has a junit output mode. That seems better than parsing go test
output?
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.
Ah, thanks! I'll switch.
2e0b246
to
908c80b
Compare
This reverts commit 4eb8bcf061ba3c6c5838e7bc20aee38950ed0e36.
This reverts commit a4d271b.
.github/workflows/ci.yaml
Outdated
- name: Install gotestsum | ||
run: go install gotest.tools/gotestsum@latest | ||
|
||
- run: mkdir junit-xml |
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 think we should name all steps (or just make the step below be a two-command thing with run: |
and this be the first command). Not all of our CIs name all steps, but in workflows that do we should probably keep doing it.
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.
Sure! Done. (I was thinking based on sdk-python
that you liked not naming trivial shell steps but this seems good.)
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 would support changing that CI to start naming everything too (that things are not named is a product of history I think)
This PR switches the tests in CI to dump junit XML files. To do so, it switches to use https://github.com/gotestyourself/gotestsum as the test runner. (This is what the server repo uses also). An example of a build with test failures is here: https://github.com/temporalio/cli/actions/runs/10631569269/job/29472736222
As in temporalio/sdk-python#617, the motivation for this is that, given appropriate tools, it will make it easier for us to study patterns of flakiness, test timings etc, across matrix builds within a run, and across runs.