Skip to content

internal/testdir: move to cmd module #60059

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 May 8, 2023 · 2 comments
Closed

internal/testdir: move to cmd module #60059

dmitshur opened this issue May 8, 2023 · 2 comments
Assignees
@dmitshur
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. Testing An issue that has been verified to require only test changes, not just a test failure.
Milestone
Go1.21

Comments

@dmitshur
Copy link
Member

dmitshur commented May 8, 2023 •
edited
Loading

CL 463276 converted the GOROOT/test/run.go test runner program to something that can be invoked via go test, and used internal/testdir as the import path. It just needed to be something inside GOROOT/src and not a part of public API. It probably mostly commonly runs as part of all.bash, which was updated for the new invocation of the test.

In https://go-review.googlesource.com/c/go/+/492985/comment/c3dbe9c2_fc1fe608/ @bcmills pointed out that it might be more appropriate to adjust the import path so that the package and its test is picked up when doing go test cmd instead of go test std. That is, to make it cmd/internal/testdir.

I'll move if it there doesn't turn out to be a downside to it (other than the slightly longer import path, relevant when not using all.bash to run it, but one we're willing to accept for now; @aclements pointed out it's possible to shorten it via a helper if desired).
@dmitshur dmitshur added the NeedsFix The path to resolution is known, but the work has not been done. label May 8, 2023
@dmitshur dmitshur added this to the Go1.21 milestone May 8, 2023
@dmitshur dmitshur self-assigned this May 8, 2023
@bcmills bcmills added the Testing An issue that has been verified to require only test changes, not just a test failure. label May 8, 2023
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/493876 mentions this issue: internal/testdir: move to cmd/internal/testdir

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/493915 mentions this issue: _content/doc: update testdir import path in contribution guide

gopherbot pushed a commit to golang/website that referenced this issue May 12, 2023
@dmitshur @gopherbot
_content/doc: update testdir import path in contribution guide
cd3dc42 
It's changed slightly to be inside 'cmd'.

For golang/go#56844.
For golang/go#60059.

Change-Id: I244f0ae627978a7b59d6a56d20aebc3ff81b3179
Reviewed-on: https://go-review.googlesource.com/c/website/+/493915
Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
@dmitshur dmitshur mentioned this issue May 12, 2023
Closed
@golang golang locked and limited conversation to collaborators May 11, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@dmitshur dmitshur

Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. Testing An issue that has been verified to require only test changes, not just a test failure.
Projects
None yet
Milestone
Go1.21
Development

No branches or pull requests
3 participants
@dmitshur @bcmills @gopherbot