diff --git a/test/utils/ktesting/internal/synctestinit/synctestinit_test.go b/test/utils/ktesting/internal/synctestinit/synctestinit_test.go new file mode 100644 index 00000000000..2920313f1c9 --- /dev/null +++ b/test/utils/ktesting/internal/synctestinit/synctestinit_test.go @@ -0,0 +1,43 @@ +/* +Copyright The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package ktesting_test + +import ( + "testing" + "testing/synctest" + + "k8s.io/kubernetes/test/utils/ktesting" +) + +// TestSyncTestInit matches the corresponding test in the main package. It +// exists here as the only test inside this package because signal.Notify fails +// inside a synctest bubble when called for the first time in a process and we +// want to enforce that situation. +func TestSyncTestInit(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + // This must work inside a synctest bubble, despite Deadline panicking there. + // We then don't have a deadline. + tCtx := ktesting.Init(t) + deadline, ok := tCtx.Deadline() + if ok { + tCtx.Errorf("Expected no deadline, got %s", deadline) + } + if !tCtx.IsSyncTest() { + tCtx.Errorf("Expected to run as synctest") + } + }) +} diff --git a/test/utils/ktesting/signals.go b/test/utils/ktesting/signals.go index 4edef18bd96..3a0a759ead5 100644 --- a/test/utils/ktesting/signals.go +++ b/test/utils/ktesting/signals.go @@ -77,7 +77,14 @@ var _ ginkgoReporter = &progressReporter{} // USR1 signal, similar to the corresponding Ginkgo feature. // // This support is active until the last test terminates. -func (p *progressReporter) init(tb TB) context.Context { +// +// Inside a bubble, signal.Notify fails with "select on synctest channel from +// outside bubble" if (and only if) it gets called for the first time, so we +// have to avoid setting up signal handling to be on the safe side. +func (p *progressReporter) init(tb TB, isSyncTest bool) context.Context { + if isSyncTest { + return context.Background() + } if _, ok := tb.(testing.TB); !ok { // Not in a Go unit test. return context.Background() diff --git a/test/utils/ktesting/tcontext.go b/test/utils/ktesting/tcontext.go index c9002527190..43d62a506f4 100644 --- a/test/utils/ktesting/tcontext.go +++ b/test/utils/ktesting/tcontext.go @@ -117,8 +117,24 @@ type ContextTB interface { // Ginkgo create a fresh context for cleanup code. // // Can be called more than once per test to get different contexts with -// independent cancellation. The default behavior describe above can be +// independent cancellation. The default behavior described above can be // modified via optional functional options defined in [initoption]. +// +// Can be called inside a synctest bubble. Signal handling (cleaning up on +// SIGINT, progress reporting on SIGUSR1) then does not get initialized because +// code running inside a bubble should not depend on outside input. Progress +// reporting still works when some parent test already initialized it. +// Therefore the recommended pattern is to initialize ktesting first, then +// create the synctest bubble: +// +// func TestSomething(t *testing.T) { ktesting.Init(t).SyncTest("", testSomething) } +// func testSomething(tCtx ktesting.TContext) { ... } +// +// This pattern also has the advantage that the test code cannot accidentally +// use the testing.T instance directly. The same works for normal tests: +// +// func TestSomething(t *testing.T) { testSomething(ktesting.Init(t)) } +// func testSomething(tCtx ktesting.TContext) { ... } func Init(tb TB, opts ...InitOption) TContext { tb.Helper() @@ -130,13 +146,31 @@ func Init(tb TB, opts ...InitOption) TContext { } // We don't need a Deadline implementation, testing.B doesn't have it. - // But if we have one, we'll use it to set a timeout shortly before - // the deadline. This needs to come before we wrap tb. - deadlineTB, deadlineOK := tb.(interface { + // But if we have one, we use it to determine the deadline and + // set a timeout shortly before it. + // + // This also allows us to detect a synctest bubble. + isSyncTest := false + var deadline *time.Time + if deadlineTB, deadlineOK := tb.(interface { Deadline() (time.Time, bool) - }) + }); deadlineOK { + func() { + defer func() { + // Calling testing.T.Deadline panics inside a synctest bubble. + // There's no API to detect that in advance, so here we react + // by catching the panic. + if r := recover(); r != nil { + isSyncTest = true + } + }() + if d, ok := deadlineTB.Deadline(); ok { + deadline = &d + } + }() + } - ctx := defaultProgressReporter.init(tb) + ctx := defaultProgressReporter.init(tb, isSyncTest) var header func() string if c.PerTestOutput { logger := newLogger(tb, c.BufferLogs) @@ -145,16 +179,15 @@ func Init(tb TB, opts ...InitOption) TContext { } var cancelTimeout func(cause string) - if deadlineOK { - if deadline, ok := deadlineTB.Deadline(); ok { - timeLeft := time.Until(deadline) - timeLeft -= CleanupGracePeriod - ctx, cancelTimeout = withTimeout(ctx, tb, timeLeft, fmt.Sprintf("test suite deadline (%s) is close, need to clean up before the %s cleanup grace period", deadline.Truncate(time.Second), CleanupGracePeriod)) - } + if deadline != nil { + timeLeft := time.Until(*deadline) + timeLeft -= CleanupGracePeriod + ctx, cancelTimeout = withTimeout(ctx, tb, timeLeft, fmt.Sprintf("test suite deadline (%s) is close, need to clean up before the %s cleanup grace period", deadline.Truncate(time.Second), CleanupGracePeriod)) } // Construct new TContext with context and settings as determined above. tCtx := InitCtx(ctx, tb) + tCtx.isSyncTest = isSyncTest if cancelTimeout != nil { tCtx.cancel = cancelTimeout } else { @@ -366,7 +399,7 @@ type TContext struct { // It's empty if there are no steps. steps string - // for SyncTest + // for IsSyncTest isSyncTest bool // for WithClient @@ -483,6 +516,9 @@ func (tCtx TContext) Run(name string, cb func(tCtx TContext)) bool { // the bubble directly in the current test context. // // Only works in Go unit tests. +// +// Cleaning up on SIGINT is not available because code running inside a bubble +// should not depend on outside input. func (tCtx TContext) SyncTest(name string, cb func(tCtx TContext)) bool { return run(tCtx, name, true, cb) } diff --git a/test/utils/ktesting/tcontext_test.go b/test/utils/ktesting/tcontext_test.go index e5f5f7f00ce..01a03015b32 100644 --- a/test/utils/ktesting/tcontext_test.go +++ b/test/utils/ktesting/tcontext_test.go @@ -19,6 +19,8 @@ package ktesting_test import ( "sync" "testing" + "testing/synctest" + "time" "github.com/onsi/gomega" "github.com/stretchr/testify/assert" @@ -59,6 +61,76 @@ func TestCancelAutomatic(t *testing.T) { }() } +func TestSyncTestInit(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + // This must work inside a synctest bubble, despite Deadline panicking there. + // We then don't have a deadline. + tCtx := ktesting.Init(t) + deadline, ok := tCtx.Deadline() + if ok { + tCtx.Errorf("Expected no deadline, got %s", deadline) + } + if !tCtx.IsSyncTest() { + tCtx.Errorf("Expected to run as synctest") + } + }) +} + +func TestNormalInit(t *testing.T) { + // The outcome depends on how the unit test was started. + // See below for deterministic deadline/no deadline testing. + expectDeadline, expectOK := t.Deadline() + expectDeadline = expectDeadline.Add(-ktesting.CleanupGracePeriod) + tCtx := ktesting.Init(t) + actualDeadline, actualOK := tCtx.Deadline() + tCtx.Expect(actualOK).To(gomega.Equal(expectOK), "have deadline") + if expectOK { + tCtx.Expect(actualDeadline).To(gomega.BeTemporally("~", expectDeadline, 2*time.Second), "deadline") + } + if tCtx.IsSyncTest() { + tCtx.Errorf("Expected to not run as synctest") + } +} + +func TestNoDeadline(t *testing.T) { + mockT := &deadlineT{T: t, deadline: nil} + tCtx := ktesting.Init(mockT) + deadline, ok := tCtx.Deadline() + if ok { + tCtx.Errorf("Expected no deadline, got %s", deadline) + } +} + +func TestDeadline(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + // Inside a synctest bubble this is always in the future. + mockDeadline := time.Date(2000, 01, 01, 0, 0, 0, 0, time.UTC) + mockT := &deadlineT{T: t, deadline: &mockDeadline} + tCtx := ktesting.Init(mockT) + actualDeadline, ok := tCtx.Deadline() + if ok { + expectDeadline := mockDeadline.Add(-ktesting.CleanupGracePeriod) + tCtx.Expect(actualDeadline).To(gomega.BeTemporally("==", expectDeadline), "deadline") + } else { + tCtx.Error("Expected a deadline, got none") + } + }) +} + +// deadlineT overrides Deadline, returning false if no +// deadline is configured and the deadline otherwise. +type deadlineT struct { + *testing.T + deadline *time.Time +} + +func (t *deadlineT) Deadline() (time.Time, bool) { + if t.deadline == nil { + return time.Time{}, false + } + return *t.deadline, true +} + func TestCancelCtx(t *testing.T) { tCtx := ktesting.Init(t) var discardLogger klog.Logger