cockroachdb / cockroachdb/cockroach

kvserver: split out integration test target

Open
#172,627 0 comments 0 reactions 0 assignees View on GitHub
A-kv C-enhancement O-agent T-kv
Dominant language
Go
Stars
32.5k
Forks
4.1k
PR merge metrics
PR metrics pending

Description

**Is your feature request related to a problem? Please describe.**

`//pkg/kv/kvserver:kvserver_test` combines dependency-light component and white-box tests with full-server and TestCluster integration tests. The target currently contains 138 test files: 59 use `package kvserver_test` and 79 use `package kvserver`.

This gives the root test target a much larger compile closure than the production library:

- 988 transitive main-workspace Go rules versus 488 for `//pkg/kv/kvserver:kvserver`;
- 2,556 configured transitive `GoCompilePkg` actions versus 1,475;
- `testcluster` alone reaches approximately 900 main-workspace Go rules.

As a result, `./dev test pkg/kv/kvserver` pulls in much of the server and SQL stack even when iterating on dependency-light KV server code.

This is not a Bazel dependency cycle. The test target embeds `kvserver` and also depends on `testcluster`, which reaches back to the production `kvserver` library through the server stack.

**Describe the solution you'd like**

Split the root tests according to their harness:

1. Keep `//pkg/kv/kvserver:kvserver_test` for white-box and component tests that do not initialize a server or TestCluster factory.
2. Add `//pkg/kv/kvserver/component:component_test` for the three lightweight external-package tests:
- `below_raft_protos_test.go`
- `raft_transport_test.go`
- `flow_control_raft_transport_test.go`
3. Add `//pkg/kv/kvserver/integration:integration_test` for server-backed and TestCluster tests. Move the existing external test package there atomically with its `TestMain` and shared helpers.
4. Incrementally extract server-backed test functions from the remaining `package kvserver` files into the integration package. Prefer existing exported APIs and narrow testing knobs; avoid a broad facade exposing KV server internals.
5. Add a dependency check preventing the root test from regaining paths to `testcluster`, `serverutils`, or `//pkg/server:server`.

Afterward, `./dev test pkg/kv/kvserver` should run only dependency-light tests. CI can run the integration target explicitly or test `pkg/kv/kvserver/...` recursively.

**Describe alternatives you've considered**

- Moving only the 59 external-package files does not complete the separation: eight `package kvserver` files also start servers or TestClusters.
- Splitting TestCluster tests into many feature-oriented packages improves selective execution and sharding, but every such target retains the approximately 900-rule TestCluster baseline. Bazel shares common configured actions, so this does not compile the closure once per target, but it also does not make any individual target dependency-light.
- Moving `client_tenant_test.go` under `pkg/ccl` would reduce the integration target's closure, but would run counter to the ongoing reintegration of CCL functionality into the main `pkg` tree. It should remain under KV server.

**Additional context**

Relevant code:

- [`pkg/kv/kvserver/BUILD.bazel`](https://github.com/cockroachlabs/cockroach/blob/bbc8c7a5792796ccb79f4f4ac6ec89fd4c4d2ab7/pkg/kv/kvserver/BUILD.bazel#L287-L634)
- [`pkg/kv/kvserver/main_test.go`](https://github.com/cockroachlabs/cockroach/blob/bbc8c7a5792796ccb79f4f4ac6ec89fd4c4d2ab7/pkg/kv/kvserver/main_test.go)
- [`pkg/testutils/testcluster/BUILD.bazel`](https://github.com/cockroachlabs/cockroach/blob/bbc8c7a5792796ccb79f4f4ac6ec89fd4c4d2ab7/pkg/testutils/testcluster/BUILD.bazel)

Related: [#79357](https://github.com/cockroachdb/cockroach/issues/79357), a broader issue about splitting large packages to speed up builds.

Completion criteria:

- [ ] No root KV server test starts a server or TestCluster.
- [ ] The root target has no dependency path to `testcluster`, `serverutils`, or `//pkg/server:server`.
- [ ] `./dev test pkg/kv/kvserver` runs only dependency-light tests.
- [ ] Recursive KV server CI discovers every moved test.
- [ ] The root target's structural build cost is close to the production library plus ordinary test-only dependencies.

Jira issue: CRDB-65874

Contributor guide

Open the contributing guide

Research direction

Start with pkg/kv/kvserver/BUILD.bazel and pkg/kv/kvserver/main_test.go, then inspect the testcluster BUILD target and the listed external test files. Run ./dev test pkg/kv/kvserver to establish the current target behavior. Done means the root target has no testcluster, serverutils, or //pkg/server:server path, moved tests are found recursively, and dependency-light tests still pass.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
build-system, testing-qa
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.