diff --git a/cli.go b/cli.go index 2f04a482..c488e350 100644 --- a/cli.go +++ b/cli.go @@ -258,6 +258,14 @@ var failOnNoTestsFlag = &cli.BoolFlag{ Destination: &cfg.FailOnNoTests, } +var locationPrefixFlag = &cli.StringFlag{ + Name: "location-prefix", + Category: "TEST RUNNER", + Usage: "Prefix to prepend to test file paths when requesting a test plan, used to match the test files reported by the test collector", + Sources: cli.EnvVars("BUILDKITE_TEST_ENGINE_LOCATION_PREFIX", "BUILDKITE_ANALYTICS_LOCATION_PREFIX"), + Destination: &cfg.LocationPrefix, +} + // Test Runner Retry Flags var testEngineRetryCountFlag = &cli.IntFlag{ Name: "test-engine-retry-count", @@ -376,6 +384,7 @@ func runCommandFlags() []cli.Flag { resultPathFlag, splitByExampleFlag, failOnNoTestsFlag, + locationPrefixFlag, // Runner Retry Flags disableRetryMutedFlag, retryCommandFlag, @@ -413,6 +422,7 @@ func planCommandFlags() []cli.Flag { testRunnerFlag, resultPathFlag, splitByExampleFlag, + locationPrefixFlag, // Runner Retry Flags disableRetryMutedFlag, retryCommandFlag, diff --git a/docs/jest.md b/docs/jest.md index a59e8fb6..24495ec5 100644 --- a/docs/jest.md +++ b/docs/jest.md @@ -46,6 +46,19 @@ export BUILDKITE_TEST_ENGINE_TEST_FILE_EXCLUDE_PATTERN=src/components > [!TIP] > This option accepts the pattern syntax supported by the [zzglob](https://github.com/DrJosh9000/zzglob?tab=readme-ov-file#pattern-syntax) library. +## Location prefix +If you have configured the [Buildkite test collector](https://buildkite.com/docs/test-engine/test-collection) with a location prefix, you should set the same prefix for bktec so that test file paths match those reported by the collector. You can set this using the `--location-prefix` flag or the `BUILDKITE_TEST_ENGINE_LOCATION_PREFIX` environment variable. + +```sh +bktec run --location-prefix "my/prefix/" +``` + +Or using the environment variable: + +```sh +export BUILDKITE_TEST_ENGINE_LOCATION_PREFIX=my/prefix/ +``` + ## Automatically retry failed tests You can configure bktec to automatically retry failed tests using the `BUILDKITE_TEST_ENGINE_RETRY_COUNT` environment variable. When this variable is set to a number greater than `0`, bktec will retry each failed test up to the specified number of times, using the following command: diff --git a/docs/playwright.md b/docs/playwright.md index 4db3e62f..c591103c 100644 --- a/docs/playwright.md +++ b/docs/playwright.md @@ -56,6 +56,19 @@ export BUILDKITE_TEST_ENGINE_TEST_FILE_EXCLUDE_PATTERN=src/components > [!TIP] > This option accepts the pattern syntax supported by the [zzglob](https://github.com/DrJosh9000/zzglob?tab=readme-ov-file#pattern-syntax) library. +## Location prefix +If you have configured the [Buildkite test collector](https://buildkite.com/docs/test-engine/test-collection) with a location prefix, you should set the same prefix for bktec so that test file paths match those reported by the collector. You can set this using the `--location-prefix` flag or the `BUILDKITE_TEST_ENGINE_LOCATION_PREFIX` environment variable. + +```sh +bktec run --location-prefix "my/prefix/" +``` + +Or using the environment variable: + +```sh +export BUILDKITE_TEST_ENGINE_LOCATION_PREFIX=my/prefix/ +``` + ## Automatically retry failed tests You can configure bktec to automatically retry failed tests using the `BUILDKITE_TEST_ENGINE_RETRY_COUNT` environment variable. When this variable is set to a number greater than `0`, bktec will retry each failed test up to the specified number of times, using either the default test command or the command specified in `BUILDKITE_TEST_ENGINE_TEST_CMD`. diff --git a/docs/rspec.md b/docs/rspec.md index 5f6ba858..5626aa65 100644 --- a/docs/rspec.md +++ b/docs/rspec.md @@ -64,6 +64,19 @@ export BUILDKITE_TEST_ENGINE_TEST_FILE_EXCLUDE_PATTERN=spec/models/user > [!TIP] > This option accepts the pattern syntax supported by the [zzglob](https://github.com/DrJosh9000/zzglob?tab=readme-ov-file#pattern-syntax) library. +## Location prefix +If you have configured the [Buildkite test collector](https://buildkite.com/docs/test-engine/test-collection) with a location prefix, you should set the same prefix for bktec so that test file paths match those reported by the collector. You can set this using the `--location-prefix` flag or the `BUILDKITE_TEST_ENGINE_LOCATION_PREFIX` environment variable. + +```sh +bktec run --location-prefix "my/prefix/" +``` + +Or using the environment variable: + +```sh +export BUILDKITE_TEST_ENGINE_LOCATION_PREFIX=my/prefix/ +``` + ## Automatically retry failed tests You can configure bktec to automatically retry failed tests using the `BUILDKITE_TEST_ENGINE_RETRY_COUNT` environment variable. When this variable is set to a number greater than `0`, bktec will retry each failed test up to the specified number of times, using the command set in `BUILDKITE_TEST_ENGINE_RETRY_CMD` environment variable. If this variable is not set, bktec will use either the default test command or the command specified in `BUILDKITE_TEST_ENGINE_TEST_CMD` to retry the tests. diff --git a/internal/command/path.go b/internal/command/path.go new file mode 100644 index 00000000..5308e0d4 --- /dev/null +++ b/internal/command/path.go @@ -0,0 +1,35 @@ +package command + +import ( + "fmt" + "path/filepath" +) + +// prefixFilePaths prepends the given prefix to the file paths of the test cases. +func prefixPath(path string, prefix string) string { + if prefix == "" { + return path + } + + var prefixedPath string + // Some test collectors (e.g. Rspec) report file paths with a "./" by default. + // Since `filepath.Join` ignore "./", we need to handle this case separately to avoid losing the "./" prefix. + if prefix == "./" { + prefixedPath = prefix + path + } else { + prefixedPath = filepath.Join(prefix, path) + } + return prefixedPath +} + +func trimFilePathPrefix(path string, prefix string) (string, error) { + if prefix == "" { + return path, nil + } + + relPath, err := filepath.Rel(prefix, path) + if err != nil { + return "", fmt.Errorf("failed to trim prefix from file path: %w", err) + } + return relPath, nil +} diff --git a/internal/command/path_test.go b/internal/command/path_test.go new file mode 100644 index 00000000..b40f5aaf --- /dev/null +++ b/internal/command/path_test.go @@ -0,0 +1,90 @@ +package command + +import ( + "testing" + + "github.com/google/go-cmp/cmp" +) + +func TestPrefixFilePath(t *testing.T) { + path := "spec/models/user_spec.rb" + + cases := []struct { + prefix string + expected string + }{ + { + prefix: "", + expected: "spec/models/user_spec.rb", + }, + { + prefix: "my/project", + expected: "my/project/spec/models/user_spec.rb", + }, + { + prefix: "/home/user/my/project", + expected: "/home/user/my/project/spec/models/user_spec.rb", + }, + { + prefix: "./", + expected: "./spec/models/user_spec.rb", + }, + } + + for _, c := range cases { + t.Run(c.prefix, func(t *testing.T) { + got := prefixPath(path, c.prefix) + if diff := cmp.Diff(got, c.expected); diff != "" { + t.Errorf("prefixPath() diff (-got +want):\n%s", diff) + } + }) + } +} + +func TestTrimFilePathPrefix(t *testing.T) { + cases := []struct { + prefix string + filePath string + expected string + }{ + { + prefix: "my/project", + filePath: "my/project/spec/models/user_spec.rb", + expected: "spec/models/user_spec.rb", + }, + { + filePath: "/home/user/my/project/spec/models/user_spec.rb", + prefix: "/home/user/my/project", + expected: "spec/models/user_spec.rb", + }, + { + filePath: "./spec/models/user_spec.rb", + prefix: "./", + expected: "spec/models/user_spec.rb", + }, + } + + for _, c := range cases { + t.Run(c.prefix, func(t *testing.T) { + got, err := trimFilePathPrefix(c.filePath, c.prefix) + if err != nil { + t.Errorf("trimFilePathPrefix() error = %v", err) + } + + if diff := cmp.Diff(got, c.expected); diff != "" { + t.Errorf("trimFilePathPrefix() diff (-got +want):\n%s", diff) + } + }) + } +} + +func TestTrimFilePathPrefix_Error(t *testing.T) { + path := "spec/foo.rb" + got, err := trimFilePathPrefix(path, "/absolute/path") + if err == nil { + t.Errorf("expected error, got nil") + } + if got != "" { + t.Errorf("expected empty string, got %q", got) + } +} diff --git a/internal/command/request_param.go b/internal/command/request_param.go index 66ed0e5f..5fb1d0e8 100644 --- a/internal/command/request_param.go +++ b/internal/command/request_param.go @@ -16,13 +16,17 @@ import ( // that are slow or contain skipped tests. These files are then split into examples // The remaining files are sent as is. // +// If location prefix is configured, the file paths are prefixed when making the request to the Test Engine API, +// so that it can correctly identify the files. +// // If tag filtering is enabled, all files are split into examples to support filtering. // Currently only the Pytest runner supports tag filtering. func createRequestParam(ctx context.Context, cfg *config.Config, files []string, client api.Client, runner TestRunner) (api.TestPlanParams, error) { testFiles := []plan.TestCase{} + for _, file := range files { testFiles = append(testFiles, plan.TestCase{ - Path: file, + Path: prefixPath(file, runner.LocationPrefix()), }) } @@ -100,6 +104,39 @@ func buildSelectionParams(strategy string, params map[string]string) *api.Select } } +func getExamplesWithPrefix(filePaths []string, runner TestRunner) ([]plan.TestCase, error) { + prefix := runner.LocationPrefix() + trimmedPaths := make([]string, len(filePaths)) + + // runner.GetExamples will call the test runner with the file paths. + // Because the test runner expects the file paths without the prefix (it doesn't know about the prefix), + // we need to trim the prefix before passing the file paths to the runner. + for i, filePath := range filePaths { + path, err := trimFilePathPrefix(filePath, prefix) + if err != nil { + return nil, fmt.Errorf("trim file path prefix: %w", err) + } + trimmedPaths[i] = path + } + + examples, err := runner.GetExamples(trimmedPaths) + if err != nil { + return nil, fmt.Errorf("get examples: %w", err) + } + + // After getting the examples from the runner, we need to re-apply the prefix to the example paths + // before sending them to the Test Engine API. + if prefix != "" { + for i := range examples { + // The 'Identifier' field in an example may not always be a file path. + // Since the Test Engine API only uses the 'Path' field, we only apply the prefix to 'Path'. + examples[i].Path = prefixPath(examples[i].Path, prefix) + } + } + + return examples, nil +} + // Splits all the test files into examples to support tag filtering. func splitAllFiles(files []plan.TestCase, runner TestRunner) (api.TestPlanParamsTest, error) { debug.Printf("Splitting all %d files", len(files)) @@ -108,9 +145,9 @@ func splitAllFiles(files []plan.TestCase, runner TestRunner) (api.TestPlanParams filePaths = append(filePaths, file.Path) } - examples, err := runner.GetExamples(filePaths) + examples, err := getExamplesWithPrefix(filePaths, runner) if err != nil { - return api.TestPlanParamsTest{}, fmt.Errorf("get examples: %w", err) + return api.TestPlanParamsTest{}, err } debug.Printf("Got %d examples from all files", len(examples)) @@ -121,24 +158,24 @@ func splitAllFiles(files []plan.TestCase, runner TestRunner) (api.TestPlanParams } // filterAndSplitFiles filters the test files through the Test Engine API and splits the filtered files into examples. -// It returns the test plan parameters with the examples from the filtered files and the remaining files. +// It returns the test plan parameters with the examples from the filtered files and the remaining files that are not filtered. // An error is returned if there is a failure in any of the process. -func filterAndSplitFiles(ctx context.Context, cfg *config.Config, client api.Client, files []plan.TestCase, runner TestRunner) (api.TestPlanParamsTest, error) { +func filterAndSplitFiles(ctx context.Context, cfg *config.Config, client api.Client, allTestFiles []plan.TestCase, runner TestRunner) (api.TestPlanParamsTest, error) { // Filter files that need to be split. - debug.Printf("Filtering %d files", len(files)) + debug.Printf("Filtering %d files", len(allTestFiles)) filteredFiles, err := client.FilterTests(ctx, cfg.SuiteSlug, api.FilterTestsParams{ - Files: files, + Files: allTestFiles, Env: cfg, }) if err != nil { return api.TestPlanParamsTest{}, fmt.Errorf("filter tests: %w", err) } - // If no files are filtered, return the all files. + // If no files are filtered, return all the files. if len(filteredFiles) == 0 { debug.Println("No filtered files found") return api.TestPlanParamsTest{ - Files: files, + Files: allTestFiles, }, nil } @@ -152,16 +189,19 @@ func filterAndSplitFiles(ctx context.Context, cfg *config.Config, client api.Cli filteredFilesPath = append(filteredFilesPath, file.Path) } - examples, err := runner.GetExamples(filteredFilesPath) + // The filtered files returned by the API include the location prefix in their paths, + // so we should trim the prefix before passing the file paths to the runner to get the examples, + // then re-apply the prefix to the example paths collected by the runner. + examples, err := getExamplesWithPrefix(filteredFilesPath, runner) if err != nil { - return api.TestPlanParamsTest{}, fmt.Errorf("get examples: %w", err) + return api.TestPlanParamsTest{}, err } debug.Printf("Got %d examples within the filtered files", len(examples)) // Get the remaining files that are not filtered. - remainingFiles := make([]plan.TestCase, 0, len(files)-len(filteredFiles)) - for _, file := range files { + remainingFiles := make([]plan.TestCase, 0, len(allTestFiles)-len(filteredFiles)) + for _, file := range allTestFiles { if _, ok := filteredFilesMap[file.Path]; !ok { remainingFiles = append(remainingFiles, file) } diff --git a/internal/command/request_param_test.go b/internal/command/request_param_test.go new file mode 100644 index 00000000..c57f970e --- /dev/null +++ b/internal/command/request_param_test.go @@ -0,0 +1,747 @@ +package command + +import ( + "context" + "fmt" + "net/http" + "net/http/httptest" + "testing" + + "github.com/buildkite/test-engine-client/internal/api" + "github.com/buildkite/test-engine-client/internal/config" + "github.com/buildkite/test-engine-client/internal/plan" + "github.com/buildkite/test-engine-client/internal/runner" + "github.com/google/go-cmp/cmp" +) + +func TestCreateRequestParams(t *testing.T) { + svr := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + fmt.Fprint(w, ` +{ + "tests": [ + { "path": "testdata/rspec/spec/fruits/banana_spec.rb", "reason": "slow file" }, + { "path": "testdata/rspec/spec/fruits/fig_spec.rb", "reason": "slow file" } + ] +}`) + })) + defer svr.Close() + + cfg := config.Config{ + OrganizationSlug: "my-org", + SuiteSlug: "my-suite", + Identifier: "identifier", + Parallelism: 7, + Branch: "", + TestRunner: "rspec", + } + + client := api.NewClient(api.ClientConfig{ + ServerBaseUrl: svr.URL, + }) + files := []string{ + "testdata/rspec/spec/fruits/apple_spec.rb", + "testdata/rspec/spec/fruits/banana_spec.rb", + "testdata/rspec/spec/fruits/cherry_spec.rb", + "testdata/rspec/spec/fruits/dragonfruit_spec.rb", + "testdata/rspec/spec/fruits/elderberry_spec.rb", + "testdata/rspec/spec/fruits/fig_spec.rb", + "testdata/rspec/spec/fruits/grape_spec.rb", + } + + got, err := createRequestParam(context.Background(), &cfg, files, *client, runner.Rspec{ + RunnerConfig: runner.RunnerConfig{ + TestCommand: "rspec", + }, + }) + if err != nil { + t.Errorf("createRequestParam() error = %v", err) + } + + // filtered files: banana_spec.rb, fig_spec.rb + // the rest: apple_spec.rb, cherry_spec.rb, dragonfruit_spec.rb, elderberry_spec.rb, grape_spec.rb + want := api.TestPlanParams{ + Identifier: "identifier", + Parallelism: 7, + Branch: "", + Runner: "rspec", + Tests: api.TestPlanParamsTest{ + Files: []plan.TestCase{ + {Path: "testdata/rspec/spec/fruits/apple_spec.rb"}, + {Path: "testdata/rspec/spec/fruits/cherry_spec.rb"}, + {Path: "testdata/rspec/spec/fruits/dragonfruit_spec.rb"}, + {Path: "testdata/rspec/spec/fruits/elderberry_spec.rb"}, + {Path: "testdata/rspec/spec/fruits/grape_spec.rb"}, + }, + Examples: []plan.TestCase{ + { + Identifier: "./testdata/rspec/spec/fruits/banana_spec.rb[1:1]", + Name: "is yellow", + Path: "./testdata/rspec/spec/fruits/banana_spec.rb[1:1]", + Scope: "Banana", + }, + { + Identifier: "./testdata/rspec/spec/fruits/banana_spec.rb[1:2:1]", + Name: "is green", + Path: "./testdata/rspec/spec/fruits/banana_spec.rb[1:2:1]", + Scope: "Banana when not ripe", + }, + { + Identifier: "./testdata/rspec/spec/fruits/fig_spec.rb[1:1]", + Name: "is purple", + Path: "./testdata/rspec/spec/fruits/fig_spec.rb[1:1]", + Scope: "Fig", + }, + }, + }, + } + + if diff := cmp.Diff(got, want); diff != "" { + t.Errorf("createRequestParam() diff (-got +want):\n%s", diff) + } +} + +func TestCreateRequestParams_NonRSpec(t *testing.T) { + svr := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + fmt.Fprint(w, ` +{ + "tests": [ + { "path": "testdata/jest/banana.spec.js", "reason": "slow file" }, + { "path": "testdata/jest/fig.spec.js", "reason": "slow file" } + ] +}`) + })) + defer svr.Close() + + runners := []TestRunner{ + runner.Jest{}, runner.Playwright{}, runner.Cypress{}, + } + + for _, r := range runners { + t.Run(r.Name(), func(t *testing.T) { + cfg := config.Config{ + OrganizationSlug: "my-org", + SuiteSlug: "my-suite", + Identifier: "identifier", + Parallelism: 7, + Branch: "", + TestRunner: r.Name(), + } + + client := api.NewClient(api.ClientConfig{ + ServerBaseUrl: svr.URL, + }) + files := []string{ + "testdata/fruits/apple.spec.js", + "testdata/fruits/banana.spec.js", + "testdata/fruits/cherry.spec.js", + } + + got, err := createRequestParam(context.Background(), &cfg, files, *client, r) + if err != nil { + t.Errorf("createRequestParam() error = %v", err) + } + + want := api.TestPlanParams{ + Identifier: "identifier", + Parallelism: 7, + Branch: "", + Runner: r.Name(), + Tests: api.TestPlanParamsTest{ + Files: []plan.TestCase{ + {Path: "testdata/fruits/apple.spec.js"}, + {Path: "testdata/fruits/banana.spec.js"}, + {Path: "testdata/fruits/cherry.spec.js"}, + }, + }, + } + + if diff := cmp.Diff(got, want); diff != "" { + t.Errorf("createRequestParam() diff (-got +want):\n%s", diff) + } + }) + } +} + +func TestCreateRequestParams_PytestPants(t *testing.T) { + svr := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + fmt.Fprint(w, ` +{ + "tests": [ + { "path": "test/banana_test.py", "reason": "slow file" }, + { "path": "test/fig_test.py", "reason": "slow file" } + ] +}`) + })) + defer svr.Close() + + runner := runner.PytestPants{} + + t.Run(runner.Name(), func(t *testing.T) { + cfg := config.Config{ + OrganizationSlug: "my-org", + SuiteSlug: "my-suite", + Identifier: "identifier", + Parallelism: 7, + Branch: "", + TestRunner: runner.Name(), + } + + client := api.NewClient(api.ClientConfig{ + ServerBaseUrl: svr.URL, + }) + files := []string{ + "test/apple_test.py", + "test/banana_test.py", + "test/cherry_test.py", + } + + got, err := createRequestParam(context.Background(), &cfg, files, *client, runner) + if err != nil { + t.Errorf("createRequestParam() error = %v", err) + } + + want := api.TestPlanParams{ + Identifier: "identifier", + Parallelism: 7, + Branch: "", + Runner: "pytest", + Tests: api.TestPlanParamsTest{ + Files: []plan.TestCase{ + {Path: "test/apple_test.py"}, + {Path: "test/banana_test.py"}, + {Path: "test/cherry_test.py"}, + }, + }, + } + + if diff := cmp.Diff(got, want); diff != "" { + t.Errorf("createRequestParam() diff (-got +want):\n%s", diff) + } + }) +} + +func TestCreateRequestParams_WithSelectionAndMetadata_NonRSpec(t *testing.T) { + cfg := config.Config{ + Identifier: "identifier", + Parallelism: 2, + Branch: "main", + TestRunner: "jest", + SelectionStrategy: "least-reliable", + SelectionParams: map[string]string{ + "top": "100", + }, + Metadata: map[string]string{ + "git_diff": "line1\nline2", + "source": "cli", + }, + } + + client := api.NewClient(api.ClientConfig{ + ServerBaseUrl: "http://example.com", + }) + + files := []string{ + "testdata/fruits/apple.spec.js", + "testdata/fruits/banana.spec.js", + } + + got, err := createRequestParam(context.Background(), &cfg, files, *client, runner.Jest{}) + if err != nil { + t.Errorf("createRequestParam() error = %v", err) + } + + want := api.TestPlanParams{ + Identifier: "identifier", + Parallelism: 2, + Branch: "main", + Runner: "jest", + Selection: &api.SelectionParams{ + Strategy: "least-reliable", + Params: map[string]string{ + "top": "100", + }, + }, + Metadata: map[string]string{ + "git_diff": "line1\nline2", + "source": "cli", + }, + Tests: api.TestPlanParamsTest{ + Files: []plan.TestCase{ + {Path: "testdata/fruits/apple.spec.js"}, + {Path: "testdata/fruits/banana.spec.js"}, + }, + }, + } + + if diff := cmp.Diff(got, want); diff != "" { + t.Errorf("createRequestParam() diff (-got +want):\n%s", diff) + } +} + +func TestCreateRequestParams_WithSelectionAndMetadata_SplitAllFilesBranch(t *testing.T) { + cfg := config.Config{ + Identifier: "identifier", + Parallelism: 2, + Branch: "main", + TestRunner: "pytest", + TagFilters: "team:frontend", + SelectionStrategy: "percent", + SelectionParams: map[string]string{ + "percent": "40", + }, + Metadata: map[string]string{ + "git_diff": "line1\nline2", + }, + } + + client := api.NewClient(api.ClientConfig{ + ServerBaseUrl: "http://example.com", + }) + + files := []string{ + "test_sample.py", + } + + stubRunner := metadataTestRunner{ + name: "pytest", + examples: []plan.TestCase{ + { + Identifier: "test_sample.py::test_happy", + Path: "test_sample.py::test_happy", + Scope: "test_sample.py", + Name: "test_happy", + Format: plan.TestCaseFormatExample, + }, + }, + } + + got, err := createRequestParam(context.Background(), &cfg, files, *client, stubRunner) + if err != nil { + t.Errorf("createRequestParam() error = %v", err) + } + + want := api.TestPlanParams{ + Identifier: "identifier", + Parallelism: 2, + Branch: "main", + Runner: "pytest", + Selection: &api.SelectionParams{ + Strategy: "percent", + Params: map[string]string{ + "percent": "40", + }, + }, + Metadata: map[string]string{ + "git_diff": "line1\nline2", + }, + Tests: api.TestPlanParamsTest{ + Examples: []plan.TestCase{ + { + Identifier: "test_sample.py::test_happy", + Path: "test_sample.py::test_happy", + Scope: "test_sample.py", + Name: "test_happy", + Format: plan.TestCaseFormatExample, + }, + }, + }, + } + + if diff := cmp.Diff(got, want); diff != "" { + t.Errorf("createRequestParam() diff (-got +want):\n%s", diff) + } +} + +type metadataTestRunner struct { + name string + examples []plan.TestCase +} + +func (r metadataTestRunner) Name() string { + return r.name +} + +func (r metadataTestRunner) GetExamples(files []string) ([]plan.TestCase, error) { + return r.examples, nil +} + +func (r metadataTestRunner) GetFiles() ([]string, error) { + return nil, nil +} + +func (r metadataTestRunner) LocationPrefix() string { + return "" +} + +func (r metadataTestRunner) Run(result *runner.RunResult, testCases []plan.TestCase, retry bool) error { + return nil +} + +func TestCreateRequestParams_FilterTestsError(t *testing.T) { + svr := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Error(w, `{ "message": "forbidden" }`, http.StatusForbidden) + })) + + defer svr.Close() + + cfg := config.Config{ + OrganizationSlug: "my-org", + SuiteSlug: "my-suite", + Identifier: "identifier", + Parallelism: 7, + Branch: "", + SplitByExample: true, + } + + client := api.NewClient(api.ClientConfig{ + ServerBaseUrl: svr.URL, + }) + files := []string{ + "apple_spec.rb", + "banana_spec.rb", + "cherry_spec.rb", + "dragonfruit_spec.rb", + "elderberry_spec.rb", + "fig_spec.rb", + "grape_spec.rb", + } + + _, err := createRequestParam(context.Background(), &cfg, files, *client, runner.Rspec{}) + + if err.Error() != "filter tests: forbidden" { + t.Errorf("createRequestParam() error = %v, want forbidden error", err) + } +} + +func TestCreateRequestParams_NoFilteredFiles(t *testing.T) { + svr := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + fmt.Fprint(w, ` +{ + "files": [] +}`) + })) + defer svr.Close() + + cfg := config.Config{ + OrganizationSlug: "my-org", + SuiteSlug: "my-suite", + Identifier: "identifier", + Parallelism: 7, + Branch: "", + SplitByExample: true, + } + + client := api.NewClient(api.ClientConfig{ + ServerBaseUrl: svr.URL, + }) + files := []string{ + "testdata/rspec/spec/fruits/apple_spec.rb", + "testdata/rspec/spec/fruits/banana_spec.rb", + "testdata/rspec/spec/fruits/cherry_spec.rb", + "testdata/rspec/spec/fruits/dragonfruit_spec.rb", + "testdata/rspec/spec/fruits/elderberry_spec.rb", + "testdata/rspec/spec/fruits/fig_spec.rb", + "testdata/rspec/spec/fruits/grape_spec.rb", + } + + got, err := createRequestParam(context.Background(), &cfg, files, *client, runner.Rspec{ + RunnerConfig: runner.RunnerConfig{ + TestCommand: "rspec", + }, + }) + if err != nil { + t.Errorf("createRequestParam() error = %v", err) + } + + want := api.TestPlanParams{ + Identifier: "identifier", + Parallelism: 7, + Branch: "", + Tests: api.TestPlanParamsTest{ + Files: []plan.TestCase{ + {Path: "testdata/rspec/spec/fruits/apple_spec.rb"}, + {Path: "testdata/rspec/spec/fruits/banana_spec.rb"}, + {Path: "testdata/rspec/spec/fruits/cherry_spec.rb"}, + {Path: "testdata/rspec/spec/fruits/dragonfruit_spec.rb"}, + {Path: "testdata/rspec/spec/fruits/elderberry_spec.rb"}, + {Path: "testdata/rspec/spec/fruits/fig_spec.rb"}, + {Path: "testdata/rspec/spec/fruits/grape_spec.rb"}, + }, + }, + } + + if diff := cmp.Diff(got, want); diff != "" { + t.Errorf("createRequestParam() diff (-got +want):\n%s", diff) + } +} + +func TestCreateRequestParams_WithTagFilters(t *testing.T) { + cfg := config.Config{ + OrganizationSlug: "my-org", + SuiteSlug: "my-suite", + Identifier: "identifier", + Parallelism: 2, + Branch: "main", + TestRunner: "pytest", + TagFilters: "team:frontend", + } + + client := api.NewClient(api.ClientConfig{ + ServerBaseUrl: "example.com", + }) + + files := []string{ + "../runner/testdata/pytest/failed_test.py", + "../runner/testdata/pytest/test_sample.py", + "../runner/testdata/pytest/spells/test_expelliarmus.py", + } + + got, err := createRequestParam(context.Background(), &cfg, files, *client, runner.Pytest{ + RunnerConfig: runner.RunnerConfig{ + TestCommand: "pytest", + TagFilters: "team:frontend", + }, + }) + if err != nil { + t.Errorf("createRequestParam() error = %v", err) + } + + want := api.TestPlanParams{ + Identifier: "identifier", + Parallelism: 2, + Branch: "main", + Runner: "pytest", + Tests: api.TestPlanParamsTest{ + Examples: []plan.TestCase{ + { + Format: "example", + Identifier: "runner/testdata/pytest/test_sample.py::test_happy", + Name: "test_happy", + Path: "runner/testdata/pytest/test_sample.py::test_happy", + Scope: "runner/testdata/pytest/test_sample.py", + }, + { + Format: "example", + Identifier: "runner/testdata/pytest/spells/test_expelliarmus.py::TestExpelliarmus::test_knocks_wand_out", + Name: "test_knocks_wand_out", + Path: "runner/testdata/pytest/spells/test_expelliarmus.py::TestExpelliarmus::test_knocks_wand_out", + Scope: "runner/testdata/pytest/spells/test_expelliarmus.py::TestExpelliarmus", + }, + }, + }, + } + + if diff := cmp.Diff(got, want); diff != "" { + t.Errorf("createRequestParam() diff (-got +want):\n%s", diff) + } +} + +func TestCreateRequestParams_WithTagFilters_NonPytest(t *testing.T) { + svr := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + fmt.Fprint(w, ` +{ + "tests": [] +}`) + })) + defer svr.Close() + + cfg := config.Config{ + OrganizationSlug: "my-org", + SuiteSlug: "my-suite", + Identifier: "identifier", + Parallelism: 2, + Branch: "main", + TestRunner: "rspec", + TagFilters: "team:frontend", + } + + client := api.NewClient(api.ClientConfig{ + ServerBaseUrl: svr.URL, + }) + + files := []string{ + "testdata/rspec/spec/fruits/apple_spec.rb", + "testdata/rspec/spec/fruits/banana_spec.rb", + } + + got, err := createRequestParam(context.Background(), &cfg, files, *client, runner.Rspec{ + RunnerConfig: runner.RunnerConfig{ + TestCommand: "rspec", + }, + }) + if err != nil { + t.Errorf("createRequestParam() error = %v", err) + } + + want := api.TestPlanParams{ + Identifier: "identifier", + Parallelism: 2, + Branch: "main", + Runner: "rspec", + Tests: api.TestPlanParamsTest{ + Files: []plan.TestCase{ + {Path: "testdata/rspec/spec/fruits/apple_spec.rb"}, + {Path: "testdata/rspec/spec/fruits/banana_spec.rb"}, + }, + }, + } + + if diff := cmp.Diff(got, want); diff != "" { + t.Errorf("createRequestParam() diff (-got +want):\n%s", diff) + } +} + +func TestCreateRequestParams_WithLocationPrefix(t *testing.T) { + svr := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + fmt.Fprint(w, ` + { + "tests": [] + }`) + })) + defer svr.Close() + + client := api.NewClient(api.ClientConfig{ + ServerBaseUrl: svr.URL, + }) + + cfg := config.Config{ + OrganizationSlug: "my-org", + SuiteSlug: "my-suite", + Identifier: "identifier", + Parallelism: 7, + Branch: "", + TestRunner: "rspec", + } + + files := []string{ + "testdata/rspec/spec/fruits/apple_spec.rb", + "testdata/rspec/spec/fruits/banana_spec.rb", + "testdata/rspec/spec/fruits/cherry_spec.rb", + } + + cases := []struct { + prefix string + wantFiles []plan.TestCase + }{ + { + prefix: "./", + wantFiles: []plan.TestCase{ + {Path: "./testdata/rspec/spec/fruits/apple_spec.rb"}, + {Path: "./testdata/rspec/spec/fruits/banana_spec.rb"}, + {Path: "./testdata/rspec/spec/fruits/cherry_spec.rb"}, + }, + }, + { + prefix: "monorepo/project-abc", + wantFiles: []plan.TestCase{ + {Path: "monorepo/project-abc/testdata/rspec/spec/fruits/apple_spec.rb"}, + {Path: "monorepo/project-abc/testdata/rspec/spec/fruits/banana_spec.rb"}, + {Path: "monorepo/project-abc/testdata/rspec/spec/fruits/cherry_spec.rb"}, + }, + }, + } + + for _, c := range cases { + t.Run(fmt.Sprintf("location prefix: %s", c.prefix), func(t *testing.T) { + cfg.LocationPrefix = c.prefix + runner, err := runner.DetectRunner(&cfg) + if err != nil { + t.Fatalf("DetectRunner() error = %v", err) + } + + got, err := createRequestParam(context.Background(), &cfg, files, *client, runner) + if err != nil { + t.Errorf("createRequestParam() error = %v", err) + } + + want := api.TestPlanParams{ + Identifier: "identifier", + Parallelism: 7, + Branch: "", + Runner: "rspec", + Tests: api.TestPlanParamsTest{ + Files: c.wantFiles, + }, + } + + if diff := cmp.Diff(got, want); diff != "" { + t.Errorf("createRequestParam() diff (-got +want):\n%s", diff) + } + }) + } +} + +func TestCreateRequestParams_WithLocationPrefix_SplitByExample(t *testing.T) { + svr := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + fmt.Fprint(w, ` +{ + "tests": [ + { "path": "my/project/testdata/rspec/spec/fruits/banana_spec.rb", "reason": "slow file" } + ] +}`) + })) + defer svr.Close() + + cfg := config.Config{ + OrganizationSlug: "my-org", + SuiteSlug: "my-suite", + Identifier: "identifier", + Parallelism: 7, + Branch: "", + TestRunner: "rspec", + TestCommand: "rspec", + LocationPrefix: "my/project", + } + + runner, err := runner.DetectRunner(&cfg) + if err != nil { + t.Fatalf("DetectRunner() error = %v", err) + } + + client := api.NewClient(api.ClientConfig{ + ServerBaseUrl: svr.URL, + }) + files := []string{ + "testdata/rspec/spec/fruits/apple_spec.rb", + "testdata/rspec/spec/fruits/banana_spec.rb", + "testdata/rspec/spec/fruits/cherry_spec.rb", + } + + got, err := createRequestParam(context.Background(), &cfg, files, *client, runner) + if err != nil { + t.Errorf("createRequestParam() error = %v", err) + } + + // filtered files: banana_spec.rb + // the rest: apple_spec.rb, cherry_spec.rb + want := api.TestPlanParams{ + Identifier: "identifier", + Parallelism: 7, + Branch: "", + Runner: "rspec", + Tests: api.TestPlanParamsTest{ + Files: []plan.TestCase{ + {Path: "my/project/testdata/rspec/spec/fruits/apple_spec.rb"}, + {Path: "my/project/testdata/rspec/spec/fruits/cherry_spec.rb"}, + }, + Examples: []plan.TestCase{ + { + Identifier: "./testdata/rspec/spec/fruits/banana_spec.rb[1:1]", + Name: "is yellow", + Path: "my/project/testdata/rspec/spec/fruits/banana_spec.rb[1:1]", + Scope: "Banana", + }, + { + Identifier: "./testdata/rspec/spec/fruits/banana_spec.rb[1:2:1]", + Name: "is green", + Path: "my/project/testdata/rspec/spec/fruits/banana_spec.rb[1:2:1]", + Scope: "Banana when not ripe", + }, + }, + }, + } + + if diff := cmp.Diff(got, want); diff != "" { + t.Errorf("createRequestParam() diff (-got +want):\n%s", diff) + } +} diff --git a/internal/command/run.go b/internal/command/run.go index a239f1fa..9c6799d7 100644 --- a/internal/command/run.go +++ b/internal/command/run.go @@ -30,6 +30,7 @@ type TestRunner interface { // This is also used to obtain a fallback non-intelligent test splitting mechanism. GetFiles() ([]string, error) Name() string + LocationPrefix() string } const Logo = ` @@ -70,6 +71,19 @@ func Run(ctx context.Context, cfg *config.Config, testListFilename string) error // get plan for this node thisNodeTask := testPlan.Tasks[strconv.Itoa(cfg.NodeIndex)] + // File paths sent to the API for test plan creation include the location prefix to match Test Engine records. + // However, the test runner expects file paths without the prefix, so we need to remove it before running the tests. + locationPrefix := testRunner.LocationPrefix() + if locationPrefix != "" && !testPlan.Fallback { + for i, test := range thisNodeTask.Tests { + path, err := trimFilePathPrefix(test.Path, locationPrefix) + if err != nil { + return fmt.Errorf("failed to trim path prefix: %w", err) + } + thisNodeTask.Tests[i].Path = path + } + } + // execute tests var timeline []api.Timeline runResult, err := runTestsWithRetry(testRunner, &thisNodeTask.Tests, cfg.MaxRetries, testPlan.MutedTests, &timeline, cfg.RetryForMutedTest, cfg.FailOnNoTests) diff --git a/internal/command/run_test.go b/internal/command/run_test.go index 02600920..82b79a3c 100644 --- a/internal/command/run_test.go +++ b/internal/command/run_test.go @@ -654,579 +654,6 @@ func TestFetchOrCreateTestPlan_BillingError(t *testing.T) { } } -func TestCreateRequestParams(t *testing.T) { - svr := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - fmt.Fprint(w, ` -{ - "tests": [ - { "path": "testdata/rspec/spec/fruits/banana_spec.rb", "reason": "slow file" }, - { "path": "testdata/rspec/spec/fruits/fig_spec.rb", "reason": "slow file" } - ] -}`) - })) - defer svr.Close() - - cfg := config.Config{ - OrganizationSlug: "my-org", - SuiteSlug: "my-suite", - Identifier: "identifier", - Parallelism: 7, - Branch: "", - TestRunner: "rspec", - } - - client := api.NewClient(api.ClientConfig{ - ServerBaseUrl: svr.URL, - }) - files := []string{ - "testdata/rspec/spec/fruits/apple_spec.rb", - "testdata/rspec/spec/fruits/banana_spec.rb", - "testdata/rspec/spec/fruits/cherry_spec.rb", - "testdata/rspec/spec/fruits/dragonfruit_spec.rb", - "testdata/rspec/spec/fruits/elderberry_spec.rb", - "testdata/rspec/spec/fruits/fig_spec.rb", - "testdata/rspec/spec/fruits/grape_spec.rb", - } - - got, err := createRequestParam(context.Background(), &cfg, files, *client, runner.Rspec{ - RunnerConfig: runner.RunnerConfig{ - TestCommand: "rspec", - }, - }) - if err != nil { - t.Errorf("createRequestParam() error = %v", err) - } - - // filtered files: banana_spec.rb, fig_spec.rb - // the rest: apple_spec.rb, cherry_spec.rb, dragonfruit_spec.rb, elderberry_spec.rb, grape_spec.rb - want := api.TestPlanParams{ - Identifier: "identifier", - Parallelism: 7, - Branch: "", - Runner: "rspec", - Tests: api.TestPlanParamsTest{ - Files: []plan.TestCase{ - {Path: "testdata/rspec/spec/fruits/apple_spec.rb"}, - {Path: "testdata/rspec/spec/fruits/cherry_spec.rb"}, - {Path: "testdata/rspec/spec/fruits/dragonfruit_spec.rb"}, - {Path: "testdata/rspec/spec/fruits/elderberry_spec.rb"}, - {Path: "testdata/rspec/spec/fruits/grape_spec.rb"}, - }, - Examples: []plan.TestCase{ - { - Identifier: "./testdata/rspec/spec/fruits/banana_spec.rb[1:1]", - Name: "is yellow", - Path: "./testdata/rspec/spec/fruits/banana_spec.rb[1:1]", - Scope: "Banana", - }, - { - Identifier: "./testdata/rspec/spec/fruits/banana_spec.rb[1:2:1]", - Name: "is green", - Path: "./testdata/rspec/spec/fruits/banana_spec.rb[1:2:1]", - Scope: "Banana when not ripe", - }, - { - Identifier: "./testdata/rspec/spec/fruits/fig_spec.rb[1:1]", - Name: "is purple", - Path: "./testdata/rspec/spec/fruits/fig_spec.rb[1:1]", - Scope: "Fig", - }, - }, - }, - } - - if diff := cmp.Diff(got, want); diff != "" { - t.Errorf("createRequestParam() diff (-got +want):\n%s", diff) - } -} - -func TestCreateRequestParams_NonRSpec(t *testing.T) { - svr := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - fmt.Fprint(w, ` -{ - "tests": [ - { "path": "testdata/jest/banana.spec.js", "reason": "slow file" }, - { "path": "testdata/jest/fig.spec.js", "reason": "slow file" } - ] -}`) - })) - defer svr.Close() - - runners := []TestRunner{ - runner.Jest{}, runner.Playwright{}, runner.Cypress{}, - } - - for _, r := range runners { - t.Run(r.Name(), func(t *testing.T) { - cfg := config.Config{ - OrganizationSlug: "my-org", - SuiteSlug: "my-suite", - Identifier: "identifier", - Parallelism: 7, - Branch: "", - TestRunner: r.Name(), - } - - client := api.NewClient(api.ClientConfig{ - ServerBaseUrl: svr.URL, - }) - files := []string{ - "testdata/fruits/apple.spec.js", - "testdata/fruits/banana.spec.js", - "testdata/fruits/cherry.spec.js", - } - - got, err := createRequestParam(context.Background(), &cfg, files, *client, r) - if err != nil { - t.Errorf("createRequestParam() error = %v", err) - } - - want := api.TestPlanParams{ - Identifier: "identifier", - Parallelism: 7, - Branch: "", - Runner: r.Name(), - Tests: api.TestPlanParamsTest{ - Files: []plan.TestCase{ - {Path: "testdata/fruits/apple.spec.js"}, - {Path: "testdata/fruits/banana.spec.js"}, - {Path: "testdata/fruits/cherry.spec.js"}, - }, - }, - } - - if diff := cmp.Diff(got, want); diff != "" { - t.Errorf("createRequestParam() diff (-got +want):\n%s", diff) - } - }) - } -} - -func TestCreateRequestParams_PytestPants(t *testing.T) { - svr := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - fmt.Fprint(w, ` -{ - "tests": [ - { "path": "test/banana_test.py", "reason": "slow file" }, - { "path": "test/fig_test.py", "reason": "slow file" } - ] -}`) - })) - defer svr.Close() - - runner := runner.PytestPants{} - - t.Run(runner.Name(), func(t *testing.T) { - cfg := config.Config{ - OrganizationSlug: "my-org", - SuiteSlug: "my-suite", - Identifier: "identifier", - Parallelism: 7, - Branch: "", - TestRunner: runner.Name(), - } - - client := api.NewClient(api.ClientConfig{ - ServerBaseUrl: svr.URL, - }) - files := []string{ - "test/apple_test.py", - "test/banana_test.py", - "test/cherry_test.py", - } - - got, err := createRequestParam(context.Background(), &cfg, files, *client, runner) - if err != nil { - t.Errorf("createRequestParam() error = %v", err) - } - - want := api.TestPlanParams{ - Identifier: "identifier", - Parallelism: 7, - Branch: "", - Runner: "pytest", - Tests: api.TestPlanParamsTest{ - Files: []plan.TestCase{ - {Path: "test/apple_test.py"}, - {Path: "test/banana_test.py"}, - {Path: "test/cherry_test.py"}, - }, - }, - } - - if diff := cmp.Diff(got, want); diff != "" { - t.Errorf("createRequestParam() diff (-got +want):\n%s", diff) - } - }) -} - -func TestCreateRequestParams_WithSelectionAndMetadata_NonRSpec(t *testing.T) { - cfg := config.Config{ - Identifier: "identifier", - Parallelism: 2, - Branch: "main", - TestRunner: "jest", - SelectionStrategy: "least-reliable", - SelectionParams: map[string]string{ - "top": "100", - }, - Metadata: map[string]string{ - "git_diff": "line1\nline2", - "source": "cli", - }, - } - - client := api.NewClient(api.ClientConfig{ - ServerBaseUrl: "http://example.com", - }) - - files := []string{ - "testdata/fruits/apple.spec.js", - "testdata/fruits/banana.spec.js", - } - - got, err := createRequestParam(context.Background(), &cfg, files, *client, runner.Jest{}) - if err != nil { - t.Errorf("createRequestParam() error = %v", err) - } - - want := api.TestPlanParams{ - Identifier: "identifier", - Parallelism: 2, - Branch: "main", - Runner: "jest", - Selection: &api.SelectionParams{ - Strategy: "least-reliable", - Params: map[string]string{ - "top": "100", - }, - }, - Metadata: map[string]string{ - "git_diff": "line1\nline2", - "source": "cli", - }, - Tests: api.TestPlanParamsTest{ - Files: []plan.TestCase{ - {Path: "testdata/fruits/apple.spec.js"}, - {Path: "testdata/fruits/banana.spec.js"}, - }, - }, - } - - if diff := cmp.Diff(got, want); diff != "" { - t.Errorf("createRequestParam() diff (-got +want):\n%s", diff) - } -} - -func TestCreateRequestParams_WithSelectionAndMetadata_SplitAllFilesBranch(t *testing.T) { - cfg := config.Config{ - Identifier: "identifier", - Parallelism: 2, - Branch: "main", - TestRunner: "pytest", - TagFilters: "team:frontend", - SelectionStrategy: "percent", - SelectionParams: map[string]string{ - "percent": "40", - }, - Metadata: map[string]string{ - "git_diff": "line1\nline2", - }, - } - - client := api.NewClient(api.ClientConfig{ - ServerBaseUrl: "http://example.com", - }) - - files := []string{ - "test_sample.py", - } - - stubRunner := metadataTestRunner{ - name: "pytest", - examples: []plan.TestCase{ - { - Identifier: "test_sample.py::test_happy", - Path: "test_sample.py::test_happy", - Scope: "test_sample.py", - Name: "test_happy", - Format: plan.TestCaseFormatExample, - }, - }, - } - - got, err := createRequestParam(context.Background(), &cfg, files, *client, stubRunner) - if err != nil { - t.Errorf("createRequestParam() error = %v", err) - } - - want := api.TestPlanParams{ - Identifier: "identifier", - Parallelism: 2, - Branch: "main", - Runner: "pytest", - Selection: &api.SelectionParams{ - Strategy: "percent", - Params: map[string]string{ - "percent": "40", - }, - }, - Metadata: map[string]string{ - "git_diff": "line1\nline2", - }, - Tests: api.TestPlanParamsTest{ - Examples: []plan.TestCase{ - { - Identifier: "test_sample.py::test_happy", - Path: "test_sample.py::test_happy", - Scope: "test_sample.py", - Name: "test_happy", - Format: plan.TestCaseFormatExample, - }, - }, - }, - } - - if diff := cmp.Diff(got, want); diff != "" { - t.Errorf("createRequestParam() diff (-got +want):\n%s", diff) - } -} - -type metadataTestRunner struct { - name string - examples []plan.TestCase -} - -func (r metadataTestRunner) Name() string { - return r.name -} - -func (r metadataTestRunner) GetExamples(files []string) ([]plan.TestCase, error) { - return r.examples, nil -} - -func (r metadataTestRunner) GetFiles() ([]string, error) { - return nil, nil -} - -func (r metadataTestRunner) Run(result *runner.RunResult, testCases []plan.TestCase, retry bool) error { - return nil -} - -func TestCreateRequestParams_FilterTestsError(t *testing.T) { - svr := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - http.Error(w, `{ "message": "forbidden" }`, http.StatusForbidden) - })) - - defer svr.Close() - - cfg := config.Config{ - OrganizationSlug: "my-org", - SuiteSlug: "my-suite", - Identifier: "identifier", - Parallelism: 7, - Branch: "", - SplitByExample: true, - } - - client := api.NewClient(api.ClientConfig{ - ServerBaseUrl: svr.URL, - }) - files := []string{ - "apple_spec.rb", - "banana_spec.rb", - "cherry_spec.rb", - "dragonfruit_spec.rb", - "elderberry_spec.rb", - "fig_spec.rb", - "grape_spec.rb", - } - - _, err := createRequestParam(context.Background(), &cfg, files, *client, runner.Rspec{}) - - if err.Error() != "filter tests: forbidden" { - t.Errorf("createRequestParam() error = %v, want forbidden error", err) - } -} - -func TestCreateRequestParams_NoFilteredFiles(t *testing.T) { - svr := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - fmt.Fprint(w, ` -{ - "files": [] -}`) - })) - defer svr.Close() - - cfg := config.Config{ - OrganizationSlug: "my-org", - SuiteSlug: "my-suite", - Identifier: "identifier", - Parallelism: 7, - Branch: "", - SplitByExample: true, - } - - client := api.NewClient(api.ClientConfig{ - ServerBaseUrl: svr.URL, - }) - files := []string{ - "testdata/rspec/spec/fruits/apple_spec.rb", - "testdata/rspec/spec/fruits/banana_spec.rb", - "testdata/rspec/spec/fruits/cherry_spec.rb", - "testdata/rspec/spec/fruits/dragonfruit_spec.rb", - "testdata/rspec/spec/fruits/elderberry_spec.rb", - "testdata/rspec/spec/fruits/fig_spec.rb", - "testdata/rspec/spec/fruits/grape_spec.rb", - } - - got, err := createRequestParam(context.Background(), &cfg, files, *client, runner.Rspec{ - RunnerConfig: runner.RunnerConfig{ - TestCommand: "rspec", - }, - }) - if err != nil { - t.Errorf("createRequestParam() error = %v", err) - } - - want := api.TestPlanParams{ - Identifier: "identifier", - Parallelism: 7, - Branch: "", - Tests: api.TestPlanParamsTest{ - Files: []plan.TestCase{ - {Path: "testdata/rspec/spec/fruits/apple_spec.rb"}, - {Path: "testdata/rspec/spec/fruits/banana_spec.rb"}, - {Path: "testdata/rspec/spec/fruits/cherry_spec.rb"}, - {Path: "testdata/rspec/spec/fruits/dragonfruit_spec.rb"}, - {Path: "testdata/rspec/spec/fruits/elderberry_spec.rb"}, - {Path: "testdata/rspec/spec/fruits/fig_spec.rb"}, - {Path: "testdata/rspec/spec/fruits/grape_spec.rb"}, - }, - }, - } - - if diff := cmp.Diff(got, want); diff != "" { - t.Errorf("createRequestParam() diff (-got +want):\n%s", diff) - } -} - -func TestCreateRequestParams_WithTagFilters(t *testing.T) { - cfg := config.Config{ - OrganizationSlug: "my-org", - SuiteSlug: "my-suite", - Identifier: "identifier", - Parallelism: 2, - Branch: "main", - TestRunner: "pytest", - TagFilters: "team:frontend", - } - - client := api.NewClient(api.ClientConfig{ - ServerBaseUrl: "example.com", - }) - - files := []string{ - "../runner/testdata/pytest/failed_test.py", - "../runner/testdata/pytest/test_sample.py", - "../runner/testdata/pytest/spells/test_expelliarmus.py", - } - - got, err := createRequestParam(context.Background(), &cfg, files, *client, runner.Pytest{ - RunnerConfig: runner.RunnerConfig{ - TestCommand: "pytest", - TagFilters: "team:frontend", - }, - }) - if err != nil { - t.Errorf("createRequestParam() error = %v", err) - } - - want := api.TestPlanParams{ - Identifier: "identifier", - Parallelism: 2, - Branch: "main", - Runner: "pytest", - Tests: api.TestPlanParamsTest{ - Examples: []plan.TestCase{ - { - Format: "example", - Identifier: "runner/testdata/pytest/test_sample.py::test_happy", - Name: "test_happy", - Path: "runner/testdata/pytest/test_sample.py::test_happy", - Scope: "runner/testdata/pytest/test_sample.py", - }, - { - Format: "example", - Identifier: "runner/testdata/pytest/spells/test_expelliarmus.py::TestExpelliarmus::test_knocks_wand_out", - Name: "test_knocks_wand_out", - Path: "runner/testdata/pytest/spells/test_expelliarmus.py::TestExpelliarmus::test_knocks_wand_out", - Scope: "runner/testdata/pytest/spells/test_expelliarmus.py::TestExpelliarmus", - }, - }, - }, - } - - if diff := cmp.Diff(got, want); diff != "" { - t.Errorf("createRequestParam() diff (-got +want):\n%s", diff) - } -} - -func TestCreateRequestParams_WithTagFilters_NonPytest(t *testing.T) { - svr := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - fmt.Fprint(w, ` -{ - "tests": [] -}`) - })) - defer svr.Close() - - cfg := config.Config{ - OrganizationSlug: "my-org", - SuiteSlug: "my-suite", - Identifier: "identifier", - Parallelism: 2, - Branch: "main", - TestRunner: "rspec", - TagFilters: "team:frontend", - } - - client := api.NewClient(api.ClientConfig{ - ServerBaseUrl: svr.URL, - }) - - files := []string{ - "testdata/rspec/spec/fruits/apple_spec.rb", - "testdata/rspec/spec/fruits/banana_spec.rb", - } - - got, err := createRequestParam(context.Background(), &cfg, files, *client, runner.Rspec{ - RunnerConfig: runner.RunnerConfig{ - TestCommand: "rspec", - }, - }) - if err != nil { - t.Errorf("createRequestParam() error = %v", err) - } - - want := api.TestPlanParams{ - Identifier: "identifier", - Parallelism: 2, - Branch: "main", - Runner: "rspec", - Tests: api.TestPlanParamsTest{ - Files: []plan.TestCase{ - {Path: "testdata/rspec/spec/fruits/apple_spec.rb"}, - {Path: "testdata/rspec/spec/fruits/banana_spec.rb"}, - }, - }, - } - - if diff := cmp.Diff(got, want); diff != "" { - t.Errorf("createRequestParam() diff (-got +want):\n%s", diff) - } -} - func TestSendMetadata(t *testing.T) { originalVersion := version.Version version.Version = "0.1.0" diff --git a/internal/config/config.go b/internal/config/config.go index a92e7b99..bbcbf63e 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -4,62 +4,66 @@ import "time" // Config is the internal representation of the complete test engine client configuration. type Config struct { - BuildId string `json:"BUILDKITE_BUILD_ID"` - JobId string `json:"BUILDKITE_JOB_ID"` - StepId string `json:"BUILDKITE_STEP_ID"` // AccessToken is the access token for the API. AccessToken string `json:"-"` + // Branch is the string value of the git branch name, used by Buildkite only. + Branch string `json:"BUILDKITE_BRANCH"` + BuildId string `json:"BUILDKITE_BUILD_ID"` + // Enable debug output + DebugEnabled bool `json:"BUILDKITE_TEST_ENGINE_DEBUG_ENABLED"` + // FailOnNoTests causes the client to exit with an error if no tests are assigned to the node + FailOnNoTests bool `json:"BUILDKITE_TEST_ENGINE_FAIL_ON_NO_TESTS"` // Identifier is the identifier of the build. Identifier string `json:"BUILDKITE_TEST_ENGINE_IDENTIFIER"` + JobId string `json:"BUILDKITE_JOB_ID"` + // JobRetryCount is the count of the number of times the job has been retried. + JobRetryCount int `json:"BUILDKITE_RETRY_COUNT"` + // LocationPrefix is prepended to test file paths when requesting a test plan. + // Use this when the test collector is configured to report files with a path prefix, + // so the test plan API can correctly match and bin-pack them across nodes. + LocationPrefix string `json:"BUILDKITE_TEST_ENGINE_LOCATION_PREFIX"` + // MaxParallelism is the maximum parallelism when calculating parallelism dynamically. + MaxParallelism int `json:"BUILDKITE_TEST_ENGINE_MAX_PARALLELISM"` // MaxRetries is the maximum number of retries for a failed test. MaxRetries int `json:"BUILDKITE_TEST_ENGINE_RETRY_COUNT"` - // RetryCommand is the command to run the retry tests. - RetryCommand string `json:"BUILDKITE_TEST_ENGINE_RETRY_CMD"` + // Metadata is additional key/value data sent to the test plan API. + Metadata map[string]string `json:"-"` // Node index is index of the current node. NodeIndex int `json:"BUILDKITE_PARALLEL_JOB"` // OrganizationSlug is the slug of the organization. OrganizationSlug string `json:"BUILDKITE_ORGANIZATION_SLUG"` // Parallelism is the number of parallel tasks to run. Parallelism int `json:"BUILDKITE_PARALLEL_JOB_COUNT"` - // Maximum parallelism when calculating parallelism dynamically. - MaxParallelism int `json:"BUILDKITE_TEST_ENGINE_MAX_PARALLELISM"` - // TargetTime is the target time in seconds for the test plan. - TargetTime time.Duration `json:"BUILDKITE_TEST_ENGINE_TARGET_TIME"` - // The path to the result file. + // ResultPath is the path to the result file. ResultPath string `json:"-"` - // Whether a failed muted test should be retried. + // RetryCommand is the command to run the retry tests. + RetryCommand string `json:"BUILDKITE_TEST_ENGINE_RETRY_CMD"` + // RetryForMutedTest indicates whether a failed muted test should be retried. // This is default to true because we want more signal for our flaky detection system. RetryForMutedTest bool `json:"-"` + // SelectionParams are additional key/value parameters for the strategy. + SelectionParams map[string]string `json:"-"` + // SelectionStrategy is the selection strategy sent to the test plan API. + SelectionStrategy string `json:"BUILDKITE_TEST_ENGINE_SELECTION_STRATEGY"` // ServerBaseUrl is the base URL of the test plan server. ServerBaseUrl string `json:"-"` // SplitByExample is the flag to enable split the test by example. - SplitByExample bool `json:"BUILDKITE_TEST_ENGINE_SPLIT_BY_EXAMPLE"` + SplitByExample bool `json:"BUILDKITE_TEST_ENGINE_SPLIT_BY_EXAMPLE"` + StepId string `json:"BUILDKITE_STEP_ID"` // SuiteSlug is the slug of the suite. SuiteSlug string `json:"BUILDKITE_TEST_ENGINE_SUITE_SLUG"` - // SelectionStrategy is the selection strategy sent to the test plan API. - SelectionStrategy string `json:"BUILDKITE_TEST_ENGINE_SELECTION_STRATEGY"` - // SelectionParams are additional key/value parameters for the strategy. - SelectionParams map[string]string `json:"-"` - // Metadata is additional key/value data sent to the test plan API. - Metadata map[string]string `json:"-"` + // TagFilters filters test examples by execution tags. + TagFilters string `json:"BUILDKITE_TEST_ENGINE_TAG_FILTERS"` + // TargetTime is the target time in seconds for the test plan. + TargetTime time.Duration `json:"BUILDKITE_TEST_ENGINE_TARGET_TIME"` // TestCommand is the command to run the tests. TestCommand string `json:"BUILDKITE_TEST_ENGINE_TEST_CMD"` - // TestFilePattern is the pattern to match the test files. - TestFilePattern string `json:"BUILDKITE_TEST_ENGINE_TEST_FILE_PATTERN"` // TestFileExcludePattern is the pattern to exclude the test files. TestFileExcludePattern string `json:"BUILDKITE_TEST_ENGINE_TEST_FILE_EXCLUDE_PATTERN"` + // TestFilePattern is the pattern to match the test files. + TestFilePattern string `json:"BUILDKITE_TEST_ENGINE_TEST_FILE_PATTERN"` // TestRunner is the name of the runner. TestRunner string `json:"BUILDKITE_TEST_ENGINE_TEST_RUNNER"` - // Branch is the string value of the git branch name, used by Buildkite only. - Branch string `json:"BUILDKITE_BRANCH"` - // JobRetryCount is the count of the number of times the job has been retried. - JobRetryCount int `json:"BUILDKITE_RETRY_COUNT"` - // Enable debug output - DebugEnabled bool `json:"BUILDKITE_TEST_ENGINE_DEBUG_ENABLED"` - // FailOnNoTests causes the client to exit with an error if no tests are assigned to the node - FailOnNoTests bool `json:"BUILDKITE_TEST_ENGINE_FAIL_ON_NO_TESTS"` - // TagFilters filters test examples by execution tags. - TagFilters string `json:"BUILDKITE_TEST_ENGINE_TAG_FILTERS"` // errs is a map of environment variables name and the validation errors associated with them. errs InvalidConfigError } diff --git a/internal/runner/cucumber_test.go b/internal/runner/cucumber_test.go index d08bd362..9b34702c 100644 --- a/internal/runner/cucumber_test.go +++ b/internal/runner/cucumber_test.go @@ -45,7 +45,7 @@ func TestNewCucumber(t *testing.T) { for _, c := range cases { got := NewCucumber(c.input) - if diff := cmp.Diff(got.RunnerConfig, c.want); diff != "" { + if diff := cmp.Diff(got.RunnerConfig, c.want, cmp.AllowUnexported(RunnerConfig{})); diff != "" { t.Errorf("NewCucumber(%v) diff (-got +want):\n%s", c.input, diff) } } diff --git a/internal/runner/detector.go b/internal/runner/detector.go index 3d97c40e..79c6c0bb 100644 --- a/internal/runner/detector.go +++ b/internal/runner/detector.go @@ -8,16 +8,22 @@ import ( ) type RunnerConfig struct { - TestRunner string - TestCommand string - TestFilePattern string - TestFileExcludePattern string - RetryTestCommand string - TagFilters string + TestRunner string + + locationPrefix string // ResultPath is used internally so bktec can read result from Test Runner. // User typically don't need to worry about setting this except in in RSpec and playwright. // In playwright, for example, it can only be configured via a config file, therefore it's mandatory for user to set. - ResultPath string + ResultPath string + RetryTestCommand string + TagFilters string + TestCommand string + TestFileExcludePattern string + TestFilePattern string +} + +func (c RunnerConfig) LocationPrefix() string { + return c.locationPrefix } type TestRunner interface { @@ -25,17 +31,20 @@ type TestRunner interface { GetExamples(files []string) ([]plan.TestCase, error) GetFiles() ([]string, error) Name() string + LocationPrefix() string } func DetectRunner(cfg *config.Config) (TestRunner, error) { runnerConfig := RunnerConfig{ - TestRunner: cfg.TestRunner, - TestCommand: cfg.TestCommand, - TestFilePattern: cfg.TestFilePattern, - TestFileExcludePattern: cfg.TestFileExcludePattern, - RetryTestCommand: cfg.RetryCommand, + TestRunner: cfg.TestRunner, + + locationPrefix: cfg.LocationPrefix, ResultPath: cfg.ResultPath, + RetryTestCommand: cfg.RetryCommand, TagFilters: cfg.TagFilters, + TestCommand: cfg.TestCommand, + TestFileExcludePattern: cfg.TestFileExcludePattern, + TestFilePattern: cfg.TestFilePattern, } switch testRunner := cfg.TestRunner; testRunner { diff --git a/internal/runner/jest_test.go b/internal/runner/jest_test.go index f91fcc32..9beda924 100644 --- a/internal/runner/jest_test.go +++ b/internal/runner/jest_test.go @@ -47,7 +47,7 @@ func TestNewJest(t *testing.T) { for _, c := range cases { got := NewJest(c.input) - if diff := cmp.Diff(got.RunnerConfig, c.want); diff != "" { + if diff := cmp.Diff(got.RunnerConfig, c.want, cmp.AllowUnexported(RunnerConfig{})); diff != "" { t.Errorf("NewJest(%v) diff (-got +want):\n%s", c.input, diff) } } diff --git a/internal/runner/rspec.go b/internal/runner/rspec.go index dc239d5d..f7978c18 100644 --- a/internal/runner/rspec.go +++ b/internal/runner/rspec.go @@ -31,6 +31,13 @@ func NewRspec(r RunnerConfig) Rspec { r.RetryTestCommand = r.TestCommand } + if r.locationPrefix == "" { + // rspec test in Test Engine is stored with leading "./" + // therefore, we need to add "./" to the file path + // to match the test path in Test Engine + r.locationPrefix = "./" + } + return Rspec{ RunnerConfig: r, } @@ -46,13 +53,6 @@ func (r Rspec) GetFiles() ([]string, error) { files, err := discoverTestFiles(r.TestFilePattern, r.TestFileExcludePattern) debug.Println("Discovered", len(files), "files") - // rspec test in Test Engine is stored with leading "./" - // therefore, we need to add "./" to the file path - // to match the test path in Test Engine - for i, file := range files { - files[i] = "./" + file - } - if err != nil { return nil, err } diff --git a/internal/runner/rspec_test.go b/internal/runner/rspec_test.go index 3d502047..82447f04 100644 --- a/internal/runner/rspec_test.go +++ b/internal/runner/rspec_test.go @@ -25,6 +25,7 @@ func TestNewRspec(t *testing.T) { TestFilePattern: "spec/**/*_spec.rb", TestFileExcludePattern: "", RetryTestCommand: "bundle exec rspec --format progress --format json --out {{resultPath}} {{testExamples}}", + locationPrefix: "./", }, }, // custom @@ -34,12 +35,14 @@ func TestNewRspec(t *testing.T) { TestFilePattern: "spec/models/**/*_spec.rb", TestFileExcludePattern: "spec/features/**/*_spec.rb", RetryTestCommand: "bin/rspec --fail-fast {{testExamples}}", + locationPrefix: "my/project", }, want: RunnerConfig{ TestCommand: "bin/rspec --format documentation {{testExamples}} --format json --out {{resultPath}}", TestFilePattern: "spec/models/**/*_spec.rb", TestFileExcludePattern: "spec/features/**/*_spec.rb", RetryTestCommand: "bin/rspec --fail-fast {{testExamples}}", + locationPrefix: "my/project", }, }, // RetryTestCommand fallback to TestCommand @@ -52,13 +55,14 @@ func TestNewRspec(t *testing.T) { TestFilePattern: "spec/**/*_spec.rb", TestFileExcludePattern: "", RetryTestCommand: "bundle exec --format json --out out.json {{testExamples}}", + locationPrefix: "./", }, }, } for _, c := range cases { got := NewRspec(c.input) - if diff := cmp.Diff(got.RunnerConfig, c.want); diff != "" { + if diff := cmp.Diff(got.RunnerConfig, c.want, cmp.AllowUnexported(RunnerConfig{})); diff != "" { t.Errorf("NewRspec(%v) diff (-got +want):\n%s", c.input, diff) } }