test(cmd/gc): cut managed-dolt + stale-key delay overhead in city runtime tests#2069
Open
hexsprite wants to merge 5 commits into
Open
test(cmd/gc): cut managed-dolt + stale-key delay overhead in city runtime tests#2069hexsprite wants to merge 5 commits into
hexsprite wants to merge 5 commits into
Conversation
github-actions
Bot
added
the
status/needs-triage
randy-release-manager
Bot
added
kind/chore
hexsprite
force-pushed
the
bugfix/test-leaks-managed-dolt
branch
from
41fee5b to
6e149bb
Compare
`gc prompt` and `gc prompt synth` were added to the CLI without regenerating docs/reference/cli.md from cmd/genschema. The pre-commit hook auto-regenerates this file when staged Go changes touch CLI definitions, so the drift surfaced on an unrelated PR's commit. Run: `go run ./cmd/genschema`
Adds TestCLIDocsFreshness, which walks the live cobra tree and asserts every non-hidden command has a heading in docs/reference/cli.md. The prior pre-commit hook silently regenerated cli.md and stapled it to whichever commit happened to touch Go files, so CLI drift leaked onto unrelated PRs. This catches the drift in CI/pre-commit and points at the right fix: go run ./cmd/genschema. Structural rather than byte-equal — cobra registers `completion`/`help` only on Execute, which the in-test render path skips.
3 tasks
Two tests build CityRuntime directly, skipping newTestCityRuntime. ensureManagedDoltPublishedForRuntime spawns real dolt sql-server at construction/tick even with stub callbacks, leaving orphaned children. Add cleanupManagedDoltTestCity(t, cityPath) for both tests. Refs hq-bgb3v
hexsprite
force-pushed
the
bugfix/test-leaks-managed-dolt
branch
from
6e149bb to
7e09c8f
Compare
3 tasks
Three city-runtime tests still spawn real managed dolt sql-servers via the controllerState bead store + the order-tracking sweep store opened during newCityRuntime. Each spawn costs ~12s and leaks a child until the test's cleanup runs. Extract two package-level vars (newControllerStateOpenCityStore, newCityRuntimeOpenSweepStore) so tests can swap in in-memory stores. Add stubManagedDoltStoreOpeners helper and apply to: - TestCityRuntimeEnsureManagedDoltPublishedForTickLogsOwnershipError - TestCityRuntimeRunStartupPreflightsManagedDoltBeforeSessionSnapshot - TestCityRuntimeControlDispatcherPreflightsManagedDoltBeforeSessionSnapshot Production behavior unchanged; only test code rebinds the vars. The managed-dolt preflight ordering invariant is still verified via the existing fake managedDoltHealth/Owned/Port hooks installed by each test.
…ests Five circuit-breaker reconciler tests loop through the start path many times. Each iteration paid 2s on the cmd/gc-layer staleKeyDetectDelay plus another 2s on the matching internal/session constant, which dominated wall-time despite the Fake runtime being synchronous (the post-start IsRunning check always succeeds, so no wait is actually needed). Promote both constants to package-level vars and add session.SetStaleKeyDetectDelayForTest, then introduce shortenStaleKeyDetectDelayForTest in cmd/gc that zeroes both delays for the duration of t. Apply to: - TestReconciler_CircuitTracksRestartsForSessionsCreatedThroughNamedSessions - TestReconciler_CircuitDisabledByDefaultAllowsRepeatedWakeAttempts - TestReconciler_CircuitUsesConfiguredDaemonThresholds - TestReconciler_CircuitDoesNotRecordRestartForWakeBudgetDeferredNamedSession - TestReconciler_CircuitTripsThroughRepeatedWakeAttempts Production behavior unchanged; the helper only flips the vars during t.
hexsprite
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Cuts managed-dolt spawn + stale-key wait overhead across several
cmd/gctests.Three commits, all test-only changes (one tiny
var-instead-of-constswap in production code so tests can rebind):1.
test: cleanup managed-dolt in city runtime tests(7e09c8f)Two tests in
cmd/gc/city_runtime_test.gobuiltCityRuntimedirectly instead of going throughnewTestCityRuntime, skipping thet.Cleanupthat tears down the managed dolt sql-server. Even with stubManagedDoltHealth/Owned/Portcallbacks,ensureManagedDoltPublishedForRuntimeran unconditionally and spawned a realdolt sql-serverrooted at the test's temp city. Every run left an orphan.Fix: register
cleanupManagedDoltTestCity(t, cityPath)for both, matching the pattern inTestCityRuntimeRunStartupPreflightsManagedDoltBeforeSessionSnapshot.2.
test(cmd/gc): stub managed-dolt store openers to skip dolt spawn(ec04dd6)Three more city-runtime tests still spawn real managed dolt sql-servers via the controllerState bead store + the order-tracking sweep store opened during
newCityRuntime. Each spawn costs ~12s and leaks a child until the test's cleanup runs.Extract two package-level vars (
newControllerStateOpenCityStore,newCityRuntimeOpenSweepStore) so tests can swap in in-memory stores. AddstubManagedDoltStoreOpenershelper and apply to:TestCityRuntimeEnsureManagedDoltPublishedForTickLogsOwnershipErrorTestCityRuntimeRunStartupPreflightsManagedDoltBeforeSessionSnapshotTestCityRuntimeControlDispatcherPreflightsManagedDoltBeforeSessionSnapshotProduction behavior unchanged; only test code rebinds the vars. The managed-dolt preflight ordering invariant is still verified via the existing fake
managedDoltHealth/Owned/Porthooks installed by each test.3.
test(reconciler): shorten stale-key detect delay in circuit-breaker tests(2132a72)Five circuit-breaker reconciler tests loop through the start path many times. Each iteration paid 2s on the
cmd/gc-layerstaleKeyDetectDelayplus another 2s on the matchinginternal/sessionconstant, which dominated wall-time despite the Fake runtime being synchronous (the post-startIsRunningcheck always succeeds, so no wait is actually needed).Promote both constants to package-level vars and add
session.SetStaleKeyDetectDelayForTest, then introduceshortenStaleKeyDetectDelayForTestincmd/gcthat zeroes both delays for the duration oft. Applied to fiveTestReconciler_Circuit*tests.Repro
Before: 10+ orphans with config paths like
/var/folders/.../TestCityRuntime*accumulate after the suite.After: only the legit managed dolt for the active city remains.
Test plan
go test -run 'TestCityRuntimeEnsureManagedDoltPublished|TestCityRuntimeRunStartupPreflightsManagedDoltBeforeSessionSnapshot|TestCityRuntimeControlDispatcherPreflightsManagedDoltBeforeSessionSnapshot|TestReconciler_Circuit' ./cmd/gc/...pgrep -fl 'dolt sql-server'after run — only legit dolt survivesmake test— greenRefs hq-bgb3v