Skip to content
Draft
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions internal/runner/rspec.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,16 @@ func (r Rspec) Run(result *RunResult, testCases []plan.TestCase, retry bool) err
result.error = fmt.Errorf("RSpec failed with errors outside of examples")
}

if exitError := new(exec.ExitError); errors.As(err, &exitError) {
if report.Summary.FailureCount == 0 && report.Summary.ErrorsOutsideOfExamplesCount == 0 {
Copy link
Contributor

@malclocke malclocke Feb 5, 2026

Choose a reason for hiding this comment

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

Is there a way we can invert the logic here? We have if ExitError and some inclusion condition and I think this would be a lot better as if ExitError and not some exclusion condition. I.e. change this to "any failure status from the runner is considered an error except for when we explicitly allow it".

This change is good but it leaves us in a similar situation to where we are now, where any unexpected failure state is a pass by default.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fair point. But it's more complicated than that. We don't return error from this function when RSpec succesfully run and finished because we want to print the result at the end of the run. Instead, we build a RunResult from the test runner json output, that decided whether bktec will exit with 0 or 1.

I can revert the logic, but we can't simply make "any unexpected failure state is a failed by default", unless we are okay with skipping report printing by default.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've updated the logic, let me know what you think.

// If Rspec exits with a non-zero exit code, but the report is parsed successfully and contains no failures or errors,
// it may mean that the process was terminated by an explicit call to `exit` in the code or specs.
// Rspec does not report this as a test failure or error, but instead exits with the same exit code that terminated the process.
// In this case, we should mark the result as an error.
result.error = fmt.Errorf("RSpec exited with code %d, but no failed tests were reported. This may be caused by an explicit call to `exit` in the code or specs", exitError.ExitCode())
}
}

return nil
}

Expand Down
30 changes: 30 additions & 0 deletions internal/runner/rspec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,36 @@ func TestRspecRun_TestSkipped(t *testing.T) {
}
}

func TestRspecRun_TestExit(t *testing.T) {
rspec := NewRspec(RunnerConfig{
TestCommand: "rspec --format json --out {{resultPath}} --format progress",
ResultPath: "tmp/rspec.json",
})

t.Cleanup(func() {
os.Remove(rspec.ResultPath)
})

testCases := []plan.TestCase{
{Path: "./testdata/rspec/spec/exit_spec.rb"},
}
result := NewRunResult([]plan.TestCase{})
err := rspec.Run(result, testCases, false)

if err != nil {
t.Errorf("Rspec.Run(%q) error = %v", testCases, err)
}

if result.Status() != RunStatusError {
t.Errorf("Rspec.Run(%q) RunResult.Status = %v, want %v", testCases, result.Status(), RunStatusError)
}

wantError := "RSpec exited with code 7, but no failed tests were reported. This may be caused by an explicit call to `exit` in the code or specs"
if diff := cmp.Diff(result.error.Error(), wantError); diff != "" {
t.Errorf("Rspec.Run(%q) RunResult.error diff (-got +want):\n%s", testCases, diff)
}
}

func TestRspecRun_ErrorOutsideOfExamples(t *testing.T) {
rspec := NewRspec(RunnerConfig{
TestCommand: "rspec --format json --out {{resultPath}} --format documentation",
Expand Down
5 changes: 5 additions & 0 deletions internal/runner/testdata/rspec/spec/exit_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
RSpec.describe("Exit") do
it("exits") do
exit 7
end
end