pingcap / pingcap/tidb

Proposal: Monolithic test binary to reduce repeated Go test link time in CI

Open
#67,753 3 comments 2 reactions 0 assignees View on GitHub
component/test type/enhancement
Dominant language
Go
Stars
40.5k
Forks
6.2k
PR merge metrics
PR metrics pending

Description

## Enhancement

### Summary
TiDB CI still spends significant time compiling and linking Go test binaries package-by-package. A large portion of package tests (especially testkit-based external tests) have highly overlapping dependencies, so we repeatedly pay the same link cost.

This proposal introduces a **monolithic test binary** path for selected packages to reduce repeated link invocations from `N` to `1`.

### Background / Problem
- TiDB has hundreds of Go packages with tests.
- For major packages, dependency overlap is very high (example: `ddl`, `executor`, `planner` share most deps).
- Go linker (`cmd/link`) is the bottleneck here: link is effectively single-threaded and not reused the same way compile artifacts are.
- Result: even with warm compile cache, package-level `go test -c` still spends substantial time in link step for each package.

### Data Points (PoC observations)
- `go test -c ./pkg/ddl` ~45s (link-heavy)
- `go test -c ./pkg/executor` ~29s
- Sequential `ddl + executor` compile/link ~74s
- Monolithic build for equivalent scope ~32s
- Test function coverage in PoC stayed effectively 1:1 for migrated test set (DDL + Executor sample)

These numbers indicate the main gain comes from reducing repeated linking, not from changing test logic.

### Proposed Design (recommended direction)
Use **Move + Wrapper** (not long-term copy-merge):

1. Move migratable external tests to `pkg//test/`.
2. Keep test logic in normal `.go` files (importable for monolithic registration).
3. Keep thin package-level `_test.go` wrappers calling the same logic, so local package test workflow remains.
4. Add a `pkg/mega` registry/runner that imports test subpackages and runs registered tests in one binary.

### Why this direction
- Keeps package ownership clear (`pkg/ddl/test`, `pkg/executor/test`, ...).
- Supports both workflows:
- local package testing (`go test ./pkg//test ...`)
- CI monolithic binary for migrated scope
- Avoids maintaining generated duplicated test copies as source of truth.
- Allows gradual migration package by package.

### Scope / Non-goals
- Not replacing `tests/integrationtest` or `tests/realtikvtest`; those cover different scenarios.
- Not conflicting with Bazel migration. Bazel helps compile/caching; this proposal focuses on reducing repeated link calls.
- Internal tests that depend on private symbols / `export_test.go` bridges may remain package-level.

### Rollout Plan (incremental)
1. Build migration tooling for external tests (`*_test` -> logic `.go` + thin wrapper `_test.go`).
2. Migrate 1-2 heavy packages first (suggested: `ddl`, then `executor`) and validate parity.
3. Run CI in dual mode for migrated scope (existing package path + monolithic path) for confidence window.
4. Switch CI to monolithic path for migrated packages; keep exceptions in package-level runs.

### Risks and Mitigations
- Migration effort: automate with scripts and staged rollout.
- Test parity risk: enforce coverage parity checks and temporary dual-run in CI.
- TestMain/init interactions: isolate global setup/teardown and audit shared state assumptions.

### Expected Benefit
For migrated heavy packages, reduce repeated link overhead substantially and shorten CI critical path without changing SQL semantics or test intent.

### Request
If this direction is acceptable, I can follow up with:
- a small RFC/implementation checklist for phase 1 tooling,
- and a first migration PR for one package to measure end-to-end CI impact.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.