diff options
| author | Bryan C. Mills <bcmills@google.com> | 2023-08-22 15:23:22 -0400 |
|---|---|---|
| committer | Gopher Robot <gobot@golang.org> | 2023-08-23 16:58:07 +0000 |
| commit | 76f4137fe5bb3d97d1ffa28cb490e71a96d5dcb9 (patch) | |
| tree | c9df4b3814fad5dec21b74dfc3b4c52fb35ce5b1 /internal | |
| parent | edcdbe543d8f35f3b18c96a63080f96b3912a7d9 (diff) | |
| download | go-x-pkgsite-76f4137fe5bb3d97d1ffa28cb490e71a96d5dcb9.tar.xz | |
all: remove arbitrary hard-coded timeouts in tests
If a test times out, that implies that it got stuck on something.
By default, the Go testing package dumps goroutines when its own
timeout is passed, which prints a goroutine dump, helping to reveal
what was stuck.
Adding an arbitrary timeout on top of the testing package's own
timeout is, in my experience, almost always counterproductive.
If the arbitrary timeout catches a real hang, it causes the test to
fail instead of dumping goroutines, making it much harder to see what
was stuck. On the other hand, if the timeouts are set aggressively
enough to make the test fail early, they are often too aggressive
for CI testing, causing flakes that then have to be triaged on an
ongoing basis.
On balance, the value of saving a minute or two for developers who
have introduced a hang is not worth the cost of suppressing debugging
information and causing flakes that have to be triaged.
Fixes #61556.
For #59347.
Change-Id: I0263d0d9b18283470f100e5a0155818b87b5312f
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/521837
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
kokoro-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
Diffstat (limited to 'internal')
25 files changed, 75 insertions, 150 deletions
diff --git a/internal/database/database_test.go b/internal/database/database_test.go index e48caf99..43b02d74 100644 --- a/internal/database/database_test.go +++ b/internal/database/database_test.go @@ -24,8 +24,6 @@ import ( "golang.org/x/pkgsite/internal/derrors" ) -const testTimeout = 5 * time.Second - const testDBName = "discovery_postgres_test" var testDB *DB @@ -136,8 +134,7 @@ func TestBulkInsert(t *testing.T) { }, } { t.Run(test.name, func(t *testing.T) { - ctx, cancel := context.WithTimeout(context.Background(), testTimeout) - defer cancel() + ctx := context.Background() createQuery := fmt.Sprintf(`CREATE TABLE %s ( colA TEXT NOT NULL, @@ -198,8 +195,7 @@ func TestBulkInsert(t *testing.T) { } func TestLargeBulkInsert(t *testing.T) { - ctx, cancel := context.WithTimeout(context.Background(), testTimeout*3) - defer cancel() + ctx := context.Background() if _, err := testDB.Exec(ctx, `CREATE TEMPORARY TABLE test_large_bulk (i BIGINT);`); err != nil { t.Fatal(err) } @@ -235,8 +231,7 @@ func TestLargeBulkInsert(t *testing.T) { } func TestBulkUpsert(t *testing.T) { - ctx, cancel := context.WithTimeout(context.Background(), testTimeout*3) - defer cancel() + ctx := context.Background() if _, err := testDB.Exec(ctx, `CREATE TEMPORARY TABLE test_replace (C1 int PRIMARY KEY, C2 int);`); err != nil { t.Fatal(err) } @@ -308,8 +303,7 @@ func TestBuildBulkUpdateQuery(t *testing.T) { } func TestBulkUpdate(t *testing.T) { - ctx, cancel := context.WithTimeout(context.Background(), testTimeout) - defer cancel() + ctx := context.Background() defer func(old int) { maxBulkUpdateArrayLen = old }(maxBulkUpdateArrayLen) maxBulkUpdateArrayLen = 5 @@ -372,8 +366,7 @@ func TestTransactSerializable(t *testing.T) { // Test that serializable transactions retry until success. // This test was taken from the example at https://www.postgresql.org/docs/11/transaction-iso.html, // section 13.2.3. - ctx, cancel := context.WithTimeout(context.Background(), testTimeout*2) - defer cancel() + ctx := context.Background() // Once in while, the test doesn't work. Repeat to de-flake. var msg string @@ -469,8 +462,7 @@ func testTransactSerializable(ctx context.Context, t *testing.T) string { func TestCopyDoesNotUpsert(t *testing.T) { // This test verifies that copying rows into a table will not overwrite existing rows. - ctx, cancel := context.WithTimeout(context.Background(), testTimeout) - defer cancel() + ctx := context.Background() conn, err := testDB.db.Conn(ctx) if err != nil { t.Fatal(err) diff --git a/internal/fetch/fetch_test.go b/internal/fetch/fetch_test.go index 562dde2d..5bc06018 100644 --- a/internal/fetch/fetch_test.go +++ b/internal/fetch/fetch_test.go @@ -32,8 +32,6 @@ import ( "golang.org/x/pkgsite/internal/testing/sample" ) -var testTimeout = 30 * time.Second - var ( templateFS = template.TrustedFSFromTrustedSource(template.TrustedSourceFromConstant("../../static")) testModules []*proxytest.Module @@ -182,8 +180,7 @@ func validateDocumentationHTML(t *testing.T, got *internal.Module, want map[stri } func TestFetchModule_Errors(t *testing.T) { - ctx, cancel := context.WithTimeout(context.Background(), testTimeout) - defer cancel() + ctx := context.Background() for _, test := range []struct { name string mod *testModule diff --git a/internal/fetch/readme_test.go b/internal/fetch/readme_test.go index 26529e0d..ba697a39 100644 --- a/internal/fetch/readme_test.go +++ b/internal/fetch/readme_test.go @@ -21,8 +21,7 @@ func TestExtractReadmes(t *testing.T) { testenv.MustHaveExecPath(t, "git") defer stdlib.WithTestData()() - ctx, cancel := context.WithTimeout(context.Background(), testTimeout) - defer cancel() + ctx := context.Background() sortReadmes := func(readmes []*internal.Readme) { sort.Slice(readmes, func(i, j int) bool { diff --git a/internal/frontend/versions/versions_test.go b/internal/frontend/versions/versions_test.go index e779357a..35cd0226 100644 --- a/internal/frontend/versions/versions_test.go +++ b/internal/frontend/versions/versions_test.go @@ -7,7 +7,6 @@ package versions import ( "context" "testing" - "time" "github.com/google/go-cmp/cmp" "golang.org/x/pkgsite/internal" @@ -19,8 +18,6 @@ import ( "golang.org/x/pkgsite/internal/vuln" ) -const testTimeout = 5 * time.Second - var testDB *postgres.DB func TestMain(m *testing.M) { @@ -226,8 +223,7 @@ func TestFetchPackageVersionsDetails(t *testing.T) { }, } { t.Run(tc.name, func(t *testing.T) { - ctx, cancel := context.WithTimeout(context.Background(), testTimeout*2) - defer cancel() + ctx := context.Background() defer postgres.ResetTestDB(testDB, t) for _, v := range tc.modules { diff --git a/internal/postgres/delete_test.go b/internal/postgres/delete_test.go index 1ff54d65..6652a240 100644 --- a/internal/postgres/delete_test.go +++ b/internal/postgres/delete_test.go @@ -23,8 +23,7 @@ func TestDeleteModule(t *testing.T) { t.Parallel() testDB, release := acquire(t) defer release() - ctx, cancel := context.WithTimeout(context.Background(), testTimeout) - defer cancel() + ctx := context.Background() v := sample.DefaultModule() diff --git a/internal/postgres/details_test.go b/internal/postgres/details_test.go index 10d83ab3..8d9e3e52 100644 --- a/internal/postgres/details_test.go +++ b/internal/postgres/details_test.go @@ -96,8 +96,7 @@ func TestGetNestedModules_Excluded(t *testing.T) { t.Parallel() testDB, release := acquire(t) defer release() - ctx, cancel := context.WithTimeout(context.Background(), testTimeout) - defer cancel() + ctx := context.Background() test := struct { name string @@ -143,8 +142,7 @@ func TestPostgres_GetModuleInfo(t *testing.T) { t.Parallel() testDB, release := acquire(t) defer release() - ctx, cancel := context.WithTimeout(context.Background(), testTimeout) - defer cancel() + ctx := context.Background() testCases := []struct { name, path, version string diff --git a/internal/postgres/excluded_test.go b/internal/postgres/excluded_test.go index 96186c53..02dd2b08 100644 --- a/internal/postgres/excluded_test.go +++ b/internal/postgres/excluded_test.go @@ -13,8 +13,7 @@ func TestIsExcluded(t *testing.T) { t.Parallel() testDB, release := acquire(t) defer release() - ctx, cancel := context.WithTimeout(context.Background(), testTimeout) - defer cancel() + ctx := context.Background() if err := testDB.InsertExcludedPrefix(ctx, "bad", "someone", "because"); err != nil { t.Fatal(err) diff --git a/internal/postgres/insert_module_test.go b/internal/postgres/insert_module_test.go index ee3c25a2..aa4fed40 100644 --- a/internal/postgres/insert_module_test.go +++ b/internal/postgres/insert_module_test.go @@ -195,8 +195,7 @@ func TestUpsertModule(t *testing.T) { t.Parallel() testDB, release := acquire(t) defer release() - ctx, cancel := context.WithTimeout(context.Background(), testTimeout) - defer cancel() + ctx := context.Background() m := sample.Module("upsert.org", "v1.2.3", "dir/p") @@ -285,8 +284,7 @@ func TestInsertModuleNewCoverage(t *testing.T) { t.Parallel() testDB, release := acquire(t) defer release() - ctx, cancel := context.WithTimeout(context.Background(), testTimeout) - defer cancel() + ctx := context.Background() m := sample.DefaultModule() newCoverage := licensecheck.Coverage{ @@ -447,8 +445,7 @@ func TestPostgres_NewerAlternative(t *testing.T) { // alternative version. testDB, release := acquire(t) defer release() - ctx, cancel := context.WithTimeout(context.Background(), testTimeout) - defer cancel() + ctx := context.Background() const ( altVersion = "v1.2.0" @@ -488,8 +485,7 @@ func TestMakeValidUnicode(t *testing.T) { t.Parallel() testDB, release := acquire(t) defer release() - ctx, cancel := context.WithTimeout(context.Background(), testTimeout) - defer cancel() + ctx := context.Background() db := testDB.Underlying() @@ -529,8 +525,7 @@ func TestLock(t *testing.T) { // that wants the lock eventually gets it. testDB, release := acquire(t) defer release() - ctx, cancel := context.WithTimeout(context.Background(), testTimeout) - defer cancel() + ctx := context.Background() db := testDB.Underlying() diff --git a/internal/postgres/licenses_test.go b/internal/postgres/licenses_test.go index 5b43469d..c0b02f9b 100644 --- a/internal/postgres/licenses_test.go +++ b/internal/postgres/licenses_test.go @@ -42,8 +42,7 @@ func TestGetLicenses(t *testing.T) { testDB, release := acquire(t) defer release() - ctx, cancel := context.WithTimeout(context.Background(), testTimeout*5) - defer cancel() + ctx := context.Background() MustInsertModule(ctx, t, testDB, testModule) MustInsertModule(ctx, t, testDB, stdlibModule) for _, test := range []struct { @@ -131,8 +130,7 @@ func TestGetModuleLicenses(t *testing.T) { testDB, release := acquire(t) defer release() - ctx, cancel := context.WithTimeout(context.Background(), testTimeout) - defer cancel() + ctx := context.Background() testModule.Licenses = nil for _, p := range testModule.Packages() { @@ -169,8 +167,7 @@ func TestGetLicensesBypass(t *testing.T) { t.Parallel() testDB, release := acquire(t) defer release() - ctx, cancel := context.WithTimeout(context.Background(), testTimeout) - defer cancel() + ctx := context.Background() bypassDB := NewBypassingLicenseCheck(testDB.db) diff --git a/internal/postgres/postgres_test.go b/internal/postgres/postgres_test.go index 5550b7d8..8f1e497f 100644 --- a/internal/postgres/postgres_test.go +++ b/internal/postgres/postgres_test.go @@ -14,8 +14,6 @@ import ( "golang.org/x/pkgsite/internal/derrors" ) -const testTimeout = 5 * time.Second - var acquire func(*testing.T) (*DB, func()) func TestMain(m *testing.M) { diff --git a/internal/postgres/requeue_test.go b/internal/postgres/requeue_test.go index 6198b65e..f106504c 100644 --- a/internal/postgres/requeue_test.go +++ b/internal/postgres/requeue_test.go @@ -28,8 +28,7 @@ func TestGetNextModulesToFetchAndUpdateModuleVersionStatesForReprocessing(t *tes t.Parallel() testDB, release := acquire(t) defer release() - ctx, cancel := context.WithTimeout(context.Background(), testTimeout) - defer cancel() + ctx := context.Background() type testData struct { modulePath, version string @@ -213,8 +212,7 @@ func TestGetNextModulesToFetchOnlyPicksUpStatus0AndStatusGreaterThan500(t *testi t.Parallel() testDB, release := acquire(t) defer release() - ctx, cancel := context.WithTimeout(context.Background(), testTimeout) - defer cancel() + ctx := context.Background() statuses := []int{ http.StatusOK, diff --git a/internal/postgres/search_test.go b/internal/postgres/search_test.go index 747d074e..40455a0d 100644 --- a/internal/postgres/search_test.go +++ b/internal/postgres/search_test.go @@ -744,8 +744,7 @@ func TestSearchPenalties(t *testing.T) { t.Parallel() testDB, release := acquire(t) defer release() - ctx, cancel := context.WithTimeout(context.Background(), testTimeout) - defer cancel() + ctx := context.Background() // All these modules will have the same text ranking for the search term "foo", // but different scores due to penalties. @@ -797,8 +796,7 @@ func TestExcludedFromSearch(t *testing.T) { t.Parallel() testDB, release := acquire(t) defer release() - ctx, cancel := context.WithTimeout(context.Background(), testTimeout) - defer cancel() + ctx := context.Background() // Insert a module with two packages. const domain = "exclude.com" @@ -827,8 +825,7 @@ func TestSearchBypass(t *testing.T) { t.Parallel() testDB, release := acquire(t) defer release() - ctx, cancel := context.WithTimeout(context.Background(), testTimeout) - defer cancel() + ctx := context.Background() bypassDB := NewBypassingLicenseCheck(testDB.db) m := nonRedistributableModule() @@ -936,8 +933,7 @@ func TestUpsertSearchDocument(t *testing.T) { t.Parallel() testDB, release := acquire(t) defer release() - ctx, cancel := context.WithTimeout(context.Background(), testTimeout) - defer cancel() + ctx := context.Background() var packagePath = sample.ModulePath + "/A" @@ -1005,8 +1001,7 @@ func TestUpsertSearchDocumentVersionHasGoMod(t *testing.T) { t.Parallel() testDB, release := acquire(t) defer release() - ctx, cancel := context.WithTimeout(context.Background(), testTimeout) - defer cancel() + ctx := context.Background() for _, hasGoMod := range []bool{true, false} { m := sample.Module(fmt.Sprintf("foo.com/%t", hasGoMod), "v1.2.3", "bar") @@ -1229,8 +1224,7 @@ func TestGetPackagesForSearchDocumentUpsert(t *testing.T) { t.Parallel() testDB, release := acquire(t) defer release() - ctx, cancel := context.WithTimeout(context.Background(), testTimeout) - defer cancel() + ctx := context.Background() moduleA := sample.Module("mod.com", "v1.2.3", "A", "A/notinternal", "A/internal", "A/internal/B") diff --git a/internal/postgres/stdlib_test.go b/internal/postgres/stdlib_test.go index ce5f2198..c15a4941 100644 --- a/internal/postgres/stdlib_test.go +++ b/internal/postgres/stdlib_test.go @@ -17,8 +17,7 @@ func TestGetStdlibPaths(t *testing.T) { t.Parallel() testDB, release := acquire(t) defer release() - ctx, cancel := context.WithTimeout(context.Background(), testTimeout) - defer cancel() + ctx := context.Background() // Insert two versions of some stdlib packages. for _, data := range []struct { diff --git a/internal/postgres/symbol_test.go b/internal/postgres/symbol_test.go index cc0766f1..07a289a3 100644 --- a/internal/postgres/symbol_test.go +++ b/internal/postgres/symbol_test.go @@ -24,8 +24,7 @@ func TestInsertSymbolNamesAndHistory(t *testing.T) { t.Parallel() testDB, release := acquire(t) defer release() - ctx, cancel := context.WithTimeout(context.Background(), testTimeout) - defer cancel() + ctx := context.Background() mod := sample.DefaultModule() if len(mod.Packages()) != 1 { @@ -87,8 +86,7 @@ func TestInsertSymbolNamesAndHistory(t *testing.T) { func TestInsertSymbolHistory_Basic(t *testing.T) { testDB, release := acquire(t) defer release() - ctx, cancel := context.WithTimeout(context.Background(), testTimeout) - defer cancel() + ctx := context.Background() mod := sample.DefaultModule() if len(mod.Packages()) != 1 { @@ -115,8 +113,7 @@ func TestInsertSymbolHistory_Basic(t *testing.T) { func TestInsertSymbolHistory_MultiVersions(t *testing.T) { testDB, release := acquire(t) defer release() - ctx, cancel := context.WithTimeout(context.Background(), testTimeout) - defer cancel() + ctx := context.Background() typ := internal.Symbol{ SymbolMeta: internal.SymbolMeta{ @@ -215,8 +212,7 @@ func TestInsertSymbolHistory_MultiVersions(t *testing.T) { func TestInsertSymbolHistory_MultiGOOS(t *testing.T) { testDB, release := acquire(t) defer release() - ctx, cancel := context.WithTimeout(context.Background(), testTimeout) - defer cancel() + ctx := context.Background() typ := internal.Symbol{ SymbolMeta: internal.SymbolMeta{ diff --git a/internal/postgres/unit_test.go b/internal/postgres/unit_test.go index 9c7813c0..5f83af4c 100644 --- a/internal/postgres/unit_test.go +++ b/internal/postgres/unit_test.go @@ -24,8 +24,7 @@ import ( func TestGetUnitMeta(t *testing.T) { t.Parallel() - ctx, cancel := context.WithTimeout(context.Background(), testTimeout*2) - defer cancel() + ctx := context.Background() testGetUnitMeta(t, ctx) } @@ -207,8 +206,7 @@ func TestGetUnitMetaBypass(t *testing.T) { t.Parallel() testDB, release := acquire(t) defer release() - ctx, cancel := context.WithTimeout(context.Background(), testTimeout) - defer cancel() + ctx := context.Background() bypassDB := NewBypassingLicenseCheck(testDB.db) @@ -580,8 +578,7 @@ func TestGetUnit(t *testing.T) { t.Parallel() testDB, release := acquire(t) defer release() - ctx, cancel := context.WithTimeout(context.Background(), testTimeout) - defer cancel() + ctx := context.Background() InsertSampleDirectoryTree(ctx, t, testDB) // Add a module that has READMEs in a directory and a package. @@ -773,8 +770,7 @@ func TestGetUnit_SubdirectoriesShowNonRedistPackages(t *testing.T) { t.Parallel() testDB, release := acquire(t) defer release() - ctx, cancel := context.WithTimeout(context.Background(), testTimeout) - defer cancel() + ctx := context.Background() m := sample.DefaultModule() m.IsRedistributable = false @@ -786,8 +782,7 @@ func TestGetUnitFieldSet(t *testing.T) { t.Parallel() testDB, release := acquire(t) defer release() - ctx, cancel := context.WithTimeout(context.Background(), testTimeout) - defer cancel() + ctx := context.Background() readme := &internal.Readme{ Filepath: "a.com/m/dir/p/README.md", @@ -878,8 +873,7 @@ func TestGetUnitBuildContext(t *testing.T) { t.Parallel() testDB, release := acquire(t) defer release() - ctx, cancel := context.WithTimeout(context.Background(), testTimeout) - defer cancel() + ctx := context.Background() // Add a module that has documentation for two Go build contexts. m := sample.Module("a.com/twodoc", "v1.2.3", "p") @@ -971,8 +965,7 @@ func TestGetUnitBypass(t *testing.T) { t.Parallel() testDB, release := acquire(t) defer release() - ctx, cancel := context.WithTimeout(context.Background(), testTimeout) - defer cancel() + ctx := context.Background() bypassDB := NewBypassingLicenseCheck(testDB.db) m := nonRedistributableModule() diff --git a/internal/postgres/version_map_test.go b/internal/postgres/version_map_test.go index 01fff1b4..3cbc325c 100644 --- a/internal/postgres/version_map_test.go +++ b/internal/postgres/version_map_test.go @@ -20,8 +20,7 @@ func TestReadAndWriteVersionMap(t *testing.T) { t.Parallel() testDB, release := acquire(t) defer release() - ctx, cancel := context.WithTimeout(context.Background(), testTimeout) - defer cancel() + ctx := context.Background() m := sample.Module("golang.org/x/tools", sample.VersionString, "go/packages") MustInsertModule(ctx, t, testDB, m) @@ -48,8 +47,7 @@ func TestUpsertVersionMap(t *testing.T) { t.Parallel() testDB, release := acquire(t) defer release() - ctx, cancel := context.WithTimeout(context.Background(), testTimeout) - defer cancel() + ctx := context.Background() upsertAndVerifyVersionMap := func(vm *internal.VersionMap) { err := testDB.UpsertVersionMap(ctx, vm) @@ -84,8 +82,7 @@ func TestGetVersionMapsWithNon2xxStatus(t *testing.T) { t.Parallel() testDB, release := acquire(t) defer release() - ctx, cancel := context.WithTimeout(context.Background(), testTimeout) - defer cancel() + ctx := context.Background() tests := []struct { path string diff --git a/internal/postgres/version_test.go b/internal/postgres/version_test.go index 7b09998b..66c0453e 100644 --- a/internal/postgres/version_test.go +++ b/internal/postgres/version_test.go @@ -58,8 +58,7 @@ func TestGetVersions(t *testing.T) { testDB, release := acquire(t) defer release() - ctx, cancel := context.WithTimeout(context.Background(), testTimeout) - defer cancel() + ctx := context.Background() for _, m := range testModules { goMod := "module " + m.ModulePath @@ -255,8 +254,7 @@ func TestGetLatestInfo(t *testing.T) { t.Parallel() testDB, release := acquire(t) defer release() - ctx, cancel := context.WithTimeout(context.Background(), testTimeout) - defer cancel() + ctx := context.Background() for _, m := range []*internal.Module{ sample.Module("a.com/M", "v99.0.0+incompatible", "all", "most"), diff --git a/internal/postgres/versionstate_test.go b/internal/postgres/versionstate_test.go index df15fa5f..8d2100cd 100644 --- a/internal/postgres/versionstate_test.go +++ b/internal/postgres/versionstate_test.go @@ -83,8 +83,7 @@ func TestModuleVersionState(t *testing.T) { t.Parallel() testDB, release := acquire(t) defer release() - ctx, cancel := context.WithTimeout(context.Background(), testTimeout) - defer cancel() + ctx := context.Background() // verify that latest index timestamp works initialTime, err := testDB.LatestIndexTimestamp(ctx) diff --git a/internal/proxy/client_test.go b/internal/proxy/client_test.go index 902f93ad..54c3a5ba 100644 --- a/internal/proxy/client_test.go +++ b/internal/proxy/client_test.go @@ -21,8 +21,6 @@ import ( "golang.org/x/pkgsite/internal/version" ) -const testTimeout = 5 * time.Second - var testModule = &proxytest.Module{ ModulePath: sample.ModulePath, Version: sample.VersionString, @@ -66,8 +64,7 @@ var uncachedModule = &proxytest.Module{ } func TestGetLatestInfo(t *testing.T) { - ctx, cancel := context.WithTimeout(context.Background(), testTimeout) - defer cancel() + ctx := context.Background() testModules := []*proxytest.Module{ { @@ -95,8 +92,7 @@ func TestGetLatestInfo(t *testing.T) { } func TestListVersions(t *testing.T) { - ctx, cancel := context.WithTimeout(context.Background(), testTimeout) - defer cancel() + ctx := context.Background() testModules := []*proxytest.Module{ { @@ -129,8 +125,7 @@ func TestListVersions(t *testing.T) { } func TestInfo(t *testing.T) { - ctx, cancel := context.WithTimeout(context.Background(), testTimeout) - defer cancel() + ctx := context.Background() client, teardownProxy := proxytest.SetupTestClient(t, []*proxytest.Module{testModule, uncachedModule}) defer teardownProxy() @@ -163,8 +158,7 @@ func TestInfo(t *testing.T) { } func TestInfo_Errors(t *testing.T) { - ctx, cancel := context.WithTimeout(context.Background(), testTimeout) - defer cancel() + ctx := context.Background() proxyServer := proxytest.NewServer(nil) proxyServer.AddRoute( @@ -196,8 +190,7 @@ func TestInfo_Errors(t *testing.T) { } func TestMod(t *testing.T) { - ctx, cancel := context.WithTimeout(context.Background(), testTimeout) - defer cancel() + ctx := context.Background() client, teardownProxy := proxytest.SetupTestClient(t, []*proxytest.Module{testModule}) defer teardownProxy() @@ -214,8 +207,7 @@ func TestMod(t *testing.T) { } func TestGetZip(t *testing.T) { - ctx, cancel := context.WithTimeout(context.Background(), testTimeout) - defer cancel() + ctx := context.Background() client, teardownProxy := proxytest.SetupTestClient(t, []*proxytest.Module{testModule}) defer teardownProxy() @@ -253,8 +245,7 @@ func TestGetZip(t *testing.T) { } func TestZipNonExist(t *testing.T) { - ctx, cancel := context.WithTimeout(context.Background(), testTimeout) - defer cancel() + ctx := context.Background() client, teardownProxy := proxytest.SetupTestClient(t, nil) defer teardownProxy() diff --git a/internal/source/source_test.go b/internal/source/source_test.go index 99e390fe..451e39f9 100644 --- a/internal/source/source_test.go +++ b/internal/source/source_test.go @@ -22,8 +22,7 @@ import ( ) var ( - testTimeout = 2 * time.Second - record = flag.Bool("record", false, "record interactions with other systems, for replay") + record = flag.Bool("record", false, "record interactions with other systems, for replay") ) func TestModuleInfo(t *testing.T) { @@ -551,7 +550,6 @@ func TestModuleInfoDynamic(t *testing.T) { client := &Client{ httpClient: &http.Client{ Transport: testTransport(testWeb), - Timeout: testTimeout, }, } // The version doesn't figure into the interesting work and we test versions to commits @@ -731,6 +729,12 @@ func TestRemoveVersionSuffix(t *testing.T) { func TestAdjustVersionedModuleDirectory(t *testing.T) { ctx := context.Background() + + testTimeout := 72 * time.Hour // arbitrary + if deadline, ok := t.Deadline(); ok { + testTimeout = time.Until(deadline) * 9 / 10 // Allow 10% for error reporting and cleanup. + } + client := NewClient(testTimeout) client.httpClient.Transport = testTransport(map[string]string{ // Repo "branch" follows the "major branch" convention: versions 2 and higher diff --git a/internal/worker/excluded_test.go b/internal/worker/excluded_test.go index d7e27491..3f7b6ae5 100644 --- a/internal/worker/excluded_test.go +++ b/internal/worker/excluded_test.go @@ -13,8 +13,7 @@ import ( ) func TestExcluded(t *testing.T) { - ctx, cancel := context.WithTimeout(context.Background(), testTimeout) - defer cancel() + ctx := context.Background() defer postgres.ResetTestDB(testDB, t) // This test is intending to test PopulateExcluded not just IsExcluded. diff --git a/internal/worker/fetch_test.go b/internal/worker/fetch_test.go index a2d90619..d0bdf98a 100644 --- a/internal/worker/fetch_test.go +++ b/internal/worker/fetch_test.go @@ -42,8 +42,7 @@ var ( ) func TestFetchAndUpdateState(t *testing.T) { - ctx, cancel := context.WithTimeout(context.Background(), testTimeout) - defer cancel() + ctx := context.Background() defer stdlib.WithTestData()() diff --git a/internal/worker/fetcherror_test.go b/internal/worker/fetcherror_test.go index 9e3ca877..89367835 100644 --- a/internal/worker/fetcherror_test.go +++ b/internal/worker/fetcherror_test.go @@ -31,8 +31,7 @@ import ( // Check that when the proxy says it does not have module@version, // we delete it from the database. func TestFetchAndUpdateState_NotFound(t *testing.T) { - ctx, cancel := context.WithTimeout(context.Background(), testTimeout) - defer cancel() + ctx := context.Background() defer postgres.ResetTestDB(testDB, t) proxyClient, teardown := proxytest.SetupTestClient(t, []*proxytest.Module{ @@ -92,8 +91,7 @@ func TestFetchAndUpdateState_NotFound(t *testing.T) { func TestFetchAndUpdateState_Excluded(t *testing.T) { // Check that an excluded module is not processed, and is marked excluded in module_version_states. - ctx, cancel := context.WithTimeout(context.Background(), testTimeout) - defer cancel() + ctx := context.Background() defer postgres.ResetTestDB(testDB, t) @@ -108,8 +106,7 @@ func TestFetchAndUpdateState_Excluded(t *testing.T) { } func TestFetchAndUpdateState_BadRequestedVersion(t *testing.T) { - ctx, cancel := context.WithTimeout(context.Background(), testTimeout) - defer cancel() + ctx := context.Background() defer postgres.ResetTestDB(testDB, t) var ( @@ -123,8 +120,7 @@ func TestFetchAndUpdateState_BadRequestedVersion(t *testing.T) { func TestFetchAndUpdateState_Incomplete(t *testing.T) { // Check that we store the special "incomplete" status in module_version_states. - ctx, cancel := context.WithTimeout(context.Background(), testTimeout) - defer cancel() + ctx := context.Background() defer postgres.ResetTestDB(testDB, t) @@ -150,8 +146,7 @@ func TestFetchAndUpdateState_Incomplete(t *testing.T) { func TestFetchAndUpdateState_Mismatch(t *testing.T) { // Check that an excluded module is not processed, and is marked excluded in module_version_states. - ctx, cancel := context.WithTimeout(context.Background(), testTimeout) - defer cancel() + ctx := context.Background() defer postgres.ResetTestDB(testDB, t) @@ -175,8 +170,7 @@ func TestFetchAndUpdateState_Mismatch(t *testing.T) { func TestFetchAndUpdateState_DeleteOlder(t *testing.T) { // Check that fetching an alternative module deletes all older versions of that // module from search_documents (but not versions). - ctx, cancel := context.WithTimeout(context.Background(), testTimeout) - defer cancel() + ctx := context.Background() defer postgres.ResetTestDB(testDB, t) @@ -219,8 +213,7 @@ func TestFetchAndUpdateState_DeleteOlder(t *testing.T) { } func TestFetchAndUpdateState_SkipIncompletePackage(t *testing.T) { - ctx, cancel := context.WithTimeout(context.Background(), testTimeout) - defer cancel() + ctx := context.Background() defer postgres.ResetTestDB(testDB, t) badModule := map[string]string{ "foo/foo.go": "// Package foo\npackage foo\n\nconst Foo = 42", @@ -263,8 +256,7 @@ func TestFetchAndUpdateState_Timeout(t *testing.T) { // Check that when the proxy says fetch timed out, we return a 5xx error so // that we automatically try to fetch it again later. func TestFetchAndUpdateState_ProxyTimedOut(t *testing.T) { - ctx, cancel := context.WithTimeout(context.Background(), testTimeout) - defer cancel() + ctx := context.Background() defer postgres.ResetTestDB(testDB, t) proxyServer := proxytest.NewServer(nil) @@ -288,8 +280,7 @@ func TestFetchAndUpdateState_ProxyTimedOut(t *testing.T) { // This makes it viable for us to show documentation for packages that // would otherwise exceed HTML size limit and not get shown at all. func TestTrimLargeCode(t *testing.T) { - ctx, cancel := context.WithTimeout(context.Background(), testTimeout*3) - defer cancel() + ctx := context.Background() defer postgres.ResetTestDB(testDB, t) trimmedModule := map[string]string{ "foo/foo.go": "// Package foo\npackage foo\n\nconst Foo = 42", diff --git a/internal/worker/refetch_test.go b/internal/worker/refetch_test.go index 12a56de9..fd97b622 100644 --- a/internal/worker/refetch_test.go +++ b/internal/worker/refetch_test.go @@ -23,12 +23,12 @@ import ( ) func TestReFetch(t *testing.T) { + ctx := context.Background() + // This test checks that re-fetching a version will cause its data to be // overwritten. This is achieved by fetching against two different versions // of the (fake) proxy, though in reality the most likely cause of changes to // a version is updates to our data model or fetch logic. - ctx, cancel := context.WithTimeout(context.Background(), testTimeout) - defer cancel() defer postgres.ResetTestDB(testDB, t) var ( diff --git a/internal/worker/server_test.go b/internal/worker/server_test.go index 96a01b3d..d4a055a0 100644 --- a/internal/worker/server_test.go +++ b/internal/worker/server_test.go @@ -30,8 +30,6 @@ import ( "golang.org/x/pkgsite/internal/testing/sample" ) -const testTimeout = 120 * time.Second - var ( testDB *postgres.DB httpClient *http.Client @@ -59,8 +57,7 @@ func setupTraceDebugging(t *testing.T) { } func TestWorker(t *testing.T) { - ctx, cancel := context.WithTimeout(context.Background(), testTimeout) - defer cancel() + ctx := context.Background() setupTraceDebugging(t) |
