Skip to content

Add selector split-unit support to Test Engine Client#558

Merged
nprizal merged 7 commits into
mainfrom
te-6156-add-selector-split-unit-support-to-test-engine-client
Jun 28, 2026
Merged

Add selector split-unit support to Test Engine Client#558
nprizal merged 7 commits into
mainfrom
te-6156-add-selector-split-unit-support-to-test-engine-client

Conversation

@meghan-kradolfer

@meghan-kradolfer meghan-kradolfer commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Description

Adds experimental selector-based split-unit support to bktec so supported non-file runners can request and execute selector plans from Test Engine. This is gated behind --selector-splitting / BUILDKITE_TEST_ENGINE_SELECTOR_SPLITTING=true, keeping existing file/example splitting behaviour unchanged by default.

Context

Linear: https://linear.app/buildkite/issue/TE-6156/add-selector-split-unit-support-to-test-engine-client

Related API/planner shape: https://linear.app/buildkite/issue/TE-6157/add-selector-split-units-to-the-test-plan-api-and-planner-model

Changes

  • Adds the experimental selector splitting config field, CLI flag, and env var source.
  • Adds runner capability modelling for selector splitting and marks go test as selector-capable.
  • Sends selector split-unit requests for supported runners when the opt-in is enabled.
  • Handles selector-format plan responses in go test execution and split summaries.
  • Adds guardrail and coverage tests for the opt-in, request payloads, supported features, selector execution, and summaries.

Testing

  • go test ./... — fails locally because integration-style runner tests require external binaries that are not installed in this environment (rspec, pytest, cucumber, yarn, gotestsum).

@meghan-kradolfer meghan-kradolfer marked this pull request as ready for review June 26, 2026 02:16
@meghan-kradolfer meghan-kradolfer requested a review from a team as a code owner June 26, 2026 02:16
Comment thread cli.go Outdated
Comment thread internal/plan/type.go
Path string `json:"path"`
Scope string `json:"scope,omitempty"`
// Value is the runnable selector for selector-based plans, for example a Go package import path.
Value string `json:"value,omitempty"`

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we call this Selector?

Suggested change
Value string `json:"value,omitempty"`
Selector string `json:"selector,omitempty"`

@meghan-kradolfer meghan-kradolfer Jun 26, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current API shape uses { "value": "github.com/..." } so I don't think we can do a full rename here but we could do:

Selector string `json:"value,omitempty"`

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’m leaning towards keeping this as value, since format: "selector" already identifies it as a selector.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok for now. I think the problem is because we use the same type for input (API request params), output (API response), and bktec internalsw, so it is a bit confusing at the moment. Maybe we should separate them at some point. We have this top level comment for that type.

// TestCase currently can represent a single test case or a single test file (when used as output of test plan API).
// TODO: it's best if we split this into two types.

Comment thread internal/command/request_param.go Outdated
// Currently only the Pytest runner supports tag filtering.
func createRequestParam(ctx context.Context, cfg *config.Config, files []string, client api.Client, runner runner.TestRunner) (api.TestPlanParams, error) {
if shouldUseSelectorSplitting(cfg, runner) {
selectors := make([]api.TestPlanParamsSelector, 0, len(files))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its an existing poor design of bktec, but currently we pass files to this function. However for go test, we are actually passing "packages". Maybe we should rename files to something like filesOrSelectors (Can't think of better name :D), or put a comment? Ideally we will pass selectors and the runner responsible and creating the selectors.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated this to be testTargets, not sure if that is any better 🤷🏼‍♀️

@nprizal

nprizal commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

@buildsworth-bk review

@nprizal nprizal left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Will test this e2e next week before merging.

Comment thread .buildkite/pipeline.yml
command: ".buildkite/steps/tests.sh"
parallelism: 2
env:
BUILDKITE_TEST_ENGINE_SELECTOR_SPLITTING: "true"

@meghan-kradolfer meghan-kradolfer Jun 26, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nprizal I added this flag so this can just be merged and tested! You can see it working on this branch

Image

It still says "no history" as we need to also turn on the TestEngineUseSelectorPrimaryTimingsForTestPlan feature flag for bk for it to hit the new selector-tag timing query

@nprizal

nprizal commented Jun 28, 2026

Copy link
Copy Markdown
Contributor
Screenshot 2026-06-29 at 9 52 33 AM

I have enabled the feature flag, and it is now using the historical duration. One issue I found is that bktec sends go packages that don't have test to the Test Plan API, so the binpacking is not optimized. Packages without tests should be excluded from Test Plan API request body. I'll create a ticket for this. Meanwhile, I will merge this PR.

@nprizal nprizal merged commit d0e57d5 into main Jun 28, 2026
3 checks passed
@nprizal nprizal deleted the te-6156-add-selector-split-unit-support-to-test-engine-client branch June 28, 2026 21:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants