Skip to content

x/build/cmd/coordinator: some subrepos want to run tests in both module and GOPATH modes #30233

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
Closed
dmitshur opened this issue Feb 14, 2019 · 3 comments
Closed

x/build/cmd/coordinator: some subrepos want to run tests in both module and GOPATH modes #30233

dmitshur opened this issue Feb 14, 2019 · 3 comments
Labels
Builders x/build issues (builders, bots, dashboards) FeatureRequest Issues asking for a new feature that does not need a proposal. FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. Testing An issue that has been verified to require only test changes, not just a test failure.
Milestone
Unreleased

Comments

@dmitshur
Copy link
Contributor

coordinator already had support for running tests (at least trybots) in module mode, it's used for oauth2 and build subrepos:

https://github.com/golang/build/blob/34b995b40b0d31f2760a2f57b9ce74e17925702c/cmd/coordinator/coordinator.go#L2506-L2507

Some subrepos, such as x/tools, may want to soon start running their tests in both GO111MODULE=on and GO111MODULE=off modes, in order to make sure there are no regressions in either mode.

We should add support for this to the coordinator.

/cc @bradfitz @matloob
@dmitshur dmitshur added Builders x/build issues (builders, bots, dashboards) NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. FeatureRequest Issues asking for a new feature that does not need a proposal. labels Feb 14, 2019
@gopherbot gopherbot added this to the Unreleased milestone Feb 14, 2019
@dmitshur
Copy link
Contributor Author

I see two ways this can be implemented:

  • a more lightweight way (i.e., without having to run all tests twice) by adding a new GOPATH mode builder for just one GOOS/GOARCH, similar to how we do that for running tests on previous supported versions of Go, e.g., see here
  • a more heavyweight way, run tests in both modes on each GOOS/GOARCH combination

@dmitshur dmitshur mentioned this issue Jun 10, 2019
Closed
@toothrot toothrot mentioned this issue Jun 18, 2019
Closed
@dmitshur dmitshur self-assigned this Sep 5, 2019
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/194277 mentions this issue: cmd/coordinator: detect current build mode in subrepo tests

@dmitshur dmitshur added the Testing An issue that has been verified to require only test changes, not just a test failure. label Sep 10, 2019
@dmitshur dmitshur mentioned this issue Sep 11, 2019
Closed
gopherbot pushed a commit to golang/build that referenced this issue Sep 11, 2019
@dmitshur
cmd/coordinator: detect current build mode in subrepo tests
43f0c4d 
It will be necessary to know whether the current environment
and configuration has resulted in module mode or GOPATH mode
being selected. Do this dynamically by invoking 'go env GOMOD',
since the outcome depends on the version of Go and its default
GO111MODULE behavior, as well as whether a go.mod file is added
to the subrepo, and what environment overrides were applied by
coordinator via the BuildConfig.ModulesEnv method.

This requires the repository being tested to be available on disk,
so move the step of fetching it to happen earlier.

For now, this change only detects and logs the build mode.
Future changes will use this information further.

Updates golang/go#34190
Updates golang/go#32528
Updates golang/go#30233

Change-Id: I641becaaae5b6cfad22961d8864a877241e61cd3
Reviewed-on: https://go-review.googlesource.com/c/build/+/194277
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Andrew Bonventre <andybons@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Alexander Rakoczy <alex@golang.org>
codebien pushed a commit to codebien/build that referenced this issue Nov 13, 2019
@dmitshur @codebien
cmd/coordinator: detect current build mode in subrepo tests
8f66d6c 
It will be necessary to know whether the current environment
and configuration has resulted in module mode or GOPATH mode
being selected. Do this dynamically by invoking 'go env GOMOD',
since the outcome depends on the version of Go and its default
GO111MODULE behavior, as well as whether a go.mod file is added
to the subrepo, and what environment overrides were applied by
coordinator via the BuildConfig.ModulesEnv method.

This requires the repository being tested to be available on disk,
so move the step of fetching it to happen earlier.

For now, this change only detects and logs the build mode.
Future changes will use this information further.

Updates golang/go#34190
Updates golang/go#32528
Updates golang/go#30233

Change-Id: I641becaaae5b6cfad22961d8864a877241e61cd3
Reviewed-on: https://go-review.googlesource.com/c/build/+/194277
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Andrew Bonventre <andybons@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Alexander Rakoczy <alex@golang.org>
@dmitshur dmitshur mentioned this issue Oct 8, 2021
Closed
@dmitshur
Copy link
Contributor Author

dmitshur commented Oct 8, 2021

It turned out we had sufficient coverage so none of the subrepos needed this,
and we got by without ever implementing it. So the decision here is "won't fix".

Closing in favor of #48875.

@dmitshur dmitshur closed this as completed Oct 8, 2021
@golang golang locked and limited conversation to collaborators Jun 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
Builders x/build issues (builders, bots, dashboards) FeatureRequest Issues asking for a new feature that does not need a proposal. FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. Testing An issue that has been verified to require only test changes, not just a test failure.
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
2 participants
@dmitshur @gopherbot