Merge pull request #230 from buildkite/tet-460-return-non-zero-code-w…

…hen-rspec-has-error-outside-of Handle errors outside of example
buildkite · Dec 10, 2024 · c798036 · c798036
2 parents da884c0 + 26bebe1
commit c798036
Show file tree

Hide file tree

Showing 17 changed files with 272 additions and 69 deletions.
diff --git a/internal/runner/cypress.go b/internal/runner/cypress.go
@@ -40,15 +40,13 @@ func (c Cypress) Run(result *RunResult, testCases []plan.TestCase, retry bool) e
 	}
 	cmdName, cmdArgs, err := c.commandNameAndArgs(c.TestCommand, testPaths)
 	if err != nil {
-		result.err = err
 		return fmt.Errorf("failed to build command: %w", err)
 	}
 
 	cmd := exec.Command(cmdName, cmdArgs...)
 
 	err = runAndForwardSignal(cmd)
 
-	result.err = err
 	return err
 }
 

diff --git a/internal/runner/cypress_test.go b/internal/runner/cypress_test.go
@@ -28,8 +28,8 @@ func TestCypressRun(t *testing.T) {
 		t.Errorf("Cypress.Run(%q) error = %v", testCases, err)
 	}
 
-	if result.Status() != RunStatusPassed {
-		t.Errorf("Cypress.Run(%q) RunResult.Status = %v, want %v", testCases, result.Status(), RunStatusPassed)
+	if result.Status() != RunStatusUnknown {
+		t.Errorf("Cypress.Run(%q) RunResult.Status = %v, want %v", testCases, result.Status(), RunStatusUnknown)
 	}
 }
 
@@ -47,8 +47,8 @@ func TestCypressRun_TestFailed(t *testing.T) {
 	result := NewRunResult([]plan.TestCase{})
 	err := cypress.Run(result, testCases, false)
 
-	if result.Status() != RunStatusError {
-		t.Errorf("Cypress.Run(%q) RunResult.Status = %v, want %v", testCases, result.Status(), RunStatusError)
+	if result.Status() != RunStatusUnknown {
+		t.Errorf("Cypress.Run(%q) RunResult.Status = %v, want %v", testCases, result.Status(), RunStatusUnknown)
 	}
 
 	exitError := new(exec.ExitError)
@@ -66,8 +66,8 @@ func TestCypressRun_CommandFailed(t *testing.T) {
 	result := NewRunResult([]plan.TestCase{})
 	err := cypress.Run(result, testCases, false)
 
-	if result.Status() != RunStatusError {
-		t.Errorf("Cypress.Run(%q) RunResult.Status = %v, want %v", testCases, result.Status(), RunStatusError)
+	if result.Status() != RunStatusUnknown {
+		t.Errorf("Cypress.Run(%q) RunResult.Status = %v, want %v", testCases, result.Status(), RunStatusUnknown)
 	}
 
 	exitError := new(exec.ExitError)
@@ -87,8 +87,8 @@ func TestCypressRun_SignaledError(t *testing.T) {
 	result := NewRunResult([]plan.TestCase{})
 	err := cypress.Run(result, testCases, false)
 
-	if result.Status() != RunStatusError {
-		t.Errorf("Cypress.Run(%q) RunResult.Status = %v, want %v", testCases, result.Status(), RunStatusError)
+	if result.Status() != RunStatusUnknown {
+		t.Errorf("Cypress.Run(%q) RunResult.Status = %v, want %v", testCases, result.Status(), RunStatusUnknown)
 	}
 
 	signalError := new(ProcessSignaledError)

diff --git a/internal/runner/discover_test.go b/internal/runner/discover_test.go
@@ -83,8 +83,10 @@ func TestDiscoverTestFiles_ExcludeNodeModules(t *testing.T) {
 		"testdata/cypress/cypress.config.js",
 		"testdata/jest/failure.spec.js",
 		"testdata/jest/jest.config.js",
+		"testdata/jest/runtimeError.spec.js",
 		"testdata/jest/spells/expelliarmus.spec.js",
 		"testdata/playwright/playwright.config.js",
+		"testdata/playwright/tests/error.spec.js",
 		"testdata/playwright/tests/example.spec.js",
 		"testdata/playwright/tests/failed.spec.js",
 	}

diff --git a/internal/runner/jest.go b/internal/runner/jest.go
@@ -69,7 +69,6 @@ func (j Jest) Run(result *RunResult, testCases []plan.TestCase, retry bool) erro
 		}
 		commandName, commandArgs, err := j.commandNameAndArgs(j.TestCommand, testPaths)
 		if err != nil {
-			result.err = err
 			return fmt.Errorf("failed to build command: %w", err)
 		}
 
@@ -81,7 +80,6 @@ func (j Jest) Run(result *RunResult, testCases []plan.TestCase, retry bool) erro
 		}
 		commandName, commandArgs, err := j.retryCommandNameAndArgs(j.RetryTestCommand, testNames)
 		if err != nil {
-			result.err = err
 			return fmt.Errorf("failed to build command: %w", err)
 		}
 
@@ -91,14 +89,12 @@ func (j Jest) Run(result *RunResult, testCases []plan.TestCase, retry bool) erro
 	err = runAndForwardSignal(cmd)
 
 	if ProcessSignaledError := new(ProcessSignaledError); errors.As(err, &ProcessSignaledError) {
-		result.err = err
 		return err
 	}
 
 	report, parseErr := j.ParseReport(j.ResultPath)
 	if parseErr != nil {
 		fmt.Println("Buildkite Test Engine Client: Failed to read Jest output, tests will not be retried.")
-		result.err = err
 		return err
 	}
 
@@ -123,6 +119,10 @@ func (j Jest) Run(result *RunResult, testCases []plan.TestCase, retry bool) erro
 		}
 	}
 
+	if report.NumRuntimeErrorTestSuites > 0 {
+		result.error = fmt.Errorf("Jest failed with runtime error test suites")
+	}
+
 	return nil
 }
 
@@ -134,8 +134,9 @@ type JestExample struct {
 }
 
 type JestReport struct {
-	NumFailedTests int
-	TestResults    []struct {
+	NumFailedTests            int
+	NumRuntimeErrorTestSuites int
+	TestResults               []struct {
 		AssertionResults []JestExample
 	}
 }

diff --git a/internal/runner/jest_test.go b/internal/runner/jest_test.go
@@ -89,15 +89,16 @@ func TestJestRun_Retry(t *testing.T) {
 
 	jest := NewJest(RunnerConfig{
 		TestCommand:      "jest --invalid-option --json --outputFile {{resultPath}}",
-		RetryTestCommand: "jest --testNamePattern '{{testNamePattern}}' --json --outputFile {{resultPath}}",
+		RetryTestCommand: "jest --testNamePattern '{{testNamePattern}}' --json --outputFile {{resultPath}} ./testdata/jest/spells/expelliarmus.spec.js ./testdata/jest/failure.spec.js",
+		ResultPath:       "jest.json",
 	})
 
 	t.Cleanup(func() {
 		os.Remove(jest.ResultPath)
 	})
 
 	testCases := []plan.TestCase{
-		{Name: "disarms the opponent"},
+		{Scope: "expelliarmus", Name: "disarms the opponent"},
 	}
 	result := NewRunResult([]plan.TestCase{})
 	err := jest.Run(result, testCases, true)
@@ -149,6 +150,34 @@ func TestJestRun_TestFailed(t *testing.T) {
 	}
 }
 
+func TestJestRun_RuntimeError(t *testing.T) {
+	changeCwd(t, "./testdata/jest")
+
+	jest := NewJest(RunnerConfig{
+		TestCommand: "jest --json --outputFile {{resultPath}}",
+		ResultPath:  "jest.json",
+	})
+
+	t.Cleanup(func() {
+		os.Remove(jest.ResultPath)
+	})
+
+	testCases := []plan.TestCase{
+		{Path: "./testdata/jest/spells/expelliarmus.spec.js"},
+		{Path: "./testdata/jest/runtimeError.spec.js"},
+	}
+	result := NewRunResult([]plan.TestCase{})
+	err := jest.Run(result, testCases, false)
+
+	if err != nil {
+		t.Errorf("Jest.Run(%q) error = %v", testCases, err)
+	}
+
+	if result.Status() != RunStatusError {
+		t.Errorf("Jest.Run(%q) RunResult.Status = %v, want %v", testCases, result.Status(), RunStatusError)
+	}
+}
+
 func TestJestRun_CommandFailed(t *testing.T) {
 	jest := NewJest(RunnerConfig{
 		TestCommand: "jest --invalid-option --outputFile {{resultPath}}",
@@ -162,8 +191,8 @@ func TestJestRun_CommandFailed(t *testing.T) {
 	result := NewRunResult([]plan.TestCase{})
 	err := jest.Run(result, testCases, false)
 
-	if result.Status() != RunStatusError {
-		t.Errorf("Jest.Run(%q) RunResult.Status = %v, want %v", testCases, result.Status(), RunStatusError)
+	if result.Status() != RunStatusUnknown {
+		t.Errorf("Jest.Run(%q) RunResult.Status = %v, want %v", testCases, result.Status(), RunStatusUnknown)
 	}
 
 	exitError := new(exec.ExitError)
@@ -183,8 +212,8 @@ func TestJestRun_SignaledError(t *testing.T) {
 	result := NewRunResult([]plan.TestCase{})
 	err := jest.Run(result, testCases, false)
 
-	if result.Status() != RunStatusError {
-		t.Errorf("Jest.Run(%q) RunResult.Status = %v, want %v", testCases, result.Status(), RunStatusError)
+	if result.Status() != RunStatusUnknown {
+		t.Errorf("Jest.Run(%q) RunResult.Status = %v, want %v", testCases, result.Status(), RunStatusUnknown)
 	}
 
 	signalError := new(ProcessSignaledError)
@@ -359,6 +388,7 @@ func TestJestGetFiles(t *testing.T) {
 
 	want := []string{
 		"failure.spec.js",
+		"runtimeError.spec.js",
 		"spells/expelliarmus.spec.js",
 	}
 

diff --git a/internal/runner/playwright.go b/internal/runner/playwright.go
@@ -43,7 +43,6 @@ func (p Playwright) Run(result *RunResult, testCases []plan.TestCase, retry bool
 
 	cmdName, cmdArgs, err := p.commandNameAndArgs(p.TestCommand, testPaths)
 	if err != nil {
-		result.err = err
 		return fmt.Errorf("failed to build command: %w", err)
 	}
 
@@ -52,14 +51,12 @@ func (p Playwright) Run(result *RunResult, testCases []plan.TestCase, retry bool
 	err = runAndForwardSignal(cmd)
 
 	if ProcessSignaledError := new(ProcessSignaledError); errors.As(err, &ProcessSignaledError) {
-		result.err = err
 		return err
 	}
 
 	report, parseErr := p.parseReport(p.ResultPath)
 	if parseErr != nil {
 		fmt.Println("Buildkite Test Engine Client: Failed to read Playwright output, tests will not be retried.")
-		result.err = err
 		return err
 	}
 
@@ -69,6 +66,11 @@ func (p Playwright) Run(result *RunResult, testCases []plan.TestCase, retry bool
 			result.RecordTestResult(testResult.TestCase, testResult.Status)
 		}
 	}
+
+	if len(report.Errors) > 0 {
+		result.error = fmt.Errorf("Playwright failed with errors")
+	}
+
 	return nil
 
 }
@@ -188,4 +190,7 @@ type PlaywrightReport struct {
 		Expected   int
 		Unexpected int
 	}
+	Errors []struct {
+		Message string
+	}
 }
diff --git a/internal/runner/playwright_test.go b/internal/runner/playwright_test.go
@@ -1,9 +1,12 @@
 package runner
 
 import (
+	"errors"
 	"os"
+	"os/exec"
 	"slices"
 	"strings"
+	"syscall"
 	"testing"
 
 	"github.com/buildkite/test-engine-client/internal/plan"
@@ -89,6 +92,74 @@ func TestPlaywrightRun_TestFailed(t *testing.T) {
 	}
 }
 
+func TestPlaywrightRun_Error(t *testing.T) {
+	changeCwd(t, "./testdata/playwright")
+
+	playwright := NewPlaywright(RunnerConfig{
+		ResultPath: "test-results/results.json",
+	})
+
+	testCases := []plan.TestCase{
+		{Path: "./tests/example.spec.js"},
+		{Path: "./tests/error.spec.js"},
+	}
+	result := NewRunResult([]plan.TestCase{})
+	err := playwright.Run(result, testCases, false)
+
+	if err != nil {
+		t.Errorf("Playwright.Run(%q) error = %v", testCases, err)
+	}
+
+	if result.Status() != RunStatusError {
+		t.Errorf("Playwright.Run(%q) RunResult.Status = %v, want %v", testCases, result.Status(), RunStatusError)
+	}
+}
+
+func TestPlaywrightRun_CommandFailed(t *testing.T) {
+	playwright := NewPlaywright(RunnerConfig{
+		TestCommand: "npx playwright test --oops",
+	})
+
+	testCases := []plan.TestCase{
+		{Path: "./doesnt-matter.spec.js"},
+	}
+	result := NewRunResult([]plan.TestCase{})
+	err := playwright.Run(result, testCases, false)
+
+	if result.Status() != RunStatusUnknown {
+		t.Errorf("Playwright.Run(%q) RunResult.Status = %v, want %v", testCases, result.Status(), RunStatusUnknown)
+	}
+
+	exitError := new(exec.ExitError)
+	if !errors.As(err, &exitError) {
+		t.Errorf("Playwright.Run(%q) error type = %T (%v), want *exec.ExitError", testCases, err, err)
+	}
+}
+
+func TestPlaywrightRun_SignaledError(t *testing.T) {
+	playwright := NewPlaywright(RunnerConfig{
+		TestCommand: "./testdata/segv.sh --outputFile {{resultPath}}",
+	})
+
+	testCases := []plan.TestCase{
+		{Path: "./doesnt-matter.spec.js"},
+	}
+	result := NewRunResult([]plan.TestCase{})
+	err := playwright.Run(result, testCases, false)
+
+	if result.Status() != RunStatusUnknown {
+		t.Errorf("Playwright.Run(%q) RunResult.Status = %v, want %v", testCases, result.Status(), RunStatusUnknown)
+	}
+
+	signalError := new(ProcessSignaledError)
+	if !errors.As(err, &signalError) {
+		t.Errorf("Playwright.Run(%q) error type = %T (%v), want *ErrProcessSignaled", testCases, err, err)
+	}
+	if signalError.Signal != syscall.SIGSEGV {
+		t.Errorf("Playwright.Run(%q) signal = %d, want %d", testCases, syscall.SIGSEGV, signalError.Signal)
+	}
+}
+
 func TestPlaywrightCommandNameAndArgs_WithPlaceholder(t *testing.T) {
 	testCases := []string{"tests/example.spec.js", "tests/failed.spec.js"}
 	testCommand := "npx playwright test {{testExamples}}"
@@ -147,6 +218,7 @@ func TestPlaywrightGetFiles(t *testing.T) {
 	}
 
 	want := []string{
+		"tests/error.spec.js",
 		"tests/example.spec.js",
 		"tests/failed.spec.js",
 	}

diff --git a/internal/runner/rspec.go b/internal/runner/rspec.go
@@ -86,7 +86,6 @@ func (r Rspec) Run(result *RunResult, testCases []plan.TestCase, retry bool) err
 
 	commandName, commandArgs, err := r.commandNameAndArgs(command, testPaths)
 	if err != nil {
-		result.err = err
 		return fmt.Errorf("failed to build command: %w", err)
 	}
 
@@ -95,7 +94,6 @@ func (r Rspec) Run(result *RunResult, testCases []plan.TestCase, retry bool) err
 	err = runAndForwardSignal(cmd)
 
 	if ProcessSignaledError := new(ProcessSignaledError); errors.As(err, &ProcessSignaledError) {
-		result.err = err
 		return err
 	}
 
@@ -104,7 +102,6 @@ func (r Rspec) Run(result *RunResult, testCases []plan.TestCase, retry bool) err
 		// If we can't parse the report, it indicates a failure in the rspec command itself (as opposed to the tests failing),
 		// therefore we need to bubble up the error.
 		fmt.Println("Buildkite Test Engine Client: Failed to read Rspec output, tests will not be retried.")
-		result.err = err
 		return err
 	}
 
@@ -120,6 +117,10 @@ func (r Rspec) Run(result *RunResult, testCases []plan.TestCase, retry bool) err
 		result.RecordTestResult(mapExampleToTestCase(example), status)
 	}
 
+	if report.Summary.ErrorsOutsideOfExamplesCount > 0 {
+		result.error = fmt.Errorf("RSpec failed with errors outside of examples")
+	}
+
 	return nil
 }
 
@@ -140,9 +141,10 @@ type RspecReport struct {
 	Seed     int            `json:"seed"`
 	Examples []RspecExample `json:"examples"`
 	Summary  struct {
-		ExampleCount int `json:"example_count"`
-		FailureCount int `json:"failure_count"`
-		PendingCount int `json:"pending_count"`
+		ExampleCount                 int `json:"example_count"`
+		FailureCount                 int `json:"failure_count"`
+		PendingCount                 int `json:"pending_count"`
+		ErrorsOutsideOfExamplesCount int `json:"errors_outside_of_examples_count"`
 	}
 }