diff --git a/README.md b/README.md index 160a53c..321b93d 100644 --- a/README.md +++ b/README.md @@ -23,3 +23,27 @@ to ensure a change to an internal service that is used by multiple APIs does not ## Examples Please see the [Example](./example) directory. + +## Test-scoped resources + +Use `testy.Cleanup(t, cleanup)` immediately after acquiring a resource in a +`Test` or `t.Run` callback. Callbacks run last-in, first-out after subtests, even +when the test fails fatally or panics. A failing or panicking cleanup does not +prevent earlier callbacks from running. Cleanup errors should fail the test; +logging and ignoring them reports a misleading pass. + +`testy.Context(t)` returns a context canceled immediately before cleanup starts. +Subtests inherit parent cancellation; native runs also honor the Go test deadline. +Use a separate, bounded context for cleanup I/O. These helpers are optional +capabilities, so existing custom `TestingT` implementations still compile, but +must implement `Cleanup(func())` and `Context() context.Context` to use them. +Legacy Before/After hooks are not resource scopes: migrate resource acquisition +into a Test/Run callback before using the lifecycle helpers. + +Use ordinary named `t.Run` cases (or `TestEach`) for repeated contracts. Hosted +execution can run packages concurrently and remains serial inside each package; `Parallel` is +only effective with `RunAsTest`. Do not make acceptance correctness depend on +subtest scheduling, and do not use a parent's `defer` to release resources needed +by native parallel children. Cleanup waits for those children in native runs. +These callbacks cannot recover resources after process termination; persistent +fixture providers must retain ownership and support external reconciliation. diff --git a/lifecycle.go b/lifecycle.go new file mode 100644 index 0000000..40e2e99 --- /dev/null +++ b/lifecycle.go @@ -0,0 +1,74 @@ +package testy + +import ( + "context" + "sync" +) + +// Cleanup registers f to run after this test and its subtests, in last-in, +// first-out order. It runs on return, Fatal/FailNow, and panic. Register cleanup +// immediately after acquiring a resource. Cleanup is supported in Test and Run +// callbacks, not the legacy Before/After hooks. +// +// This optional capability leaves TestingT source-compatible with custom +// implementations. Custom runners must implement Cleanup(func()) to use it; +// unsupported implementations panic rather than silently leak resources. +func Cleanup(t TestingT, f func()) { + t.Helper() + owner, ok := t.(interface{ Cleanup(func()) }) + if !ok { + panic("testy: TestingT does not support Cleanup") + } + owner.Cleanup(f) +} + +// Context returns this test's context, canceled immediately before its cleanup +// callbacks run. Subtest contexts inherit their parent's cancellation. Cleanup +// that performs I/O must use its own bounded context. Like Cleanup, this is an +// optional capability for custom TestingT implementations and is unavailable in +// legacy Before/After hooks. +func Context(t TestingT) context.Context { + owner, ok := t.(interface{ Context() context.Context }) + if !ok { + panic("testy: TestingT does not support Context") + } + return owner.Context() +} + +type lifecycle struct { + ctx context.Context + cancel context.CancelFunc + mu sync.Mutex + cleanups []func() +} + +func newLifecycle(parent context.Context) *lifecycle { + ctx, cancel := context.WithCancel(parent) + return &lifecycle{ctx: ctx, cancel: cancel} +} + +func (l *lifecycle) add(f func()) { + if f == nil { + panic("testy: nil cleanup") + } + l.mu.Lock() + defer l.mu.Unlock() + l.cleanups = append(l.cleanups, f) +} + +// Each callback defers the remainder so even panic or Goexit cannot abandon +// earlier registrations. Pop at execution time to support nested registration. +func (l *lifecycle) finish() { + l.cancel() + l.mu.Lock() + if len(l.cleanups) == 0 { + l.mu.Unlock() + return + } + i := len(l.cleanups) - 1 + f := l.cleanups[i] + l.cleanups = l.cleanups[:i] + l.mu.Unlock() + defer l.finish() + f() +} diff --git a/lifecycle_test.go b/lifecycle_test.go new file mode 100644 index 0000000..08407cb --- /dev/null +++ b/lifecycle_test.go @@ -0,0 +1,189 @@ +package testy + +import ( + "context" + "os" + "os/exec" + "reflect" + "strings" + "testing" + "time" + + "github.com/gametimesf/testy/internal/orderedmap" +) + +func lifecycleExercise(t TestingT, mode string, record func(string)) { + ctx := Context(t) + Cleanup(t, func() { record("first") }) + Cleanup(t, func() { + if ctx.Err() != context.Canceled { + t.Errorf("context was not canceled before cleanup") + } + record("last") + switch mode { + case "cleanup-fatal": + t.Fatal("cleanup failure") + case "cleanup-panic": + panic("cleanup failure") + case "nested-cleanup": + Cleanup(t, func() { record("nested") }) + } + }) + t.Run("child", func(child TestingT) { + childCtx := Context(child) + if childCtx == ctx { + child.Fatal("child must have its own context") + } + Cleanup(child, func() { record("child") }) + }) + if ctx.Err() != nil { + t.Errorf("child cleanup canceled parent") + } + switch mode { + case "fatal": + t.Fatal("body failure") + case "panic": + panic("body failure") + } +} + +func TestHostedLifecycle(t *testing.T) { + for _, mode := range []string{"normal", "fatal", "panic", "cleanup-fatal", "cleanup-panic", "nested-cleanup"} { + t.Run(mode, func(t *testing.T) { + var got []string + result := runTest("lifecycle", mode, func(tt TestingT) { + lifecycleExercise(tt, mode, func(s string) { got = append(got, s) }) + }) + want := []string{"child", "last", "first"} + if mode == "nested-cleanup" { + want = []string{"child", "last", "nested", "first"} + } + if !reflect.DeepEqual(want, got) { + t.Fatalf("cleanup order: got %v want %v", got, want) + } + failed := mode != "normal" && mode != "nested-cleanup" + if (result.Result == ResultFailed) != failed { + t.Fatalf("unexpected result %s", result.Result) + } + }) + } +} + +// A subprocess exercises native testing.T fatal/panic paths without making the +// parent suite fail. The same behavior is checked against the hosted runner above. +func TestNativeLifecycle(t *testing.T) { + if mode := os.Getenv("TESTY_LIFECYCLE_MODE"); mode != "" { + lifecycleExercise(newTWrapper(t, context.Background()), mode, func(s string) { t.Log("cleanup-marker:" + s) }) + return + } + for _, mode := range []string{"normal", "fatal", "panic", "cleanup-fatal", "cleanup-panic", "nested-cleanup"} { + t.Run(mode, func(t *testing.T) { + cmd := exec.Command(os.Args[0], "-test.run=^TestNativeLifecycle$", "-test.v") + cmd.Env = append(os.Environ(), "TESTY_LIFECYCLE_MODE="+mode) + output, err := cmd.CombinedOutput() + failed := mode != "normal" && mode != "nested-cleanup" + if (err != nil) != failed { + t.Fatalf("exit error=%v: %s", err, output) + } + markers := []string{"child", "last", "first"} + if mode == "nested-cleanup" { + markers = []string{"child", "last", "nested", "first"} + } + text := string(output) + for _, marker := range markers { + i := strings.Index(text, "cleanup-marker:"+marker) + if i < 0 { + t.Fatalf("missing or out-of-order %s: %s", marker, output) + } + text = text[i+len("cleanup-marker:"+marker):] + } + if strings.Contains(string(output), "context was not canceled") || strings.Contains(string(output), "child cleanup canceled") { + t.Fatalf("context lifecycle: %s", output) + } + }) + } +} + +func TestNativeParallelChildKeepsParentFixture(t *testing.T) { + parent := newTWrapper(t, context.Background()) + alive := true + Cleanup(parent, func() { alive = false }) + parent.Run("parallel", func(child TestingT) { + child.Parallel() + if !alive { + child.Fatal("parent fixture cleaned before parallel child") + } + }) +} + +func TestLifecycleHooksRejected(t *testing.T) { + for _, tt := range []TestingT{&tHelper{}, tWrapper{t: t}} { + func() { + defer func() { + if recover() == nil { + t.Error("unsupported cleanup must panic") + } + }() + Cleanup(tt, func() {}) + }() + } +} + +// Embedding preserves source compatibility with existing custom TestingT types. +type tHelper struct{ TestingT } + +func (*tHelper) Helper() {} + +func TestLifecycleAfterTestParity(t *testing.T) { + for _, runner := range []string{"native", "hosted"} { + t.Run(runner, func(t *testing.T) { + var order []string + pkg := &testPkg{tests: orderedmap.OrderedMap[string, testCase]{}} + pkg.BeforeTest = func(TestingT) { order = append(order, "before") } + pkg.AfterTest = func(TestingT) { order = append(order, "after") } + pkg.tests["case"] = testCase{Name: "case", tester: func(tt TestingT) { + order = append(order, "body") + Cleanup(tt, func() { order = append(order, "cleanup") }) + }} + if runner == "hosted" { + runPackage("lifecycle", pkg) + } else { + previous := instance + instance = testy{tests: orderedmap.OrderedMap[string, *testPkg]{"lifecycle": pkg}} + defer func() { instance = previous }() + t.Run("suite", RunAsTest) + } + if !reflect.DeepEqual(order, []string{"before", "body", "cleanup", "after"}) { + t.Fatalf("order %v", order) + } + }) + } +} + +func TestLifecycleContextInheritance(t *testing.T) { + parent, cancel := context.WithCancel(context.Background()) + wrapper := newTWrapper(t, parent) + cancel() + if Context(wrapper).Err() != context.Canceled { + t.Fatal("wrapper did not inherit parent cancellation") + } + result := runTestContext(parent, "lifecycle", "canceled", func(tt TestingT) { + if Context(tt).Err() != context.Canceled { + tt.Fatal("hosted test did not inherit cancellation") + } + tt.Run("child", func(child TestingT) { + if Context(child).Err() != context.Canceled { + child.Fatal("child did not inherit cancellation") + } + }) + }) + if result.Result != ResultPassed { + t.Fatalf("result %+v", result) + } + deadlineParent, stop := context.WithTimeout(context.Background(), time.Minute) + defer stop() + child := newTWrapper(t, deadlineParent) + if _, ok := Context(child).Deadline(); !ok { + t.Fatal("deadline was dropped") + } +} diff --git a/run.go b/run.go index fc88c46..d797c19 100644 --- a/run.go +++ b/run.go @@ -1,6 +1,7 @@ package testy import ( + "context" "errors" "fmt" "runtime/debug" @@ -60,9 +61,10 @@ func RunAsTest(t *testing.T) { t.Run(test.Name, func(tt *testing.T) { tt.Helper() - // if we have an AfterTest, defer it so it always runs even if BeforeTest or the test itself panic + // Register enclosing teardown before the test lifecycle so it runs + // after resource cleanup, including fatal/panic and parallel children. if pkgTests.AfterTest != nil { - defer pkgTests.AfterTest(tWrapper{t: tt}) + tt.Cleanup(func() { pkgTests.AfterTest(tWrapper{t: tt}) }) } // if we have a BeforeTest, just run it directly; panics will sort themselves out @@ -70,7 +72,7 @@ func RunAsTest(t *testing.T) { pkgTests.BeforeTest(tWrapper{t: tt}) } - test.tester(tWrapper{t: tt}) + test.tester(newTWrapper(tt, context.Background())) }) return true }) @@ -370,6 +372,10 @@ func runPackage(pkg string, pkgTests *testPkg) TestResult { } func runTest(pkg, baseName string, tester Tester) TestResult { + return runTestContext(context.Background(), pkg, baseName, tester) +} + +func runTestContext(ctx context.Context, pkg, baseName string, tester Tester) TestResult { result := TestResult{ Package: pkg, Name: baseName, @@ -378,6 +384,7 @@ func runTest(pkg, baseName string, tester Tester) TestResult { subtests := make(chan subtest) subtestDone := make(chan bool) t := &t{ + lifecycle: newLifecycle(ctx), name: baseName, tester: tester, subtests: subtests, @@ -391,7 +398,7 @@ func runTest(pkg, baseName string, tester Tester) TestResult { go func() { defer stWg.Done() for st := range subtests { - stResult := runTest(pkg, baseName+"/"+st.name, st.tester) + stResult := runTestContext(t.Context(), pkg, baseName+"/"+st.name, st.tester) if stResult.Result == ResultFailed { // TODO does this need to be an atomic operation? anyFailures = true diff --git a/t.go b/t.go index 671bad7..ced7241 100644 --- a/t.go +++ b/t.go @@ -1,12 +1,14 @@ package testy import ( + "context" "fmt" "runtime" "strings" ) type t struct { + lifecycle *lifecycle name string tester Tester failed bool @@ -38,6 +40,7 @@ func (t *t) run() { close(t.subtests) }() + defer t.lifecycle.finish() t.tester(t) } @@ -95,3 +98,17 @@ func (t *t) Run(name string, tester Tester) bool { // Parallel does nothing for this implementation. // TODO figure out how to support it. func (*t) Parallel() {} + +func (t *t) Cleanup(f func()) { + if t.lifecycle == nil { + panic("testy: Cleanup is only supported in Test and Run callbacks") + } + t.lifecycle.add(f) +} + +func (t *t) Context() context.Context { + if t.lifecycle == nil { + panic("testy: Context is only supported in Test and Run callbacks") + } + return t.lifecycle.ctx +} diff --git a/twrapper.go b/twrapper.go index 83b8c5f..457e657 100644 --- a/twrapper.go +++ b/twrapper.go @@ -1,12 +1,14 @@ package testy import ( + "context" "testing" ) // tWrapper wraps a real testing.T, because Run takes a concrete implementation. type tWrapper struct { - t *testing.T + t *testing.T + lifecycle *lifecycle } var _ TestingT = (*tWrapper)(nil) @@ -55,10 +57,40 @@ func (t tWrapper) Run(s string, tester Tester) bool { t.t.Helper() return t.t.Run(s, func(tt *testing.T) { t.t.Helper() - tester(tWrapper{t: tt}) + parent := context.Background() + if t.lifecycle != nil { + parent = t.lifecycle.ctx + } + tester(newTWrapper(tt, parent)) }) } func (t tWrapper) Parallel() { t.t.Parallel() } + +func newTWrapper(t *testing.T, parent context.Context) tWrapper { + l := newLifecycle(parent) + if deadline, ok := t.Deadline(); ok { + ctx, cancel := context.WithDeadline(l.ctx, deadline) + l.ctx = ctx + previousCancel := l.cancel + l.cancel = func() { cancel(); previousCancel() } + } + t.Cleanup(l.finish) + return tWrapper{t: t, lifecycle: l} +} + +func (t tWrapper) Cleanup(f func()) { + if t.lifecycle == nil { + panic("testy: Cleanup is only supported in Test and Run callbacks") + } + t.lifecycle.add(f) +} + +func (t tWrapper) Context() context.Context { + if t.lifecycle == nil { + panic("testy: Context is only supported in Test and Run callbacks") + } + return t.lifecycle.ctx +}