cockroachdb / cockroachdb/cockroach
kvserver: split out integration test target
- 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
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