test(cubemaster): add race and docker-backed test gates, de-flake unit tests

[fslongjin]

Summary

Add race and Docker-backed test gates for CubeMaster unit tests, and replace time.Sleep polling with channel-based synchronization to remove flakiness.

• CI: extend the CubeMaster test matrix with Race and Docker jobs, introducing three run modes: root_make, cubemaster_builder, and cubemaster_host
• Makefile: add test-race and test-docker targets; test-docker fails fast when the Docker daemon is unreachable instead of skipping
• queueworker / recov tests: replace time.Sleep polling with channel-based synchronization to avoid intermittent failures
• tests/unittest: gate the full race sweep and the Docker-backed database tests

Testing

• The race gate covers all CubeMaster unit-test packages (-race -count=1)
• The Docker gate covers MySQL/PostgreSQL-related DAO and template-center tests, run on the host against the Docker daemon
• The regular -short suite is unaffected

[cubesandboxbot]

The Docker gate can silently pass with zero tests executed. go test -run exits 0 with "no tests to run" when the regex matches nothing, so if any of the six hardcoded test names is renamed/split or this regex drifts, the gate goes green without exercising anything — defeating the purpose of a hard docker-backed gate. Consider asserting the expected tests actually ran (e.g. fail when a package reports zero matching tests, or parse go test -v/-json output for each expected test name).

[cubesandboxbot]

This race sweep covers ./pkg/templatecenter, whose gomonkey-based tests the existing test target already documents as needing -gcflags=all=-l because "a stub intermittently fails to take effect." gomonkey patches a function's entry instructions; the race detector instruments function prologues with tsan calls, changing the instruction layout gomonkey rewrites. This combination is a known source of intermittent "stub did not take effect" failures — worth validating this gate is stable across several CI runs before relying on it. If it flakes, scoping -race to packages that don't use gomonkey would reduce noise.

[cubesandboxbot]

AI-generated review of PR #1491 (test(cubemaster): add race and docker-backed test gates, de-flake unit tests) against the base branch. Reviewed on the base tree; the workflow matrix / CI jobs could not be executed from here.

Overall

Solid, well-scoped test-infra PR. The channel-based synchronization rewrites are correct and are genuine de-flakes — the old code also contained non-atomic shared writes (got = true, plain reads of panicTime) that the new -race gate would have flagged. The Makefile targets and workflow run-mode dispatch are coherent, and the six names in DOCKER_TEST_PATTERN all exist and are exactly the docker-backed tests in pkg/base/db + pkg/templatecenter (verified against the base tree). Findings below are non-blocking.

Findings

1. test-docker can silently pass with zero tests run (CubeMaster/Makefile, inline comment).
   The gate is a hardcoded anchored -run list. go test exits 0 with [no tests to run] if any of the six names drifts (rename/re-home/delete), so the gate reports green while enforcing nothing — the exact failure mode a gate is supposed to prevent. Recommend asserting the filter matched (e.g. go test -list count) or inverting selection so testing.Short()/CUBEMASTER_REQUIRE_DOCKER_TESTS decides. Future docker tests added to those packages also won't join the gate unless the list is edited.

2. Job multiplication: 6 CubeMaster jobs per PR (.github/workflows/unit-test-check.yml, inline comment).
   Race and Docker variants are inserted before the amd64/arm64 fan-out, so every CubeMaster change runs base + Race + Docker on both architectures. The Docker variant pulls mysql:8.0/postgres:16-alpine via dockertest on arm64 to re-validate arch-independent SQL. Consider restricting the Docker variant (and deciding whether the long -race sweep truly needs both arches) to control CI cost.

3. The race sweep is broader than the de-flaked packages — worth confirming green before it becomes a required check.
   test-race runs -race -count=1 over all of COVERAGE_PACKAGES, including the many gomonkey runtime-patching tests in pkg/templatecenter, pkg/service, pkg/selector, pkg/localcache, etc. gomonkey rewrites function machine code, which interacts with race instrumentation in ways that can surface as flakes/failures on top of any latent data races. The PR only fixes races in queueworker/recov; if the full sweep hasn't been run locally on both amd64 and arm64 yet, expect a first CI run that may need follow-up fixes.

4. Minor: test-race does go clean -testcache although -count=1 already bypasses the test-result cache (harmless; matches the existing test: target). In run.sh, cubemaster-race invokes builder-run without ensuring the local cube-sandbox-builder:ubuntu2004 image exists — consistent with the other builder-run entries in that script (users are expected to make builder-image first), so only a footnote.

What's good

• The recov rewrites are race-clean: panicTime is now written/read atomically and done gives a real completion signal instead of a fixed 2s sleep. Both tests correctly model WithRetry semantics (3 panics → handler invoked exactly 3 times).
• TestQueueBlock no longer has the got data race and the 50 ms "must block" probe cannot false-fire (it only triggers if BPop returns while the queue is provably empty).
• Failing fast on an unreachable Docker daemon (docker info guard + skipOrFail* → Fatal under CUBEMASTER_REQUIRE_DOCKER_TESTS=1/CI=true) is the right behavior for a gate — it avoids the silent green the old skip-based approach would give.

[cubesandboxbot]

The test-docker gate can silently pass with zero tests executed. If any of these six test names is renamed, moved to another package, or deleted, go test -run '^(...)$' exits 0 with [no tests to run] (both packages still compile), so CI stays green while the Docker coverage the gate exists to enforce quietly disappears.

Since the whole point of this gate is to force the Docker-backed tests to run in CI (they're skipped under -short), I'd suggest a self-check that the filter actually matched something — e.g. run go test -list '^...$' ./pkg/base/db ./pkg/templatecenter first and fail unless the expected 6 top-level tests are all present, or invert the selection (drop -run and run the two packages without -short, letting the CUBEMASTER_REQUIRE_DOCKER_TESTS/testing.Short() helpers in mysql_testutil_test.go/postgres_testutil_test.go decide). A hardcoded name list also means any future Docker test added to these packages following the same env-helper pattern won't join the gate until someone edits this line.

[cubesandboxbot]

The special CubeMaster entries are generated before the amd64/arm64 fan-out, so every CubeMaster-touching PR now runs 6 CubeMaster jobs: base + Race + Docker, each on both amd64 and arm64. The Docker variant spins up mysql:8.0/postgres:16-alpine via dockertest on the host runner — doing that on the arm64 runner too means pulling multi-arch DB images and running containers to re-validate arch-independent SQL logic. If the arm64 DB coverage isn't specifically needed, consider restricting the Docker variant to amd64 (and confirming whether the full -race sweep needs to run on both arches, since it is the longest job).