diff --git a/docs/pytest-pants.md b/docs/pytest-pants.md index efdc0839..ad96ee2f 100644 --- a/docs/pytest-pants.md +++ b/docs/pytest-pants.md @@ -10,64 +10,70 @@ > > - Filter test files > - Split slow files by individual test example +> - Filter test by tags > - Skip tests -To integrate bktec with pants, you need to [install and configure Buildkite Test Collector for pytest](https://buildkite.com/docs/test-engine/python-collectors#pytest-collector) first. Then set the `BUILDKITE_TEST_ENGINE_TEST_RUNNER` environment variable to `pytest-pants`. +Set the `BUILDKITE_TEST_ENGINE_TEST_RUNNER` environment variable to `pytest-pants` to use bktec with pants. -Look at the example configuration files in the [pytest_pants testdata directory](../internal/runner/testdata/pytest_pants) for an example of how to add buildkite-test-collector to the pants resolve used by pytest. Specifically: +```sh +export BUILDKITE_TEST_ENGINE_TEST_RUNNER=pytest-pants +bktec run +``` -- [pants.toml](../internal/runner/testdata/pytest_pants/pants.toml) - pants configuration -- [3rdparty/python/BUILD](../internal/runner/testdata/pytest_pants/3rdparty/python/BUILD) - python_requirement targets -- [3rdparty/python/pytest-requirements.txt](../internal/runner/testdata/pytest_pants/3rdparty/python/pytest-requirements.txt) - Python requirements.txt +bktec works with pytest-pants in two modes depending on which result flag you include in your test command: -In the example in the repository, you would need to generate a lockfile next, i.e. +- **With `--junit-xml={{resultPath}}`** (default): bktec uses JUnit XML output. No additional dependencies are required. +- **With `--json={{resultPath}}`**: bktec uses the collector's JSON output for richer test result data. This requires [Buildkite Test Collector for pytest](https://buildkite.com/docs/test-engine/python-collectors#pytest-collector) to be added to the pants resolve used by pytest. + +## Configure test command + +There is no default command for pants. You must set `BUILDKITE_TEST_ENGINE_TEST_CMD`. + +bktec determines the output format by detecting `--junit-xml` or `--json=` in your test command. Below are a few recommendations for specific scenarios. + +### JUnit XML output (default) ```sh -pants generate-lockfiles --resolve=pytest +export BUILDKITE_TEST_ENGINE_TEST_CMD="pants --filter-target-type=python_test test //:: -- --junit-xml={{resultPath}}" ``` -Only running `pants test` with `python_test` targets is supported at this time. +This command is a good option if you want to run all python tests in your repository. ```sh -export BUILDKITE_TEST_ENGINE_TEST_RUNNER=pytest-pants -export BUILDKITE_TEST_ENGINE_TEST_CMD="pants --filter-target-type=python_test --changed-since=HEAD~1 test -- --json={{resultPath}} --merge-json" -bktec run +export BUILDKITE_TEST_ENGINE_TEST_CMD="pants --filter-target-type=python_test --changed-since=HEAD~1 test -- --junit-xml={{resultPath}}" ``` -## Configure test command +This command is a good option if you want to only run the python tests that were impacted by any changes made since `HEAD~1`. Checkout [pants Advanced target selection doc][pants-advanced-target-selection] for more information on `--changed-since`. -While pants support is experimental there is no default command. That means it is required to set `BUILDKITE_TEST_ENGINE_TEST_CMD`. -Below are a few recommendations for specific scenarios: +> [!IMPORTANT] +> Make sure to include `-- --junit-xml={{resultPath}}` in your custom pants test command, as bktec requires this option to read the test results for retries and verification purposes. ---- +### JSON output (with buildkite-test-collector) -```sh -export BUILDKITE_TEST_ENGINE_TEST_CMD="pants --filter-target-type=python_test test //:: -- --json={{resultPath}} --merge-json"" -``` +To use JSON output, add `buildkite-test-collector` to the pants resolve used by pytest. Look at the example configuration files in the [pytest_pants testdata directory](../internal/runner/testdata/pytest_pants) for reference: -This command is a good option if you want to run all python tests in your repository. +- [pants.toml](../internal/runner/testdata/pytest_pants/pants.toml) - pants configuration +- [3rdparty/python/BUILD](../internal/runner/testdata/pytest_pants/3rdparty/python/BUILD) - python_requirement targets +- [3rdparty/python/pytest-requirements.txt](../internal/runner/testdata/pytest_pants/3rdparty/python/pytest-requirements.txt) - Python requirements.txt ---- +After updating the configuration, generate a lockfile: ```sh -export BUILDKITE_TEST_ENGINE_TEST_CMD="pants --filter-target-type=python_test --changed-since=HEAD~1 test -- --json={{resultPath}} --merge-json" +pants generate-lockfiles --resolve=pytest ``` -This command is a good option if you want to only run the python tests that were -impacted by any changes made since `HEAD~1`. Checkout [pants Advanced target -selection doc][pants-advanced-target-selection] for more information on -`--changed-since`. +Then set your test command to use `--json={{resultPath}} --merge-json`: ---- - -In both commands, `{{resultPath}}` is replaced with a unique temporary path created by bktec. `--json` option is a custom pytest option added by Buildkite Test Collector to save the result into a JSON file at given path. You can further customize the test command for your specific use case. +```sh +export BUILDKITE_TEST_ENGINE_TEST_CMD="pants --filter-target-type=python_test test //:: -- --json={{resultPath}} --merge-json" +``` > [!IMPORTANT] -> Make sure to append `-- --json={{resultPath}} --merge-json` in your custom pants test command, as bktec requires these options to read the test results for retries and verification purposes. +> Make sure to include `-- --json={{resultPath}} --merge-json` in your custom pants test command, as bktec requires these options to read the test results for retries and verification purposes. ## Filter test files -There is not support for filtering test files at this time. +There is no support for filtering test files at this time. ## Automatically retry failed tests diff --git a/internal/runner/pytest_pants.go b/internal/runner/pytest_pants.go index 263b6974..23ab3f5b 100644 --- a/internal/runner/pytest_pants.go +++ b/internal/runner/pytest_pants.go @@ -13,6 +13,7 @@ import ( type PytestPants struct { RunnerConfig + resultFormat string } func (p PytestPants) Name() string { @@ -20,13 +21,20 @@ func (p PytestPants) Name() string { } func NewPytestPants(c RunnerConfig) PytestPants { - fmt.Fprintln(os.Stderr, "Info: Python package 'buildkite-test-collector' is required and will not be verified by bktec. Please ensure it is added to the pants resolve used by pytest. See https://github.com/buildkite/test-engine-client/blob/main/docs/pytest-pants.md for more information.") - if c.TestCommand == "" { fmt.Fprintln(os.Stderr, "Error: The test command must be set via BUILDKITE_TEST_ENGINE_TEST_CMD.") os.Exit(1) } + var resultFormat string + switch { + case strings.Contains(c.TestCommand, "--junit-xml"): + resultFormat = "junit" + case strings.Contains(c.TestCommand, "--json="): + resultFormat = "json" + fmt.Fprintln(os.Stderr, "Info: Python package 'buildkite-test-collector' is required and will not be verified by bktec. Please ensure it is added to the pants resolve used by pytest. See https://github.com/buildkite/test-engine-client/blob/main/docs/pytest-pants.md for more information.") + } + if c.TestFilePattern != "" || c.TestFileExcludePattern != "" { fmt.Fprintln(os.Stderr, "Warning: Pants test runner variant does not support discovering test files. Please ensure the test command is set correctly via BUILDKITE_TEST_ENGINE_TEST_CMD and do *not* set either:") fmt.Fprintf(os.Stderr, " BUILDKITE_TEST_ENGINE_TEST_FILE_PATTERN=%q\n", c.TestFilePattern) @@ -42,14 +50,28 @@ func NewPytestPants(c RunnerConfig) PytestPants { } if c.ResultPath == "" { - c.ResultPath = getRandomTempFilename("pytest-results.json") + if resultFormat == "junit" { + c.ResultPath = getRandomTempFilename("pytest-results.xml") + } else { + c.ResultPath = getRandomTempFilename("pytest-results.json") + } } return PytestPants{ RunnerConfig: c, + resultFormat: resultFormat, } } +func (p PytestPants) ResultFormat() string { + if p.resultFormat == "junit" { + return "junit" + } + // JSON results are uploaded by buildkite-test-collector directly, so + // returning "" prevents bktec from double-uploading. + return "" +} + func (p PytestPants) SupportedFeatures() SupportedFeatures { return SupportedFeatures{ SplitByFile: false, @@ -82,16 +104,27 @@ func (p PytestPants) Run(result *RunResult, testCases []plan.TestCase, retry boo parseExit2JSON = true } - tests, parseErr := parseTestEngineTestResult(p.ResultPath) + var parseErr error + if p.resultFormat == "junit" { + parseErr = p.runParseJUnit(result) + } else { + parseErr = p.runParseJSON(result, parseExit2JSON, cmdErr) + } if parseErr != nil { - fmt.Printf("Buildkite Test Engine Client: Failed to read json output, failed tests will not be retried: %v\n", parseErr) + fmt.Printf("Buildkite Test Engine Client: Failed to read test output, failed tests will not be retried: %v\n", parseErr) if parseExit2JSON { result.error = cmdErr } - // We don't want to fail the build if we fail to parse the report, - // therefore we return the command error (which can be nil), instead of the parse error. - return cmdErr + } + + return cmdErr +} + +func (p PytestPants) runParseJSON(result *RunResult, parseExit2JSON bool, cmdErr error) error { + tests, parseErr := parseTestEngineTestResult(p.ResultPath) + if parseErr != nil { + return parseErr } for _, test := range tests { @@ -101,8 +134,27 @@ func (p PytestPants) Run(result *RunResult, testCases []plan.TestCase, retry boo result.error = cmdErr } - // Return any command error after processing the report - return cmdErr + return nil +} + +func (p PytestPants) runParseJUnit(result *RunResult) error { + tests, parseErr := loadAndParseJUnitXML(p.ResultPath) + if parseErr != nil { + return parseErr + } + + for _, test := range tests { + scope, path := pytestNodeIDFromJUnit(test.Classname, test.Name) + result.RecordTestResult(plan.TestCase{ + Identifier: path, + Format: plan.TestCaseFormatExample, + Scope: scope, + Name: test.Name, + Path: path, + }, test.Result) + } + + return nil } func (p PytestPants) GetFiles() ([]string, error) { @@ -129,14 +181,22 @@ func (p PytestPants) CommandNameAndArgs(testCases []plan.TestCase, retry bool) ( return "", []string{}, fmt.Errorf("please ensure the test command in BUILDKITE_TEST_ENGINE_TEST_CMD includes a -- separator") } - // Check that both required flags are after the -- afterDash := parts[1] - if !strings.Contains(afterDash, "--json={{resultPath}}") { - return "", []string{}, fmt.Errorf("please ensure the test command in BUILDKITE_TEST_ENGINE_TEST_CMD includes --json={{resultPath}} after the -- separator") - } - if !strings.Contains(afterDash, "--merge-json") { - return "", []string{}, fmt.Errorf("please ensure the test command in BUILDKITE_TEST_ENGINE_TEST_CMD includes --merge-json after the -- separator") + switch p.resultFormat { + case "junit": + if !strings.Contains(afterDash, "--junit-xml={{resultPath}}") { + return "", []string{}, fmt.Errorf("please ensure the test command in BUILDKITE_TEST_ENGINE_TEST_CMD includes --junit-xml={{resultPath}} after the -- separator") + } + case "json": + if !strings.Contains(afterDash, "--json={{resultPath}}") { + return "", []string{}, fmt.Errorf("please ensure the test command in BUILDKITE_TEST_ENGINE_TEST_CMD includes --json={{resultPath}} after the -- separator") + } + if !strings.Contains(afterDash, "--merge-json") { + return "", []string{}, fmt.Errorf("please ensure the test command in BUILDKITE_TEST_ENGINE_TEST_CMD includes --merge-json after the -- separator") + } + default: + return "", []string{}, fmt.Errorf("please ensure the test command in BUILDKITE_TEST_ENGINE_TEST_CMD includes either --junit-xml={{resultPath}} or --json={{resultPath}} after the -- separator") } cmd = strings.Replace(cmd, "{{resultPath}}", p.ResultPath, 1) diff --git a/internal/runner/pytest_pants_test.go b/internal/runner/pytest_pants_test.go index 3cd9a009..c9df6ee8 100644 --- a/internal/runner/pytest_pants_test.go +++ b/internal/runner/pytest_pants_test.go @@ -4,6 +4,7 @@ import ( "errors" "os/exec" "path/filepath" + "strings" "testing" "github.com/buildkite/test-engine-client/v2/internal/plan" @@ -228,6 +229,162 @@ func TestPytestPantsGetExamples(t *testing.T) { } } +func TestPytestPantsRun_JUnit_TestPassed(t *testing.T) { + changeCwd(t, "./testdata/pytest_pants") + + pytest := PytestPants{ + RunnerConfig: RunnerConfig{ + TestCommand: "pants test //passing_test.py -- --junit-xml={{resultPath}}", + ResultPath: "result-passed.xml", + }, + resultFormat: "junit", + } + + testCases := []plan.TestCase{{Path: "passing_test.py"}} + result := NewRunResult([]plan.TestCase{}) + err := pytest.Run(result, testCases, false) + if err != nil { + t.Errorf("PytestPants.Run(%q) error = %v", testCases, err) + } + + if result.Status() != RunStatusPassed { + t.Errorf("PytestPants.Run(%q) RunResult.Status = %v, want %v", testCases, result.Status(), RunStatusPassed) + } +} + +func TestPytestPantsRun_JUnit_TestFailed(t *testing.T) { + changeCwd(t, "./testdata/pytest_pants") + + pytest := PytestPants{ + RunnerConfig: RunnerConfig{ + TestCommand: "pants test //:: -- --junit-xml={{resultPath}}", + ResultPath: "result-failed.xml", + }, + resultFormat: "junit", + } + + testCases := []plan.TestCase{{Path: "failing_test.py"}} + result := NewRunResult([]plan.TestCase{}) + err := pytest.Run(result, testCases, false) + + exitError := new(exec.ExitError) + assert.ErrorAs(t, err, &exitError) + + if result.Status() != RunStatusFailed { + t.Errorf("PytestPants.Run(%q) RunResult.Status = %v, want %v", testCases, result.Status(), RunStatusFailed) + } + + failedTest := result.FailedTests() + if len(failedTest) != 1 { + t.Errorf("len(result.FailedTests()) = %d, want 1", len(failedTest)) + } + + wantFailedTests := []plan.TestCase{ + { + Format: "example", + Identifier: "tests/failing_test.py::test_failed", + Name: "test_failed", + Path: "tests/failing_test.py::test_failed", + Scope: "tests/failing_test.py", + }, + } + + if diff := cmp.Diff(failedTest, wantFailedTests); diff != "" { + t.Errorf("PytestPants.Run(%q) RunResult.FailedTests() diff (-got +want):\n%s", testCases, diff) + } +} + +func TestPytestPantsResultFormat_JUnit(t *testing.T) { + pytest := PytestPants{resultFormat: "junit"} + if got := pytest.ResultFormat(); got != "junit" { + t.Errorf("ResultFormat() = %q, want %q", got, "junit") + } +} + +func TestNewPytestPants_JUnit_DetectsFormatAndXmlPath(t *testing.T) { + pytest := NewPytestPants(RunnerConfig{ + TestCommand: "pants test //:: -- --junit-xml={{resultPath}}", + }) + + if got := pytest.ResultFormat(); got != "junit" { + t.Errorf("ResultFormat() = %q, want %q", got, "junit") + } + + if !strings.HasSuffix(pytest.ResultPath, ".xml") { + t.Errorf("ResultPath = %q, want path ending in .xml", pytest.ResultPath) + } +} + +func TestPytestPantsResultFormat_JSON(t *testing.T) { + pytest := PytestPants{resultFormat: "json"} + if got := pytest.ResultFormat(); got != "" { + t.Errorf("ResultFormat() = %q, want %q", got, "") + } +} + +func TestPytestPantsCommandNameAndArgs_JUnit_ValidCommand(t *testing.T) { + testCases := []plan.TestCase{{Path: "failing_test.py"}, {Path: "passing_test.py"}} + testCommand := "pants test //:: -- --junit-xml={{resultPath}}" + + pytest := PytestPants{ + RunnerConfig: RunnerConfig{ + TestCommand: testCommand, + ResultPath: "result.xml", + }, + resultFormat: "junit", + } + + gotName, gotArgs, err := pytest.CommandNameAndArgs(testCases, false) + if err != nil { + t.Errorf("commandNameAndArgs(%q, %q) error = %v", testCases, testCommand, err) + } + + wantName := "pants" + wantArgs := []string{"test", "//::", "--", "--junit-xml=result.xml"} + + if diff := cmp.Diff(gotName, wantName); diff != "" { + t.Errorf("commandNameAndArgs() diff (-got +want):\n%s", diff) + } + if diff := cmp.Diff(gotArgs, wantArgs); diff != "" { + t.Errorf("commandNameAndArgs() diff (-got +want):\n%s", diff) + } +} + +func TestPytestPantsCommandNameAndArgs_JUnit_WithoutResultPath(t *testing.T) { + testCases := []plan.TestCase{{Path: "failing_test.py"}} + testCommand := "pants test //:: -- --merge-json" + + pytest := PytestPants{ + RunnerConfig: RunnerConfig{ + TestCommand: testCommand, + ResultPath: "result.xml", + }, + resultFormat: "junit", + } + + _, _, err := pytest.CommandNameAndArgs(testCases, false) + if err == nil { + t.Error("commandNameAndArgs() error = nil, want error") + } +} + +func TestPytestPantsCommandNameAndArgs_NeitherResultFlag(t *testing.T) { + testCases := []plan.TestCase{{Path: "failing_test.py"}} + + pytest := PytestPants{ + RunnerConfig: RunnerConfig{ + TestCommand: "pants test //:: -- --some-other-flag", + ResultPath: "result.json", + }, + resultFormat: "", + } + + _, _, err := pytest.CommandNameAndArgs(testCases, false) + if err == nil { + t.Error("commandNameAndArgs() error = nil, want error") + } +} + func TestPytestPantsCommandNameAndArgs_WithoutMergeJson(t *testing.T) { testCases := []plan.TestCase{{Path: "failing_test.py"}, {Path: "passing_test.py"}} testCommand := "pants test //:: -- --json={{resultPath}}" diff --git a/internal/runner/testdata/pytest_pants/result-failed.xml b/internal/runner/testdata/pytest_pants/result-failed.xml new file mode 100644 index 00000000..ff502f8b --- /dev/null +++ b/internal/runner/testdata/pytest_pants/result-failed.xml @@ -0,0 +1,8 @@ + + + + + AssertionError: assert 3 == 5 + + + diff --git a/internal/runner/testdata/pytest_pants/result-passed.xml b/internal/runner/testdata/pytest_pants/result-passed.xml new file mode 100644 index 00000000..fedf0614 --- /dev/null +++ b/internal/runner/testdata/pytest_pants/result-passed.xml @@ -0,0 +1,6 @@ + + + + + +