diff --git a/CHANGELOG.md b/CHANGELOG.md index 1693c443a9..55a827c8c8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,16 @@ project adheres to [Semantic Versioning](http://semver.org/). ## Unreleased +### Breaking changes + +Since its introduction in 0.34.0, the `--exit-zero-on-skipped` option always made the `opa test` command return an exit code 0. When used, it now returns the exit code 0 only if no failed tests were found. + +Test runs on existing projects using `--exit-zero-on-skipped` will fail if any failed tests were inhibited by this behavior. + +### Tooling, SDK, and Runtime + +- `opa test`: Fix `--exit-zero-on-skipped` behavior to make test runs fail with failing test rules ([#6126](https://github.com/open-policy-agent/opa/issues/6126)) reported and authored by @fdaguin + ## 0.55.0 > **_NOTES:_** diff --git a/cmd/test.go b/cmd/test.go index a36b4b4d4f..3750bc2a2a 100644 --- a/cmd/test.go +++ b/cmd/test.go @@ -179,12 +179,10 @@ func runTests(ctx context.Context, txn storage.Transaction, runner *tester.Runne go func() { defer close(dup) for tr := range ch { - if !tr.Pass() && !testParams.skipExitZero { - exitCode = 2 - } - if tr.Skip && exitCode == 0 && testParams.skipExitZero { - // there is a skipped test, adding the flag -z exits 0 if there are no failures - exitCode = 0 + if !tr.Pass() { + if !(tr.Skip && testParams.skipExitZero) { + exitCode = 2 + } } tr.Trace = filterTrace(&testParams, tr.Trace) dup <- tr diff --git a/cmd/test_test.go b/cmd/test_test.go index b4132f6dcb..f1dfe31e39 100644 --- a/cmd/test_test.go +++ b/cmd/test_test.go @@ -752,6 +752,101 @@ Watching for changes ...`, } } +func testExitCode(rego string, skipExitZero bool) (int, error) { + files := map[string]string{ + "test.rego": rego, + } + + var exitCode int + var err error + test.WithTempFS(files, func(path string) { + regoFilePath := filepath.Join(path, "test.rego") + + testParams := newTestCommandParams() + testParams.count = 1 + testParams.skipExitZero = skipExitZero + + exitCode, err = opaTest([]string{regoFilePath}, testParams) + }) + return exitCode, err +} + +func TestExitCode(t *testing.T) { + testCases := map[string]struct { + Test string + ExitZeroOnSkipped bool + ExpectedExitCode int + }{ + "pass when no failed or skipped tests": { + Test: `package foo + test_pass { true } + `, + ExitZeroOnSkipped: false, + ExpectedExitCode: 0, + }, + "fail when failed tests": { + Test: `package foo + test_pass { true } + test_fail { false } + `, + ExitZeroOnSkipped: false, + ExpectedExitCode: 2, + }, + "fail when skipped tests": { + Test: `package foo + test_pass { true } + todo_test_skip { true } + `, + ExitZeroOnSkipped: false, + ExpectedExitCode: 2, + }, + "fail when failed tests and skipped tests": { + Test: `package foo + test_pass { true } + test_fail { false } + todo_test_skip { true } + `, + ExitZeroOnSkipped: false, + ExpectedExitCode: 2, + }, + "pass when skipped tests and exit zero on skipped": { + Test: `package foo + test_pass { true } + todo_test_skip { true } + `, + ExitZeroOnSkipped: true, + ExpectedExitCode: 0, + }, + "fail when failed tests and exit zero on skipped": { + Test: `package foo + test_pass { true } + test_fail { false } + `, + ExitZeroOnSkipped: true, + ExpectedExitCode: 2, + }, + "fail when failed tests, skipped tests and exit zero on skipped": { + Test: `package foo + test_pass { true } + test_fail { false } + todo_test_skip { true } + `, + ExitZeroOnSkipped: true, + ExpectedExitCode: 2, + }, + } + + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + exitCode, _ := testExitCode(tc.Test, tc.ExitZeroOnSkipped) + + if exitCode != tc.ExpectedExitCode { + t.Errorf("Expected exit code to be %d but got %d", tc.ExpectedExitCode, exitCode) + } + }) + } +} + type blockingWriter struct { m sync.Mutex buf bytes.Buffer