From 27f41bb15391668fa8ba18561efe364bab9b8312 Mon Sep 17 00:00:00 2001 From: Ian Lance Taylor Date: Thu, 21 Mar 2024 21:42:52 +0000 Subject: [PATCH] Revert "testing: add TB.SetGOMAXPROCS function" This reverts CL 519235. Reason for revert: Proposal is still in incoming. For #62020 Change-Id: Icccb930209f36097f5d930c01eda6b5042bdddc1 Reviewed-on: https://go-review.googlesource.com/c/go/+/573516 Reviewed-by: Than McIntosh Auto-Submit: Ian Lance Taylor LUCI-TryBot-Result: Go LUCI Reviewed-by: Ian Lance Taylor --- api/next/62020.txt | 4 - doc/next/6-stdlib/99-minor/testing/62020.md | 1 - src/testing/testing.go | 49 +---------- src/testing/testing_test.go | 97 --------------------- 4 files changed, 2 insertions(+), 149 deletions(-) delete mode 100644 api/next/62020.txt delete mode 100644 doc/next/6-stdlib/99-minor/testing/62020.md diff --git a/api/next/62020.txt b/api/next/62020.txt deleted file mode 100644 index 3820d88816..0000000000 --- a/api/next/62020.txt +++ /dev/null @@ -1,4 +0,0 @@ -pkg testing, type TB interface, SetGOMAXPROCS(int) #62020 -pkg testing, method (*T) SetGOMAXPROCS(int) #62020 -pkg testing, method (*B) SetGOMAXPROCS(int) #62020 -pkg testing, method (*F) SetGOMAXPROCS(int) #62020 diff --git a/doc/next/6-stdlib/99-minor/testing/62020.md b/doc/next/6-stdlib/99-minor/testing/62020.md deleted file mode 100644 index 3a73a1d8d6..0000000000 --- a/doc/next/6-stdlib/99-minor/testing/62020.md +++ /dev/null @@ -1 +0,0 @@ -The [`SetGOMAXPROCS`](/pkg/testing#T.SetGOMAXPROCS) method changes GOMAXPROCS for the duration of a single test. diff --git a/src/testing/testing.go b/src/testing/testing.go index a5441f24d5..5c06aea5f8 100644 --- a/src/testing/testing.go +++ b/src/testing/testing.go @@ -890,7 +890,6 @@ type TB interface { Logf(format string, args ...any) Name() string Setenv(key, value string) - SetGOMAXPROCS(n int) Skip(args ...any) SkipNow() Skipf(format string, args ...any) @@ -917,9 +916,8 @@ var _ TB = (*B)(nil) // may be called simultaneously from multiple goroutines. type T struct { common - isEnvSet bool - isGOMAXPROCSSet bool - context *testContext // For running tests and subtests. + isEnvSet bool + context *testContext // For running tests and subtests. } func (c *common) private() {} @@ -1308,19 +1306,6 @@ func (c *common) Setenv(key, value string) { } } -// SetGOMAXPROCS calls runtime.GOMAXPROCS(n) and uses Cleanup to -// restore the value of GOMAXPROCS after the test. -// -// Because GOMAXPROCS affects the whole process, it cannot be used -// in parallel tests or tests with parallel ancestors. -func (c *common) SetGOMAXPROCS(n int) { - c.checkFuzzFn("SetGOMAXPROCS") - prev := runtime.GOMAXPROCS(n) - c.Cleanup(func() { - runtime.GOMAXPROCS(prev) - }) -} - // panicHanding controls the panic handling used by runCleanup. type panicHandling int @@ -1461,9 +1446,6 @@ func (t *T) Parallel() { if t.isEnvSet { panic("testing: t.Parallel called after t.Setenv; cannot set environment variables in parallel tests") } - if t.isGOMAXPROCSSet { - panic("testing: t.Parallel called after t.SetGOMAXPROCS; cannot set GOMAXPROCS in parallel tests") - } t.isParallel = true if t.parent.barrier == nil { // T.Parallel has no effect when fuzzing. @@ -1545,33 +1527,6 @@ func (t *T) Setenv(key, value string) { t.common.Setenv(key, value) } -// SetGOMAXPROCS calls runtime.GOMAXPROCS(n) and uses Cleanup to -// restore the value of GOMAXPROCS after the test. -// -// Because GOMAXPROCS affects the whole process, it cannot be used -// in parallel tests or tests with parallel ancestors. -func (t *T) SetGOMAXPROCS(n int) { - // Non-parallel subtests that have parallel ancestors may still - // run in parallel with other tests: they are only non-parallel - // with respect to the other subtests of the same parent. - // Since SetGOMAXPROCS affects the whole process, we need to disallow it - // if the current test or any parent is parallel. - isParallel := false - for c := &t.common; c != nil; c = c.parent { - if c.isParallel { - isParallel = true - break - } - } - if isParallel { - panic("testing: t.SetGOMAXPROCS called after t.Parallel; cannot set GOMAXPROCS in parallel tests") - } - - t.isGOMAXPROCSSet = true - - t.common.SetGOMAXPROCS(n) -} - // InternalTest is an internal type but exported because it is cross-package; // it is part of the implementation of the "go test" command. type InternalTest struct { diff --git a/src/testing/testing_test.go b/src/testing/testing_test.go index 28b5809eea..d3822dfd57 100644 --- a/src/testing/testing_test.go +++ b/src/testing/testing_test.go @@ -13,7 +13,6 @@ import ( "os/exec" "path/filepath" "regexp" - "runtime" "slices" "strings" "sync" @@ -259,102 +258,6 @@ func TestSetenvWithParallelGrandParentBeforeSetenv(t *testing.T) { }) } -func TestSetGOMAXPROCS(t *testing.T) { - if runtime.GOARCH == "wasm" { - t.Skip("not supported on wasm yet") - } - tests := []struct { - name string - newP int - }{ - { - name: "overriding value", - newP: 1, - }, - } - - for _, test := range tests { - p := runtime.GOMAXPROCS(0) - t.Run(test.name, func(t *testing.T) { - t.SetGOMAXPROCS(test.newP + 1) - if runtime.GOMAXPROCS(0) != test.newP+1 { - t.Fatalf("unexpected value after t.SetGOMAXPROCS: got %d, want %d", runtime.GOMAXPROCS(0), test.newP+1) - } - }) - if runtime.GOMAXPROCS(0) != p { - t.Fatalf("unexpected value after t.SetGOMAXPROCS cleanup: got %d, want %d", runtime.GOMAXPROCS(0), p) - } - } -} - -func TestSetGOMAXPROCSWithParallelAfterSetGOMAXPROCS(t *testing.T) { - if runtime.GOARCH == "wasm" { - t.Skip("not supported on wasm yet") - } - defer func() { - want := "testing: t.Parallel called after t.SetGOMAXPROCS; cannot set GOMAXPROCS in parallel tests" - if got := recover(); got != want { - t.Fatalf("expected panic; got %#v want %q", got, want) - } - }() - p := runtime.GOMAXPROCS(0) - t.SetGOMAXPROCS(p + 1) - t.Parallel() -} - -func TestSetGOMAXPROCSWithParallelBeforeSetGOMAXPROCS(t *testing.T) { - if runtime.GOARCH == "wasm" { - t.Skip("not supported on wasm yet") - } - defer func() { - want := "testing: t.SetGOMAXPROCS called after t.Parallel; cannot set GOMAXPROCS in parallel tests" - if got := recover(); got != want { - t.Fatalf("expected panic; got %#v want %q", got, want) - } - }() - t.Parallel() - p := runtime.GOMAXPROCS(0) - t.SetGOMAXPROCS(p + 1) -} - -func TestSetGOMAXPROCSWithParallelParentBeforeSetGOMAXPROCS(t *testing.T) { - if runtime.GOARCH == "wasm" { - t.Skip("not supported on wasm yet") - } - t.Parallel() - t.Run("child", func(t *testing.T) { - defer func() { - want := "testing: t.SetGOMAXPROCS called after t.Parallel; cannot set GOMAXPROCS in parallel tests" - if got := recover(); got != want { - t.Fatalf("expected panic; got %#v want %q", got, want) - } - }() - - p := runtime.GOMAXPROCS(0) - t.SetGOMAXPROCS(p + 1) - }) -} - -func TestSetGOMAXPROCSWithParallelGrandParentBeforeSetGOMAXPROCS(t *testing.T) { - if runtime.GOARCH == "wasm" { - t.Skip("not supported on wasm yet") - } - t.Parallel() - t.Run("child", func(t *testing.T) { - t.Run("grand-child", func(t *testing.T) { - defer func() { - want := "testing: t.SetGOMAXPROCS called after t.Parallel; cannot set GOMAXPROCS in parallel tests" - if got := recover(); got != want { - t.Fatalf("expected panic; got %#v want %q", got, want) - } - }() - - p := runtime.GOMAXPROCS(0) - t.SetGOMAXPROCS(p + 1) - }) - }) -} - // testingTrueInInit is part of TestTesting. var testingTrueInInit = false -- 2.48.1