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.

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

[chore] Use gotestsum wrapper for go tests #31163

Merged
merged 5 commits into from
Feb 13, 2024

Conversation

mx-psi
Copy link
Member

@mx-psi mx-psi commented Feb 9, 2024

Description:

Use https://github.com/gotestyourself/gotestsum for running tests.

This PR only adds the wrapper. A future PR could add --rerun-fails, but since that is, I presume, more controversial, I want to start just with the basics, which already improves readability.

Link to tracking Issue: Relates to #30880

@mx-psi mx-psi marked this pull request as ready for review February 9, 2024 12:36
@mx-psi mx-psi requested review from a team and atoulme February 9, 2024 12:36
@mx-psi
Copy link
Member Author

mx-psi commented Feb 9, 2024

cc @open-telemetry/collector-contrib-approvers does this seem like a good idea to you? The main benefit from this PR is that the output format is nicer and it makes tests logs a bit more manageable. We could also introduce rerun on failed (see https://github.com/gotestyourself/gotestsum#documentation for more details)

Copy link
Member

@andrzej-stencel andrzej-stencel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like it! The vague output of go test is ridiculous.

The only nit is that GOTEST used GOCMD, which (IIUC) could be used to set a different version of go, but I don't think we used this anyway?

@mx-psi
Copy link
Member Author

mx-psi commented Feb 13, 2024

The only nit is that GOTEST used GOCMD, which (IIUC) could be used to set a different version of go, but I don't think we used this anyway?

I can't say with certainty that someone out there is not using this, but we don't use it on CI AFAICT, and we could add it back if needed by using gotestsum's --raw-command option

@mx-psi mx-psi merged commit 591b428 into open-telemetry:main Feb 13, 2024
142 checks passed
@mx-psi mx-psi deleted the mx-psi/gotestsum branch February 13, 2024 23:45
@github-actions github-actions bot added this to the next release milestone Feb 13, 2024
TylerHelmuth pushed a commit that referenced this pull request Feb 14, 2024
**Description:** 

Re-runs failed unit tests automatically. Follow up to #31163
This re-runs the tests once if there are less than 10 total test
failures.

This should speed up development, but it comes with the risk of missing
real issues.
I think given the current situation our CI is in this is acceptable, but
I assume this PR is going to be controversial :)

One improvement would be to keep this but auto-generate Github issues
when a test fails and then passes on main's CI.

**Link to tracking Issue:** Relates to #30880 (does not speed up
individual tests but reduces the number of attempts to be made)
XinRanZhAWS pushed a commit to XinRanZhAWS/opentelemetry-collector-contrib that referenced this pull request Mar 13, 2024
**Description:** <Describe what has changed.>
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->

Use https://github.com/gotestyourself/gotestsum for running tests.

This PR only adds the wrapper. A future PR could add `--rerun-fails`,
but since that is, I presume, more controversial, I want to start just
with the basics, which already improves readability.

**Link to tracking Issue:** Relates to open-telemetry#30880
XinRanZhAWS pushed a commit to XinRanZhAWS/opentelemetry-collector-contrib that referenced this pull request Mar 13, 2024
**Description:** 

Re-runs failed unit tests automatically. Follow up to open-telemetry#31163
This re-runs the tests once if there are less than 10 total test
failures.

This should speed up development, but it comes with the risk of missing
real issues.
I think given the current situation our CI is in this is acceptable, but
I assume this PR is going to be controversial :)

One improvement would be to keep this but auto-generate Github issues
when a test fails and then passes on main's CI.

**Link to tracking Issue:** Relates to open-telemetry#30880 (does not speed up
individual tests but reduces the number of attempts to be made)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants