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.

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

cmd/dist, misc/cgo: drop host test support #59999

Closed
aclements opened this issue May 5, 2023 · 4 comments
Closed

cmd/dist, misc/cgo: drop host test support #59999

aclements opened this issue May 5, 2023 · 4 comments
Assignees
@aclements
Labels
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

@aclements
Copy link
Member

I plan to drop the “host test” mechanism in cmd/dist. Host tests are used for emulated builders that use cross-compilation. Today, this is the android-{386,amd64}-emu builders and all wasm builders. These builders run all.bash on a linux/amd64 host to build all packages and most tests for the emulated guest, and then run the resulting test binaries inside the emulated guest. A small number of test packages are “host tests”: these run on the host rather than the guest because they invoke the Go toolchain themselves (which only lives on the host) and run the resulting binaries in the guest.

However, this host test mechanism is barely used today, despite being quite complex. This complexity is also causing significant friction to implementing structured all.bash output.

Today, excluding internal/testdir, the whole host test mechanism runs a total of 10 test cases on a total of two builders (android-{386,amd64}-emu). There are clearly several tests that are incorrectly being skipped, so we could expand it to cover more test cases, but it would still apply to only two builders. Furthermore, the two other Android builders (android-{arm,arm64}-corellium) build the Go toolchain directly inside Android and also have access to a C toolchain, so they are able to get significantly better test coverage without the use of host tests. This suggests that the android-*-emu builders could do the same.

The internal/testdir package is the other use of host test mode, and the only one that runs on the other cross-compile platform (wasm), but that one on its own should be fairly easy to keep working.

Given the incredibly low value of host tests today, they are not worth their implementation complexity and the friction they cause.

/cc @golang/android @golang/wasm @dmitshur
@aclements aclements added this to the Go1.21 milestone May 5, 2023
@aclements aclements self-assigned this May 5, 2023
@bcmills bcmills added the Testing An issue that has been verified to require only test changes, not just a test failure. label May 5, 2023
@aclements
Copy link
Member Author

For the record, if we decide that we do want coverage of these tests, I believe the right path is to delete the existing host test support anyway and reimplement it using the technique in CL 492985.

@gopherbot
Copy link

Change https://go.dev/cl/492986 mentions this issue: cmd/dist: drop host test support

@gopherbot
Copy link

Change https://go.dev/cl/492985 mentions this issue: cmd/dist,internal/testdir: more cooperative host test mechanism

@dmitshur dmitshur added the NeedsFix The path to resolution is known, but the work has not been done. label May 6, 2023
@aclements aclements mentioned this issue May 9, 2023
Closed
gopherbot pushed a commit that referenced this issue May 12, 2023
@aclements
cmd/dist,internal/testdir: more cooperative host test mechanism
6890411 
On cross-compiling builder machines, we run internal/testdir on the
host, where it can access the Go toolchain to build binaries for the
guest and run them through an exec wrapper. Currently this uses dist
test's existing host test mechanism, which is quite complicated and we
are planning to eliminate (#59999).

Switch internal/testdir to use a more cooperative mechanism. With this
CL, dist still understands that it has to build and run the test using
the host GOOS/GOARCH, but rather than doing complicated manipulation
of environment variables itself, it passes the guest GOOS/GOARCH to
the test, which can easily inject it into its environment. This means
dist test can use "go test" directly, rather than having to split up
the build and run steps.

For #37486.

Change-Id: I556938c0b641960bb778b88b13f2b26256edc7c9
Reviewed-on: https://go-review.googlesource.com/c/go/+/492985
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
@gopherbot
Copy link

Change https://go.dev/cl/497077 mentions this issue: cmd/cgo/internal/testcshared: drop bespoke host test support

gopherbot pushed a commit that referenced this issue May 22, 2023
@aclements
cmd/cgo/internal/testcshared: drop bespoke host test support
96a773d 
Updates #59999.

Change-Id: If0b80713a6bb5d8c59d9dd0b219f2f47173090e1
Reviewed-on: https://go-review.googlesource.com/c/go/+/497077
Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Austin Clements <austin@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@aclements aclements

Labels
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
4 participants
@dmitshur @aclements @bcmills @gopherbot