Skip to content

fix: prevent panic in cucumber JSON formatter on stopOnFailure#717

Open
andrelince wants to merge 1 commit into
cucumber:mainfrom
andrelince:fix-cucumber-stop-on-first-failure
Open

fix: prevent panic in cucumber JSON formatter on stopOnFailure#717
andrelince wants to merge 1 commit into
cucumber:mainfrom
andrelince:fix-cucumber-stop-on-first-failure

Conversation

@andrelince
Copy link
Copy Markdown

@andrelince andrelince commented Nov 27, 2025

🤔 What's changed?

  • Wrap access to Storage.MustGetPickleResult in the cucumber formatter so that missing PickleResults (for scenarios that never execute due to StopOnFailure) do not panic, but instead return a nil result ultimately allowing for a report output
  • Add a unit test to cucumber formater (fmt_cucumber_test.go) to assert the changes

This PR is a follow up to this other PR

⚡️ What's your motivation?

I use godog for professional integration tests that run in our PR pipeline, and we rely on a report being generated for every run (both success and failure). These test suites can take up to an hour when they pass, so we enable stopOnFailure to fail fast and free up CI resources as soon as a single scenario fails.

However, when stopOnFailure is enabled and a scenario fails, the cucumber JSON formatter panics before writing the report, so we can’t collect the partial report for the scenarios that did run. This change ensures we still get a valid JSON report even when the run stops early.

🏷️ What kind of change is this?

  • 🐛 Bug fix (non-breaking change which fixes a defect)

♻️ Anything particular you want feedback on?

📋 Checklist:

  • I agree to respect and uphold the Cucumber Community Code of Conduct
  • I've changed the behaviour of the code
    • I have added/updated tests to cover my changes.
  • My change requires a change to the documentation.
    • I have updated the documentation accordingly.
  • Users should know about my change
    • I have added an entry to the "Unreleased" section of the CHANGELOG, linking to this pull request.

This text was originally generated from a template, then edited by hand. You can modify the template here.

@andrelince andrelince marked this pull request as ready for review December 1, 2025 23:09
@vearutop vearutop requested a review from Copilot May 8, 2026 16:07
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a panic in the Cucumber JSON formatter when StopOnFailure stops execution early, ensuring a partial (but valid) JSON report can still be produced.

Changes:

  • Guard Cucumber formatter access to Storage.MustGetPickleResult so missing results don’t panic under StopOnFailure.
  • Add a unit test that runs the cucumber formatter with StopOnFailure enabled and verifies output is still valid JSON.
  • Add cucumber formatter “golden” output for the stop_on_first_failure feature to the formatter output test fixtures.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
internal/formatters/fmt_cucumber.go Avoids panics by safely handling missing PickleResult entries when runs stop early.
internal/formatters/fmt_cucumber_test.go Adds regression coverage to ensure cucumber JSON output is still produced with StopOnFailure.
internal/formatters/formatter-tests/cucumber/stop_on_first_failure Adds expected cucumber JSON output fixture for the stop_on_first_failure feature in formatter output comparisons.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +84 to 86
pickleResult := f.getPickleResult(pickle.Id)
pickleStepResults := f.Storage.MustGetPickleStepResultsByPickleID(pickle.Id)

return cukeStep
}

// getPickleResult deals with the fact that if there's no result due to 'StopOnFirstFailure' being
Comment on lines +44 to +49
elements := features[0]["elements"].([]interface{})
if len(elements) != 2 {
t.Fatalf("expected two scenarios, got %d", len(elements))
}

second := elements[1].(map[string]interface{})
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants