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

Run tests/build with go work off to catch more problems #9170

Closed
codyoss opened this issue Dec 14, 2023 · 3 comments · Fixed by #9396
Closed

Run tests/build with go work off to catch more problems #9170

codyoss opened this issue Dec 14, 2023 · 3 comments · Fixed by #9396
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@codyoss
Copy link
Member

codyoss commented Dec 14, 2023

          > As a side note to @eliben @jba I don't understand why go build did not catch this... Does it have to do with the module workspace or am I missing something?

The root repo has a go.work file, which will mask such issues. This is the reason why having a go.work file committed is considered dangerous; when it's committed, it's worth at least running the tests with GOWORK=off to catch such issues.

You can try to run all tests in this repo with GOWORK=off in CI - this may uncover surprises :)

Originally posted by @eliben in #9168 (comment)

@codyoss codyoss added type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. priority: p2 Moderately-important priority. Fix may not be included in next release. labels Dec 14, 2023
@eliben
Copy link
Contributor

eliben commented Dec 14, 2023

It's not documented officially at the moment, AFAIK. The best discussion I'm aware of happens on golang/go#53502 - discussing the various use cases people have of committing go.work and how to mitigate issues.

I'll try to get some topic experts' opinions on this

@bcmills
Copy link

bcmills commented Dec 14, 2023

Testing with GOWORK=off would catch some problems, but not all of them. Notably:

Ideally, a project like google-cloud-go that is developing multiple modules together for release versioning should test in at least two configurations: one with the working directory in the module and workspace, and one with the module as seen from outside (compare golang/go#34352).

@bcmills
Copy link

bcmills commented Dec 14, 2023

Unfortunately, we don't yet have a good story for how to structure the “seen from outside” tests for pre-commit testing. (golang/go#28835 proposes one such mechanism, but it's not clear whether there is a better alternative.)

One option for post-commit testing is to create an external module, use go get to slot in the module to be tested, and then run go test $MOD_DIR/... on the original module directory, to run the tests as seen by the external module. (For pre-commit testing, you would also need to use go mod edit -replace to slot in the module, since go get can't fetch it otherwise.)

That might look something like: https://go.dev/play/p/3GuBSk1lP0p.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants