[TE-5303] Add location prefix support by nprizal · Pull Request #453 · buildkite/test-engine-client

nprizal · 2026-03-03T21:14:07Z

When using bktec with a custom runner for minitest, test plans weren't being optimally bin-packed. All test durations were coming back as 1000 (the default). The root cause is that the minitest collector uploads file paths with a ./ prefix (e.g., ./spec/models/user_spec.rb), but bktec sends paths without it (e.g., spec/models/user_spec.rb). Since the paths don't match, the API can't look up actual durations.

This is the same class of problem that affects customers using the JS collector with a custom prefix (e.g., frontend/ in a monorepo). The collector knows about the prefix, but bktec doesn't.

I've added a --location-prefix flag (sourced from both BUILDKITE_TEST_ENGINE_LOCATION_PREFIX and BUILDKITE_ANALYTICS_LOCATION_PREFIX) that prepends a prefix to file paths when requesting a test plan from the API, and strips it before passing paths to the test runner for execution.

Usage

bktec run \
  --test-runner custom \
  --test-command "bundle exec rake test TEST_FILES='{{testExamples}}'" \
  --test-file-pattern "test/**/*_test.rb" \
  --location-prefix "./"

Or via environment variable:

export BUILDKITE_TEST_ENGINE_LOCATION_PREFIX="./"

nprizal · 2026-03-03T21:19:00Z

cli.go

+	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"),


Using only BUILDKITE_ANALYTICS_LOCATION_PREFIX is inconsistent with other environment variables namespace in bktec. The location prefix can be configured for the test collector without that environment variable (e.g via collector config object) so it might not be present. Supporting BUILDKITE_TEST_ENGINE_LOCATION_PREFIX will make it consistent.

Should be encouraging the deprecation of BUILDKITE_ANALYTICS_LOCATION_PREFIXin favour of BUILDKITE_TEST_ENGINE_LOCATION_PREFIX in our collectors?

gchan

This looks good and I've left some comments and questions.

While we did planned to use locationPrefix to handle both ./ and /path/to/dir cases, I wonder if we are overloading it given that I noticed we treat both cases differently (one we trim, and the other we don't).

I wonder if we can make it clearer how we are handling location prefixes and why (with examples) because I'm afraid that this knowledge might slowly disappear.

gchan · 2026-03-04T20:38:13Z

internal/runner/detector.go

-	TagFilters             string
+	TestRunner string
+
+	locationPrefix string


Nipick - should this attribute be capitalised?
Is there a reason to not export this field?

Some test runner (e.g. rspec) will set a default value to this attribute, so the value should be accessed from the runner interface. In golang, you can't have a static attribute on an interface and it should be a getter function. The options are:

have a public attribute LocationPrefix and create a getter GetLocationPrefix() on the runner interface

have a private attribute locationPrefix and create a getter LocationPrefix() on the runner interface

I chose 2 as it is recommended by golang https://go.dev/doc/effective_go#Getters, and since other package should not access the raw locationPrefix directly (without default value that runner set)

internal/command/path.go

internal/command/run.go

internal/command/request_param.go

gchan · 2026-03-04T21:05:58Z

internal/command/request_param.go

-		filteredFilesPath = append(filteredFilesPath, file.Path)
+		// File paths sent to the API for filtering tests 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.
+		trimmedPath, err := trimFilePathPrefix(file.Path, prefix)


To clarify what's going on in this class:

createRequestParam prepends the prefixes

splitAllFiles calls runner.GetExamples with the prefixes and doesn't remove the prefixes from the examples it returns

filterAndSplitFiles trims the prefixes (unless it is ./) and then calls runner.GetExamples with the prefixes removed. It appears the examples returned has it's prefixes removed?

What's the reason behind this difference it handling?

gchan · 2026-03-04T21:12:11Z

internal/command/request_param_test.go

+	}
+}
+
+func TestCreateRequestParams_WithLocationPrefix(t *testing.T) {


This test and the next one are the new tests, the other tests have been moved from run_test.go

gchan · 2026-03-04T21:17:09Z

internal/command/request_param_test.go

+				{
+					Identifier: "./testdata/rspec/spec/fruits/banana_spec.rb[1:1]",
+					Name:       "is yellow",
+					Path:       "./testdata/rspec/spec/fruits/banana_spec.rb[1:1]",


What's the reason behind the different treatment of examples vs files?

We don't identify examples by filename, but with name and scope. So bktec doesn't need to prepend the fixes when requesting the test plan. Do you think we should add the prefix as well for consistency, even if we don't need it?

Yep, for consistency and to reduce confusion, I think we should!

I handle the examples' Path, but I'm leaving Identifier as is because Identifier might not be a path for other runners.

Do we want to add a brief comment in getExamplesWithPrefix to explain the divergence for future travellers?

gchan · 2026-03-04T21:18:58Z

cli.go

+	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"),


Should be encouraging the deprecation of BUILDKITE_ANALYTICS_LOCATION_PREFIXin favour of BUILDKITE_TEST_ENGINE_LOCATION_PREFIX in our collectors?

gchan · 2026-03-04T21:24:46Z

internal/runner/jest_test.go

 	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 != "" {


My understanding is that this is so we can compare for the unexported field locationPrefix.

gchan · 2026-03-04T21:26:56Z

internal/config/config.go

Nice sorting of the attributes.

Nitpick - I wonder if it makes sense to continue grouping common attributes together similar to how we display them in the cli --help command?

gchan

Giving this an early approval given my major questions about consistency has (or will) been handled.

nprizal · 2026-03-05T01:09:15Z

internal/command/request_param.go

 	}
 }

+func getExamplesWithPrefix(filePaths []string, runner TestRunner) ([]plan.TestCase, error) {


this function is called by filterAndSplitFiles and splitAllFiles since we need to handle it the same way.

gchan

Nice work! Thanks for getting rid of the special ./ case.

nprizal added 4 commits March 4, 2026 09:12

Add LocationPrefix option

98b8d63

Prefix location when making API requests and trim it for local execution

e4f0609

Support test collector location prefix ENV var

48e5b48

Fix runner tests

72887f3

nprizal commented Mar 3, 2026

View reviewed changes

nprizal self-assigned this Mar 3, 2026

nprizal marked this pull request as ready for review March 3, 2026 21:19

nprizal requested a review from a team as a code owner March 3, 2026 21:19

gchan reviewed Mar 4, 2026

View reviewed changes

Trim "./" for consistency

a5ece80

gchan approved these changes Mar 4, 2026

View reviewed changes

nprizal added 2 commits March 5, 2026 13:44

Handle prefix for example paths

c1aee15

Skip trimming path from fallback test plan

a1721ea

nprizal force-pushed the te-5303-add-location-prefix-support branch from 8c27f10 to a1721ea Compare March 5, 2026 01:05

nprizal requested a review from gchan March 5, 2026 01:07

nprizal commented Mar 5, 2026

View reviewed changes

gchan approved these changes Mar 5, 2026

View reviewed changes

Add comment around getExamplesWithPrefix

10a1e5c

Conversation

nprizal commented Mar 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Usage

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gchan left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nprizal Mar 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gchan left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gchan left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

nprizal commented Mar 3, 2026 •

edited

Loading

nprizal Mar 4, 2026 •

edited

Loading