From a67f9a02507564dfb702d5a60cd21e9935b62a37 Mon Sep 17 00:00:00 2001 From: lcx Date: Wed, 23 Jul 2025 17:55:56 -0400 Subject: [PATCH 1/4] fix: avoid panic when post hook panic --- suite.go | 48 ++++++++++++++++++++++++++++++++++++------------ 1 file changed, 36 insertions(+), 12 deletions(-) diff --git a/suite.go b/suite.go index 6ca1bf53..5d50ed06 100644 --- a/suite.go +++ b/suite.go @@ -320,25 +320,38 @@ func (s *suite) runBeforeStepHooks(ctx context.Context, step *Step, err error) ( return ctx, err } -func (s *suite) runAfterStepHooks(ctx context.Context, step *Step, status StepResultStatus, err error) (context.Context, error) { +func (s *suite) runAfterStepHooks(ctx context.Context, step *Step, status StepResultStatus, err error) (rctx context.Context, rerr error) { + rerr = err + rctx = ctx + defer func() { + if r := recover(); r != nil { + pe := fmt.Errorf("panic: %v", r) + if rerr == nil { + rerr = pe + } else { + rerr = fmt.Errorf("%v; %w", pe, rerr) + } + } + }() + for _, f := range s.afterStepHandlers { - hctx, herr := f(ctx, step, status, err) + hctx, herr := f(rctx, step, status, rerr) // Adding hook error to resulting error without breaking hooks loop. if herr != nil { - if err == nil { - err = herr + if rerr == nil { + rerr = herr } else { - err = fmt.Errorf("%v, %w", herr, err) + rerr = fmt.Errorf("%v, %w", herr, rerr) } } if hctx != nil { - ctx = hctx + rctx = hctx } } - return ctx, err + return rctx, rerr } func (s *suite) runBeforeScenarioHooks(ctx context.Context, pickle *messages.Pickle) (context.Context, error) { @@ -367,15 +380,26 @@ func (s *suite) runBeforeScenarioHooks(ctx context.Context, pickle *messages.Pic return ctx, err } -func (s *suite) runAfterScenarioHooks(ctx context.Context, pickle *messages.Pickle, lastStepErr error) (context.Context, error) { - err := lastStepErr +func (s *suite) runAfterScenarioHooks(ctx context.Context, pickle *messages.Pickle, lastStepErr error) (rctx context.Context, err error) { + rctx = ctx + err = lastStepErr + defer func() { + if r := recover(); r != nil { + pe := fmt.Errorf("panic: %v", r) + if err == nil { + err = pe + } else { + err = fmt.Errorf("%v; %w", pe, err) + } + } + }() hooksFailed := false isStepErr := true // run after scenario handlers for _, f := range s.afterScenarioHandlers { - hctx, herr := f(ctx, pickle, err) + hctx, herr := f(rctx, pickle, err) // Adding hook error to resulting error without breaking hooks loop. if herr != nil { @@ -394,7 +418,7 @@ func (s *suite) runAfterScenarioHooks(ctx context.Context, pickle *messages.Pick } if hctx != nil { - ctx = hctx + rctx = hctx } } @@ -402,7 +426,7 @@ func (s *suite) runAfterScenarioHooks(ctx context.Context, pickle *messages.Pick err = fmt.Errorf("after scenario hook failed: %w", err) } - return ctx, err + return rctx, err } func (s *suite) maybeUndefined(ctx context.Context, text string, arg interface{}, stepType messages.PickleStepType) (context.Context, []string, error) { From a919f6a2119aec1df97d8a69e112bc897641b1f5 Mon Sep 17 00:00:00 2001 From: lcx Date: Thu, 24 Jul 2025 00:21:11 -0400 Subject: [PATCH 2/4] feat: provide test for previous change. --- features/count.feature | 9 ++++++ run_test.go | 63 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 72 insertions(+) create mode 100644 features/count.feature diff --git a/features/count.feature b/features/count.feature new file mode 100644 index 00000000..91791d60 --- /dev/null +++ b/features/count.feature @@ -0,0 +1,9 @@ +Feature: count + + Scenario: count one two + When one + Then two + + Scenario: count three four + When three + Then four \ No newline at end of file diff --git a/run_test.go b/run_test.go index 6a070d68..d7248805 100644 --- a/run_test.go +++ b/run_test.go @@ -795,3 +795,66 @@ func Test_TestSuite_RetreiveFeatures(t *testing.T) { }) } } + +// TestHookPanic verifies that panics in ScenarioContext.After, ScenarioContext.Before, +// StepContext.Before, and StepContext.After hooks are handled gracefully by the godog. +// If this test panics and fails to report gracefully, it means the panic handling in the hooks is insufficient. +func TestHookPanic(t *testing.T) { + setupSteps := func(sc *ScenarioContext) { + doNothing := func(ctx context.Context) error { + return nil + } + sc.Step(`^one$`, doNothing) + sc.Step(`^two$`, doNothing) + sc.Step(`^three$`, doNothing) + sc.Step(`^four$`, doNothing) + } + for name, test := range map[string]struct { + initScenarios func(*ScenarioContext) + }{ + "before scenario": { + func(ctx *ScenarioContext) { + ctx.Before(func(ctx context.Context, sc *Scenario) (context.Context, error) { + panic("before scenario") + }) + setupSteps(ctx) + }, + }, + "after scenario": { + func(ctx *ScenarioContext) { + setupSteps(ctx) + ctx.After(func(ctx context.Context, sc *Scenario, err error) (context.Context, error) { + panic("after scenario") + }) + }, + }, + "after step": { + func(ctx *ScenarioContext) { + setupSteps(ctx) + ctx.StepContext().After(func(_ context.Context, _ *Step, _ StepResultStatus, _ error) (context.Context, error) { + panic("after step") + }) + }, + }, + "before step": { + func(ctx *ScenarioContext) { + setupSteps(ctx) + ctx.StepContext().Before(func(_ context.Context, _ *Step) (context.Context, error) { + panic("before step") + }) + }, + }, + } { + t.Run(name, func(t *testing.T) { + opts := Options{ + Format: "pretty", + Paths: []string{"features/count.feature"}, + } + TestSuite{ + Name: "HookPanic!", + Options: &opts, + ScenarioInitializer: test.initScenarios, + }.Run() + }) + } +} From ca22df4e047ae141ef2f864b90d72387de70f8e0 Mon Sep 17 00:00:00 2001 From: lcx Date: Thu, 24 Jul 2025 09:40:24 -0400 Subject: [PATCH 3/4] fix: use temporary file instead of features/count.feature to avoid affecting other tests --- features/count.feature | 9 --------- run_test.go | 42 ++++++++++++++++++++++++++++++++++-------- 2 files changed, 34 insertions(+), 17 deletions(-) delete mode 100644 features/count.feature diff --git a/features/count.feature b/features/count.feature deleted file mode 100644 index 91791d60..00000000 --- a/features/count.feature +++ /dev/null @@ -1,9 +0,0 @@ -Feature: count - - Scenario: count one two - When one - Then two - - Scenario: count three four - When three - Then four \ No newline at end of file diff --git a/run_test.go b/run_test.go index d7248805..00a68dd1 100644 --- a/run_test.go +++ b/run_test.go @@ -800,14 +800,40 @@ func Test_TestSuite_RetreiveFeatures(t *testing.T) { // StepContext.Before, and StepContext.After hooks are handled gracefully by the godog. // If this test panics and fails to report gracefully, it means the panic handling in the hooks is insufficient. func TestHookPanic(t *testing.T) { - setupSteps := func(sc *ScenarioContext) { - doNothing := func(ctx context.Context) error { - return nil + // create a temporary file + f, err := os.CreateTemp("", "godog_test_hook_panic_*.feature") + if err != nil { + t.Fatalf("failed to create temporary file: %v", err) + } + defer func(name string) { + if err := os.Remove(f.Name()); err != nil { + t.Fatalf("failed to remove temporary file: %v", err) } - sc.Step(`^one$`, doNothing) - sc.Step(`^two$`, doNothing) - sc.Step(`^three$`, doNothing) - sc.Step(`^four$`, doNothing) + }(f.Name()) + + _, err = f.Write([]byte(` +Feature: count + + Scenario: count one two + When one + Then two + + Scenario: count three four + When three + Then four +`)) + if err != nil { + t.Fatalf("failed to write to temporary file: %v", err) + } + err = f.Close() + if err != nil { + t.Fatalf("failed to close temporary file: %v", err) + } + setupSteps := func(sc *ScenarioContext) { + sc.Step(`^one$`, func() {}) + sc.Step(`^two$`, func() {}) + sc.Step(`^three$`, func() {}) + sc.Step(`^four$`, func() {}) } for name, test := range map[string]struct { initScenarios func(*ScenarioContext) @@ -848,7 +874,7 @@ func TestHookPanic(t *testing.T) { t.Run(name, func(t *testing.T) { opts := Options{ Format: "pretty", - Paths: []string{"features/count.feature"}, + Paths: []string{f.Name()}, } TestSuite{ Name: "HookPanic!", From 4585c9fbff2542e33c8dd484b39f3d04a997dafb Mon Sep 17 00:00:00 2001 From: lcx Date: Thu, 24 Jul 2025 09:52:51 -0400 Subject: [PATCH 4/4] typo --- run_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/run_test.go b/run_test.go index 00a68dd1..b3ccd4d2 100644 --- a/run_test.go +++ b/run_test.go @@ -815,8 +815,8 @@ func TestHookPanic(t *testing.T) { Feature: count Scenario: count one two - When one - Then two + When one + Then two Scenario: count three four When three