test: put the Docker acceptance bootstrap behind an acc build tag - #171
Open
fdcastel wants to merge 1 commit into
Open
test: put the Docker acceptance bootstrap behind an acc build tag#171fdcastel wants to merge 1 commit into
acc build tag#171fdcastel wants to merge 1 commit into
Conversation
testcontainers-go/modules/compose is imported only by provider_test.go's TestMain, but nothing gated that import, so the Docker/containerd/ compose-go tree was compiled into the test binary for every `go test` run -- including plain unit runs that never start a container. Measured on this package with `go list -deps -test`: before 138 docker/containerd/compose-go packages after 0 (untagged); the 138 move behind -tags=acc The bootstrap moves to provider_docker_acc_test.go (//go:build acc) with a stub in provider_docker_stub_test.go (//go:build !acc) that explains how to enable it. TestMain stays untagged, so target dispatch and unit runs are unchanged. Taskfile: test:acc gains -tags=acc. test:acc:hardware deliberately does not -- it talks to an existing controller, so leaving the tag off keeps the Docker tree out of that build too. vet gains a second tagged pass so the acc file cannot rot silently. One thing this does NOT do, since alexklibisz#159 raised it specifically: it does not move the dependency out of go.mod's main require block. `go mod tidy` evaluates all build configurations, so a tagged test import is still a direct requirement -- I verified this rather than assuming it. Demoting it for real would need the bootstrap in a separate module, which seems disproportionate. The compile-time cost is the part that was actually hurting, and that part is gone.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follows up on the
testcontainerspoint from #159, which you said seemed worth fixing.What was happening
testcontainers-go/modules/composeis imported only byTestMaininprovider_test.go, but nothing gated that import — so the Docker/containerd/compose-go tree got compiled into the test binary on everygo testrun, including plain unit runs that never start a container.Measured with
go list -deps -test ./internal/provider/:-tags=acc)What changed
runDockerTests,waitForAPI) moves toprovider_docker_acc_test.gobehind//go:build acc.provider_docker_stub_test.go(//go:build !acc) provides arunDockerTeststhat prints how to enable the tag.TestMainstays untagged, so target dispatch and unit runs behave exactly as before.Taskfile.yml:test:accgains-tags=acc.test:acc:hardwaredeliberately does not — it talks to an existing controller, so leaving the tag off keeps the Docker tree out of that build too.vetgains a second tagged pass, otherwise the acc file rots silently until someone runstest:acc.CI needs no change —
ci-local.yamlgoes throughtask test:unit/task test:acc.One thing this does not do
Since #159 raised go.mod specifically, I want to be straight about this: it does not move the dependency out of the main
requireblock. I tried it and checked rather than assuming —go mod tidyevaluates all build configurations, so a tagged test import is still a direct requirement and tidy leaves it exactly where it is.Demoting it for real would mean putting the bootstrap in a separate Go module, which feels disproportionate for what it buys. The part that was actually costing something — compiling 138 packages to run unit tests — is gone, and consumers importing the provider package never had compose in their build anyway, since it was always test-only.
Happy to go the separate-module route instead if you'd rather have the go.mod line gone; just didn't want to make that call unilaterally.
Side effect: it should make Dependabot's compose bumps (#169 currently) cheaper to reason about, since the dependency is now visibly test-only in the source tree.
Written with Claude Code, reviewed by me.