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

cmd/go: migrate tests from vcs-test.golang.org to a test-local server #27494

Closed
jasonkeene opened this issue Sep 4, 2018 · 28 comments
Closed
Assignees
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

Comments

@jasonkeene
Copy link
Contributor

There are a few tests that rely on vcs-test.golang.org. Do we need to have these tests talk to a vcs server on the internet or can the tests just start this server themselves? If the server can be ran locally I would think that would be better than having them rely on the availability of vcs-test.golang.org.

Follow up from: #27127 (comment)

cc/ @FiloSottile

@andybons andybons changed the title Tests rely on vcs-test.golang.org cmd/go: tests rely on vcs-test.golang.org Sep 4, 2018
@andybons andybons added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Sep 4, 2018
@andybons andybons added this to the Unplanned milestone Sep 4, 2018
@andybons
Copy link
Member

andybons commented Sep 4, 2018

@bcmills @rsc

@bcmills bcmills added the Testing An issue that has been verified to require only test changes, not just a test failure. label Jan 17, 2019
@bcmills bcmills changed the title cmd/go: tests rely on vcs-test.golang.org cmd/go: migrate tests from vcs-test.golang.org to a test-local server Jul 15, 2022
@bcmills bcmills modified the milestones: Unplanned, Backlog Jul 15, 2022
@bcmills bcmills self-assigned this Jul 15, 2022
@gopherbot
Copy link

Change https://go.dev/cl/418103 mentions this issue: cmd/go: convert TestAccidentalGitCheckout to a script test

@gopherbot
Copy link

Change https://go.dev/cl/417658 mentions this issue: cmd/go: remove testdata/modlegacy directory

@gopherbot
Copy link

Change https://go.dev/cl/419874 mentions this issue: cmd/go: avoid passing testing.T to isCaseSensitive and goVersion

@gopherbot
Copy link

Change https://go.dev/cl/419875 mentions this issue: cmd/go: extract the TestScript engine into a standalone package

@gopherbot
Copy link

Change https://go.dev/cl/420054 mentions this issue: cmd/go/internal/script: define GOOS, GOARCH, and compiler conditions using suffixes

@gopherbot
Copy link

Change https://go.dev/cl/421455 mentions this issue: cmd/go: redirect vcs-test.golang.org URLs to a test-local server

@gopherbot
Copy link

Change https://go.dev/cl/421454 mentions this issue: cmd/go: replace the '[exec:git]' condition with a '[git]' condition

gopherbot pushed a commit that referenced this issue Aug 16, 2022
As far as I can determine, this is the only existing non-script test
that relies on vcs-test.golang.org. I am attempting to eliminate the
vcs-test server, and to keep configuration straightforward I would
like to only set up its replacement for script tests.

For #27494.

Change-Id: Ib978228b9c3f403af5f1c445e08942cbe7915b62
Reviewed-on: https://go-review.googlesource.com/c/go/+/418103
Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
gopherbot pushed a commit that referenced this issue Aug 16, 2022
This test was apparently mistakenly removed without a replacement in
CL 213223, but its testdata was left in the tree. This change removes
the orphaned testdata subdirectory, and restores the test that
previously used that data as a self-contained script.

For #27494.

Change-Id: Ice81895a44c558aaab198b8ef7ec046d92f5d58f
Reviewed-on: https://go-review.googlesource.com/c/go/+/417658
Auto-Submit: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopherbot pushed a commit that referenced this issue Aug 18, 2022
The previous implementation of isCaseSensitive called t.Fatalf in the
wrong place, causing tests after the first to proceed past an error
determining case-sensitivity. That could lead to confusing errors.

(Moreover, I would like to try to disentangle the script engine from
testing.T so that I can also use it to generate serving contents in
the replacement for vcs-test.golang.org.)

The implementation of goVersion called ts.fatalf, which is probably
fine but prevents the script environment from being computed outside
of a test, as we might want to do for debugging and other scripting.

For #27494.

Change-Id: Ibfee0704523fdcd6174b544ff84267216435025b
Reviewed-on: https://go-review.googlesource.com/c/go/+/419874
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Auto-Submit: Bryan Mills <bcmills@google.com>
@bcmills bcmills modified the milestones: Backlog, Go1.20 Aug 22, 2022
@gopherbot
Copy link

Change https://go.dev/cl/426854 mentions this issue: cmd/go/internal/script: remove special-case escaping logic for $WORK

@gopherbot
Copy link

Change https://go.dev/cl/427094 mentions this issue: cmd/go: replace the 'addcrlf' script command with a more general 'replace' command

@gopherbot
Copy link

Change https://go.dev/cl/427254 mentions this issue: cmd/go: reroute vcs-test.golang.org HTTPS requests to the test-local server

romaindoumenc pushed a commit to TroutSoftware/go that referenced this issue Nov 3, 2022
This makes it more obvious that the condition is testing for something
beyond just the existence of a 'git' executable.

For golang#27494.

Change-Id: I7608b6c84f9f373292687b3a2066b0ded7deb6e1
Reviewed-on: https://go-review.googlesource.com/c/go/+/421454
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Russ Cox <rsc@golang.org>
Auto-Submit: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
romaindoumenc pushed a commit to TroutSoftware/go that referenced this issue Nov 3, 2022
…using suffixes

This replaces a large set of individual GOOS and GOARCH conditions
with a smaller set of more verbose conditions. On balance, the more
uniform structure and more concise documentation seem worth the
verbosity.

For golang#27494.

Change-Id: I73fdf9e7180a92cb1baf5d4631aeecb26ce15181
Reviewed-on: https://go-review.googlesource.com/c/go/+/420054
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Run-TryBot: Bryan Mills <bcmills@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
romaindoumenc pushed a commit to TroutSoftware/go that referenced this issue Nov 3, 2022
…lace' command

This allows the "reuse_git" test to avoid depending on exact JSON
blobs, which will be important when the URLs start referring to
test-local vcweb servers.

For golang#27494.

Change-Id: I22fde5110b3267b8fb9fb9c59fabc3b8a8b492c8
Reviewed-on: https://go-review.googlesource.com/c/go/+/427094
Reviewed-by: Russ Cox <rsc@golang.org>
Auto-Submit: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Bryan Mills <bcmills@google.com>
romaindoumenc pushed a commit to TroutSoftware/go that referenced this issue Nov 3, 2022
This fixes a regression introduced in CL 419875
that causes features that require cgo to be tested
on the nocgo builders.

For golang#27494.

Change-Id: Iee61225c98c1275810256ab002a698fc4b42c053
Reviewed-on: https://go-review.googlesource.com/c/go/+/445235
Auto-Submit: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
romaindoumenc pushed a commit to TroutSoftware/go that referenced this issue Nov 3, 2022
The new server reconstructs the vcs-test repos on the fly using
scripts that run the actual version-control binaries.

This allows those repos to be code-reviewed using our normal tools —
and, crucially, allows contributors to add new vcs-test contents
as part of a contributed CL.

It also prevents failures due to network errors reaching
vcs-test.golang.org (such as when developing offline), and allows us
to iterate on the repo contents without dealing with annoying and
unpredictable GCS caching behavior.

We can't quite turn down vcs-test.golang.org yet — this server doesn't
yet handle "go-import" metadata (and related authentication behaviors),
and doesn't serve Subversion repos.

But we're getting much closer!

For golang#27494.

Change-Id: I233fc718617aed287b0f7248bd8cfe1e5cebe96b
Reviewed-on: https://go-review.googlesource.com/c/go/+/421455
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Bryan Mills <bcmills@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
romaindoumenc pushed a commit to TroutSoftware/go that referenced this issue Nov 3, 2022
…server

After this CL, the only test requests that should still reach
vcs-test.golang.org are for Subversion repos, which are not yet handled.

The interceptor implementation should also allow us to redirect other
servers (such as gopkg.in) fairly easily in a followup change if
desired.

For golang#27494.

Change-Id: I8cb85f3a7edbbf0492662ff5cfa779fb9b407136
Reviewed-on: https://go-review.googlesource.com/c/go/+/427254
Auto-Submit: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Russ Cox <rsc@golang.org>
romaindoumenc pushed a commit to TroutSoftware/go that referenced this issue Nov 3, 2022
With this change applied, 'go test cmd/go/...' passes
even with the IP routing for vcs-test.golang.org disabled
using 'ip route add blackhole $VCSTEST_IP/32'.

Fixes golang#27494.

Change-Id: I45651d2429c7fea7bbf693b2f129e260e1c59891
Reviewed-on: https://go-review.googlesource.com/c/go/+/427914
Run-TryBot: Bryan Mills <bcmills@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
Reviewed-by: Russ Cox <rsc@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
romaindoumenc pushed a commit to TroutSoftware/go that referenced this issue Nov 3, 2022
The scripts added in CL 421455 passed on the TryBots, but failed on a
subset of the builders that have older 'git' binaries installed.

Notably, the older versions of git do not support:
- 'git branch -m' before the current branch has a commit
- 'init.defaultBranch' in the '.gitconfig' file, and
- 'git branch -c'.

We address those by, respectively:
- waiting to run 'git branch -m' until after the first commit
- always running 'git branch -m' explicitly to set the branch name, and
- using 'git checkout' instead of 'git branch -c' to set branch parents.

Updates golang#27494.

Change-Id: I42f012f5add8f31e41d077d752d8268aacbce8a4
Reviewed-on: https://go-review.googlesource.com/c/go/+/445355
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
romaindoumenc pushed a commit to TroutSoftware/go that referenced this issue Nov 3, 2022
The scripts added in CL 421455 passed on the TryBots, but failed on
the "-stretch" builders, which supply Mercurial 4.0
(released 2016-11-01).

Debian 9 “Stretch” has been at end-of-life since June 30, 2022, but
until we can turn down the outdated builders (golang#56414) we should keep
them passing tests.

For golang#27494.
Updates golang#56414.

Change-Id: I9df0ed452dfbfaeb1b4c0d869d02dd9ed21b3ff6
Reviewed-on: https://go-review.googlesource.com/c/go/+/445356
Auto-Submit: Bryan Mills <bcmills@google.com>
Reviewed-by: Russ Cox <rsc@golang.org>
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
romaindoumenc pushed a commit to TroutSoftware/go that referenced this issue Nov 3, 2022
…ocesses

The Cancel and WaitDelay fields recently added to exec.Cmd are
intended to support exactly the sort of cancellation behavior that we
need for script tests. Use them, and simplify the cmd/go tests
accordingly.

The more robust implementation may also help to diagose recurring test
hangs (golang#50187).

For golang#50187.
Updates golang#27494.

Change-Id: I7817fca0dd9a18e18984a252d3116f6a5275a401
Reviewed-on: https://go-review.googlesource.com/c/go/+/445357
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
romaindoumenc pushed a commit to TroutSoftware/go that referenced this issue Nov 3, 2022
This should reduce spurious VCS failures on slow builders, like the
one observed in
https://build.golang.org/log/e773fe404b2009d67fa34f048e023f0a86663a13

Updates golang#27494.

Change-Id: Ibb094c8ddf79e0ab481c00fcf501dc955b0da787
Reviewed-on: https://go-review.googlesource.com/c/go/+/447116
Auto-Submit: Bryan Mills <bcmills@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
romaindoumenc pushed a commit to TroutSoftware/go that referenced this issue Nov 3, 2022
…tions

This is an oversight from https://go-review.googlesource.com/c/go/+/419875,
where script commands were refactored and factored out to a new package.

For golang#27494.

Change-Id: Ie606cab39f60859ee1da5165dcc94c8470c94325
Reviewed-on: https://go-review.googlesource.com/c/go/+/447575
Run-TryBot: Quim Muntal <quimmuntal@gmail.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Bryan Mills <bcmills@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
@gopherbot
Copy link

Change https://go.dev/cl/449517 mentions this issue: cmd/go/internal/vcweb: fix a data race in the overview handler

gopherbot pushed a commit that referenced this issue Nov 11, 2022
I forgot to lock the scriptResult in the overview handler, and
apparently a cmd/go test is incidentally fetching the overview page at
some point during test execution, triggering the race.

This race was caught almost immediately by the new
linux-amd64-longtest-race builder (see
https://build.golang.org/log/85ab78169a6382a73b1a26c89e64138b387da217).

Updates #27494.

Change-Id: I06ee8d54dba400800284401428ba4a59809983b2
Reviewed-on: https://go-review.googlesource.com/c/go/+/449517
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Bryan Mills <bcmills@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
@gopherbot
Copy link

Change https://go.dev/cl/454503 mentions this issue: cmd/go: skip TestScript/mod_replace_gopkgin

gopherbot pushed a commit that referenced this issue Dec 2, 2022
(Until it can be made hermetic.)

The gopkg.in service has had a lot of flakiness lately. Go users in
general are isolated from that flakiness by the Go module mirror
(proxy.golang.org), but this test intentionally bypasses the module
mirror because the mirror itself uses cmd/go to download the module.

In the long term, we can redirect the gopkg.in URL to the local
(in-process) vcweb server added for #27494.

In the meantime, let's skip the test to reduce the impact of upstream
outages.

For #54503.

Change-Id: Icf3de7ca416db548e53864a71776fe22b444fcea
Reviewed-on: https://go-review.googlesource.com/c/go/+/454503
Run-TryBot: Bryan Mills <bcmills@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
@gopherbot
Copy link

Change https://go.dev/cl/454839 mentions this issue: [release-branch.go1.19] cmd/go: skip TestScript/mod_replace_gopkgin

@gopherbot
Copy link

Change https://go.dev/cl/454840 mentions this issue: [release-branch.go1.18] cmd/go: skip TestScript/mod_replace_gopkgin

gopherbot pushed a commit that referenced this issue Dec 9, 2022
(Until it can be made hermetic.)

The gopkg.in service has had a lot of flakiness lately. Go users in
general are isolated from that flakiness by the Go module mirror
(proxy.golang.org), but this test intentionally bypasses the module
mirror because the mirror itself uses cmd/go to download the module.

In the long term, we can redirect the gopkg.in URL to the local
(in-process) vcweb server added for #27494.

In the meantime, let's skip the test to reduce the impact of upstream
outages.

Fixes #57058.
Updates #54503.

Change-Id: Icf3de7ca416db548e53864a71776fe22b444fcea
Reviewed-on: https://go-review.googlesource.com/c/go/+/454503
Run-TryBot: Bryan Mills <bcmills@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
(cherry picked from commit c5f5cb659adda026d01b7fa9bd39b2ad3b58c5bf)
Reviewed-on: https://go-review.googlesource.com/c/go/+/454839
Reviewed-by: Michael Pratt <mpratt@google.com>
gopherbot pushed a commit that referenced this issue Dec 9, 2022
(Until it can be made hermetic.)

The gopkg.in service has had a lot of flakiness lately. Go users in
general are isolated from that flakiness by the Go module mirror
(proxy.golang.org), but this test intentionally bypasses the module
mirror because the mirror itself uses cmd/go to download the module.

In the long term, we can redirect the gopkg.in URL to the local
(in-process) vcweb server added for #27494.

In the meantime, let's skip the test to reduce the impact of upstream
outages.

Fixes #57057.
Updates #54503.

Change-Id: Icf3de7ca416db548e53864a71776fe22b444fcea
Reviewed-on: https://go-review.googlesource.com/c/go/+/454503
Run-TryBot: Bryan Mills <bcmills@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
(cherry picked from commit c5f5cb659adda026d01b7fa9bd39b2ad3b58c5bf)
Reviewed-on: https://go-review.googlesource.com/c/go/+/454840
@gopherbot
Copy link

Change https://go.dev/cl/473276 mentions this issue: cmd/go: declare net hosts in script tests

gopherbot pushed a commit that referenced this issue Apr 24, 2023
Although we aren't precise about enforcing the hosts just yet,
we can eventually use the declared hostnames to selectively skip
tests (for example, if an external service has an outage while
a Go release is being tested).

Also relax the constraint to [short] in tests that require only
vcs-test.golang.org, which has redirected to an in-process server
since around CL 427914.

Also enforce that tests that use the network actually use the [net]
constraint, by setting TESTGONETWORK=panic in the test environment
until the condition is evaluated.

For #52545.
For #54503.
Updates #27494.

Change-Id: I13be6b42a9beee97657eb45424882e787ac164c3
Reviewed-on: https://go-review.googlesource.com/c/go/+/473276
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Bypass: Bryan Mills <bcmills@google.com>
Reviewed-by: Russ Cox <rsc@golang.org>
Auto-Submit: Bryan Mills <bcmills@google.com>
@gopherbot
Copy link

Change https://go.dev/cl/510895 mentions this issue: vcs-test/vcweb: extend time on the deletion deathclock

@gopherbot
Copy link

Change https://go.dev/cl/510896 mentions this issue: vcs-test: delete

gopherbot pushed a commit to golang/build that referenced this issue Jul 19, 2023
The deathclock test started to fail, woohoo! Unfortunately because of
early thaw, it's failing at tip before we can actually turn down this
service since Go 1.19 is still supported for a bit longer.

File a tracking issue and extend time on the deletion deathclock. :)

For golang/go#27494.
For golang/go#61425.

Change-Id: Ibdc55163b2248ddfa2e467ffaf47128e158f6a02
Reviewed-on: https://go-review.googlesource.com/c/build/+/510895
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org>
gopherbot pushed a commit to golang/build that referenced this issue Sep 12, 2023
Thank you vcs-test for the service.

For golang/go#27494.
Fixes golang/go#61425.

Change-Id: I02fa14017dfeae28ebaf02d2a68ec467f14b2466
Co-authored-by: Heschi Kreinick <heschi@google.com>
Reviewed-on: https://go-review.googlesource.com/c/build/+/510896
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
Development

No branches or pull requests

4 participants