From 06d78b66d3fa5d99b94fc576860ffb177d3c0dd4 Mon Sep 17 00:00:00 2001 From: "courtmanr@gmail.com" Date: Tue, 29 Sep 2026 13:14:14 +0100 Subject: [PATCH] Balance internal/api race shards by measured run time The four Backend tests (api-N) shards took equal-count quarters of go test's list, which left them at 4.0, 10.6, 4.0 and 16.8 minutes on the first PR run, and at 1.9, 8.0, 1.5 and 10.3 minutes of go test time on the following main push (run 36560070496). Almost all of it sits in a few tests. One contract mock chart test runs about six minutes on the runner, and the 28 integration server tests at the end of the list take about twenty seconds each, so the fourth quarter held nearly half the package. Shards are still contiguous runs of go test's own order, since some tests depend on package state from their neighbours, but the cut points now come from estimated seconds. .github/scripts/select-internal-api-shard.sh weighs each test from .github/scripts/internal-api-test-seconds.txt (the 199 tests at 0.5s or more, seeded from a local -race -json run scaled to that main run's shard totals, with a 0.05s default for anything unlisted), binary searches the smallest per-shard budget that fits the list into four slices, and fills up to it. The weights only move cut points. Every test is assigned exactly one shard in a single pass, so a stale file can unbalance shards but never drop or repeat a test, and no shard is ever empty. With today's list the slices hold 1187, 23, 2556 and 21 tests, estimated at 1.9, 6.9, 6.5 and 6.7 minutes of tests, or about 9.5 minutes per job with the roughly 2.6 minutes of setup and race compile. The 2556-test slice would need a 124 KB -run pattern, so a shard passes -skip of the other shards' tests whenever that is shorter, which runs the same tests in the same order. --- .github/scripts/internal-api-test-seconds.txt | 212 ++++++++++++++++++ .github/scripts/select-internal-api-shard.sh | 107 +++++++++ .github/workflows/build-and-test.yml | 38 +++- .../subsystems/deployment-installability.md | 21 +- .../v6/internal/subsystems/registry.json | 4 + .../installtests/build_release_assets_test.go | 99 +++++++- 6 files changed, 466 insertions(+), 15 deletions(-) create mode 100644 .github/scripts/internal-api-test-seconds.txt create mode 100755 .github/scripts/select-internal-api-shard.sh diff --git a/.github/scripts/internal-api-test-seconds.txt b/.github/scripts/internal-api-test-seconds.txt new file mode 100644 index 0000000000..d3465b3b52 --- /dev/null +++ b/.github/scripts/internal-api-test-seconds.txt @@ -0,0 +1,212 @@ +# Estimated -race run time in seconds of the slowest internal/api top-level +# tests on a GitHub ubuntu-24.04 runner. Used only by +# select-internal-api-shard.sh to choose where the Backend tests (api-N) +# shards cut go test's list. Tests not named here weigh DEFAULT_WEIGHT (0.05s, +# the measured mean of the unlisted tests). A missing, stale or renamed entry +# can make the shards uneven but can never drop or repeat a test. +# +# Refresh when one api shard runs well over the others: run +# GOMAXPROCS=4 go test -race -json ./internal/api +# and list every top-level test at 0.5s or more. Seeded 2026-09-29 from such a +# local run scaled to the four shard totals of Build and Test run 36560070496 +# (the contract mock chart test runs about 1.4x slower on the runner, the +# wait-bound integration server tail about 0.8x). +TestActionRunnerRotationProductionRouterTLSPersistenceRestart 0.69 +TestMockActionInboxAndDetailUseCanonicalResponses 2.40 +TestMockActionInboxHonorsLimitAndLegacyPendingProjection 1.79 +TestMockActionMutationsFailReadOnly 0.98 +TestAgentReportHandlersClassifyBodyFailures 1.10 +TestStart_MockModeStartsChatServiceWhenPersistedConfigDisabled 0.95 +TestRestart_MockModeStartsChatServiceWhenPersistedConfigDisabled 0.95 +TestHandleChat_EmitsIdleProgressWhileServiceIsSilent 2.40 +TestAISettingsHandler_ProviderSettersConcurrentServiceCreation 8.31 +TestHandleSaveGuestNote_VerifySavedFields 0.52 +TestHandleClearGuestKnowledge_OversizedBody 0.57 +TestHandlePatrolStream_LastEventIDHeaderTakesPrecedenceOverQueryFallback 1.99 +TestHandleExecuteStream_LicenseRequired 1.05 +TestHandleExecuteStream_PromptRequired 1.06 +TestHandleExecuteStream_Success 1.80 +TestHandleExecuteStream_EmitsIdleProgressWhileServiceIsSilent 1.74 +TestHandleInvestigateAlert_EmitsIdleProgressWhileServiceIsSilent 1.69 +TestAISettingsHandler_PatrolReadinessBranches 0.85 +TestHandleInvestigateAlert_UsesVisibleIdleProgressStream 1.73 +TestHandleGetPatterns_UnlockedWithData 1.04 +TestHandleGetPredictions_WithData 1.04 +TestHandleGetCorrelations_WithData 1.05 +TestHandleGetCorrelations_UsesCanonicalIntelligenceFacade 1.04 +TestExtractAndStoreAuthContext_UsesTenantConfigForToken 2.98 +TestAuthDenialPruneKeepsTrackedSetBounded 1.92 +TestAvailabilityHandlersListReturnsMockTargetsInMockMode 1.03 +TestAvailabilityHandlersTestSavedMockTargetUsesSyntheticStatus 1.03 +TestChangePasswordRefusesNonAdminSession 0.83 +TestChangePasswordAllowsConfiguredAdminSession 1.52 +TestChangePasswordBasicAuthPathUnaffected 4.19 +TestChangePasswordFixtureOwnsRouterLifecycle 0.84 +TestConfigTransferAnonymousAuthenticatedModesDenyBeforeBodyRead 0.50 +TestConfigTransferAuthorizedInstanceModesReachHandler 0.70 +TestConnectionsHandleListIncludesMockAvailabilityTargets 1.13 +TestConnectionsHandleListDropsRealSourcesInMockMode 1.19 +TestConnectionsLedgerKeepsRealSourcesWhenRealPollingRetained 1.15 +TestContract_MockAvailabilityTargetsUseSavedTargetPayloads 1.13 +TestContract_MockDiscoveryEndpointsUseCanonicalPayloads 1.05 +TestContract_MockChartRoutesUseCanonicalMockUnifiedReadStateForVMwareHosts 391.48 +TestContract_PlatformMockToggleRebindsRuntimeConnectionsAndResources 19.96 +TestContract_PlatformMockConnectionListsUseSharedFixtureMetadata 1.08 +TestContract_RecoveryPointsMockPathReturnsCanonicalProviderBackedFixtures 1.61 +TestContract_SecurityStatusPresentationPolicyProEditionCommercialContext 0.50 +TestContract_StateAndResourceListShareCanonicalMockResourceContract 4.64 +TestContract_AgentCommandConfigGateMirrorsChannelAdmission 1.13 +TestContract_HostedDiagnosticsDockerPrepareTokenValidatesOriginBeforeMutation 1.22 +TestContract_HostedInstallerOriginsFailClosedAtRouter 4.04 +TestContract_RuntimeDisplayServesPresentationValuesWithoutAdmin 0.84 +TestSecurityStatusCurrentUserForScopedLocalSession 0.84 +TestSecurityStatusOrgBoundSettingsFollowOrgManagementRule 0.84 +TestSecurityStatusOrgBoundReportingRespectsAuthorizer 0.83 +TestDiscoveryHandlers_MockModeServesFixturesWithoutService 1.13 +TestDockerAgentReportSizeContractAcceptsOrdinaryAndLargeFleets 0.55 +TestDockerAgentReportSizeContractBoundaries 2.57 +TestCollectorUninstallTransactionFailsClosedAndRetriesAfterRestart 2.15 +TestHostedTenantAgentInstallCommand_GeneratesOrgBoundTokenAndCommand 3.13 +TestGenerateHostedTenantAgentInstallCommandEnforcesHostedOrgBoundary 1.11 +TestHostedTenantAgentInstallCommandCarriesCommandPolicyIntent 1.49 +TestHostedTenantAgentInstallTokenCannotReportToOtherTenant 0.50 +TestIssue1753SameNameProxmoxAgentsAuthenticateAndStayDistinctEndToEnd 2.20 +TestSyncReleaseDemoFixtureRuntime_DoesNotPoliceMockFixturesInDevBuilds 1.23 +TestMetricsHistoryFallbackSynthesizesUnseededMockWorkloadHistory 1.64 +TestMetricsHistoryFallbackSynthesizesMockHostAndNodeDrawerSeries 1.99 +TestMetricsHistoryFallbackSupplementsPartialMockStoreSeries 1.68 +TestMetricsHistoryFallbackMockDiskSynthesizesSeries 1.65 +TestMetricsHistoryFallbackMockUnifiedStorageSynthesizesSeries 2.27 +TestMetricsHistoryFallbackMockStorageBypassesSparseStoreSeries 2.47 +TestSSOOnlyExplicitAdminReachesEveryAdminGuard 1.42 +TestUnrelatedSSOUserStillRefusedWhenLocalAdminConfigured 2.10 +TestPatrolActionBrokerAutonomyModeIsAnUpperBound 0.53 +TestPlatformAdminRouteRefusesUnrelatedSSOUser 0.84 +TestJourneyGeneralAPIBudgetIsolation 1.75 +TestTenantRBACProvider_ConcurrentGetAndRemove 0.52 +TestTenantRBACProvider_NoConnectionLeak 1.43 +TestHandleListPointsAcceptsCanonicalPlatformQuery 1.82 +TestHandleListRollupsExposeCanonicalPlatformsPayload 1.86 +TestHandleListRollupsMetaReportsNormalizedPagination 2.71 +TestHandleListPointsMetaReportsNormalizedPagination 2.74 +TestStartPatrolForContext_DoesNotOverwriteOtherTenantPatrolComponents 2.30 +TestStartPatrolForContext_RejectsMismatchedAIServiceOrg 1.24 +TestRouterMockMode_SeedsTrueNASAndVMwareSupplementalResources 1.58 +TestRouterMockMode_SeedsVMwareSupplementalActivity 1.21 +TestRouteExecuteStream_Success 0.94 +TestRouteExecuteStream_MethodNotAllowed 0.76 +TestRouteExecuteStream_NoAuth 0.60 +TestRouteExecuteStream_WrongScope 0.65 +TestRouteExecuteStream_EmptyPrompt 0.83 +TestRouteExecuteStream_WhitespaceOnlyPrompt 0.73 +TestRouteExecuteStream_InvalidJSON 0.80 +TestRouteExecuteStream_AIDisabled 0.78 +TestRouteExecuteStream_AutofixLicenseRequired 0.75 +TestRouteExecuteStream_RemediationLicenseRequired 0.66 +TestRouteExecuteStream_OversizedBody 0.82 +TestRouteExecuteStream_UseCaseCaseInsensitive 0.86 +TestRouteExecuteStream_SSEHeaders 0.66 +TestRouteExecuteStream_CompleteEventBeforeDone 0.76 +TestRouteExecuteStream_EmptyBody 0.68 +TestRouteExecuteStream_NullPrompt 0.74 +TestRouteExecuteStream_EmptyJSONObject 0.55 +TestRouteExecuteStream_ServiceError 0.69 +TestRouteExecuteStream_InvalidTargetType 0.60 +TestRouteExecuteStream_InvalidHistoryRole 0.70 +TestRouteExecuteStream_TargetIDTooLong 0.58 +TestRouteExecuteStream_ValidTargetTypes 2.08 +TestRouteExecuteStream_EmptyTargetTypeAllowed 1.50 +TestRouteExecuteStream_WithConversationHistory 0.58 +TestRouteExecuteStream_WithTargetContext 0.75 +TestRouteExecuteStream_DefaultUseCaseIsChat 0.73 +TestRouteExecuteStream_FindingIDPassedThrough 0.73 +TestRouteExecuteStream_ModelFieldPassedThrough 0.63 +TestRouteExecute_Success 0.74 +TestRouteExecute_MethodNotAllowed 0.75 +TestRouteExecute_NoAuth 0.79 +TestRouteExecute_WrongScope 0.69 +TestRouteExecute_EmptyPrompt 0.74 +TestRouteExecute_InvalidJSON 0.75 +TestRouteExecute_WithConversationHistory 0.68 +TestRouteExecute_WithTargetContext 0.87 +TestRouteExecute_DefaultUseCaseIsChat 0.73 +TestRouteExecute_AutofixLicenseRequired 0.75 +TestRouteExecute_RemediationLicenseRequired 0.56 +TestRouteExecute_OversizedBody 0.93 +TestRouteExecute_EmptyBody 0.66 +TestRouteExecute_EmptyJSONObject 0.94 +TestRouteExecute_NullPrompt 0.75 +TestRouteExecute_ServiceError 0.73 +TestRouteExecute_InvalidTargetType 0.75 +TestRouteExecute_ValidTargetTypes 2.17 +TestRouteExecute_EmptyTargetTypeAllowed 0.75 +TestRouteExecute_InvalidHistoryRole 0.73 +TestRouteExecute_FindingIDPassedThrough 0.76 +TestRouteExecute_ModelFieldPassedThrough 0.73 +TestRouteListModels_Success 0.68 +TestRouteListModels_MethodNotAllowed 0.66 +TestRouteListModels_WrongScope 0.72 +TestRouteListModels_ProviderError 0.75 +TestRouteListModels_EmptyModelList 0.76 +TestRouteListModels_ResponseContentType 0.63 +TestRouteListModels_CachedField 0.76 +TestRouteListModels_MalformedProviderResponse 0.78 +TestRouteListModels_HEADMethod 0.87 +TestRouteTestConnection_Success 0.76 +TestRouteTestConnection_MethodNotAllowed 0.72 +TestRouteTestConnection_NoAuth 0.74 +TestRouteTestConnection_WrongScope 0.74 +TestRouteTestProvider_Success 0.72 +TestRouteTestProvider_UnconfiguredProvider 0.66 +TestRouteTestProvider_ConnectionFailure 0.60 +TestRouteTestProvider_MethodNotAllowed 0.76 +TestRouteTestProvider_NoAuth 0.75 +TestRouteTestProvider_MultipleProviders 0.74 +TestRouteTestProvider_UnknownProvider 0.60 +TestRouteTestProvider_WrongScope 0.75 +TestRouteTestProvider_InvalidProviderNames 0.74 +TestRouteTestProvider_PathTraversal 0.85 +TestRouter_HandleState_MockIsolation 4.14 +TestAITestConnectionRouteWithValidScope 0.75 +TestAITestConnectionRouteRejectsWrongScope 0.82 +TestAITestProviderRouteRejectsWrongScope 0.74 +TestPublicEndpointsBypassAuthInAPIMode 0.61 +TestSettingsCapabilitiesMatchRouteEnforcementWithoutRBAC 0.86 +TestSettingsCapabilitiesGrantConfiguredAdminWithoutRBAC 0.93 +TestTrueNASHandlers_HandleList_ReturnsMockConnectionsInMockMode 1.23 +TestFreeBSDOfflineAgentPreflight 1.83 +TestBranchcov0723AmExtractFromZip 8.86 +TestHandleApplyUpdate_BlocksWhenReadinessBlocked 1.47 +TestHandleGetUpdatePlan 1.65 +TestHandleGetUpdatePlan_IncludesUpgradeReadiness 1.34 +TestHandleGetUpdatePlan_InvalidChannel 1.09 +TestHandleGetUpdatePlan_PrepareError 1.43 +TestVMwareHandlers_HandleList_ReturnsMockConnectionsInMockMode 1.33 +TestAlertsEndpoints 20.56 +TestRecoveryRollupsEndpointToleratesMalformedPersistedMetadata 0.55 +TestHealthEndpoint 19.04 +TestVersionEndpointUsesRepoVersion 19.47 +TestAlertAcknowledge_AllowsPrintableAlertIDs 18.81 +TestStateEndpointReturnsMockData 20.72 +TestRecoveryRollupsEndpointReturnsMockData 20.65 +TestProtectedEndpointsRequireAuthentication 18.62 +TestAPIOnlyModeRequiresToken 21.79 +TestServerInfoEndpointReportsDevelopment 18.85 +TestRecoveryPointsEndpointReturnsMockData 21.75 +TestServerInfoEndpointMethodNotAllowed 18.65 +TestHealthEndpointMethodNotAllowed 18.76 +TestHealthEndpointHeadAllowed 18.68 +TestConfigNodesUsesMockTopology 17.84 +TestMockModeToggleEndpoint 25.43 +TestMockModeToggleFromRealDataRebindsPlatformSupplementalProviders 19.24 +TestAuthenticatedEndpointsRequireToken 19.40 +TestAPITokenQueryAndHeaderAuth 23.11 +TestRecoveryEndpointRequiresDirectLoopback 20.32 +TestWebSocketSendsInitialState 19.38 +TestWebsocketPayloadContractShape 17.62 +TestWebsocketPayloadUsesCanonicalStateContract 19.56 +TestSessionCookieAllowsAuthenticatedAccess 20.16 +TestRevokedAPITokenImmediatelyLosesAccess 24.33 +TestLimitedAPITokenCannotCreateBroaderToken 20.31 +TestPublicURLDetectionUsesForwardedHeaders 17.31 +TestPublicURLDetectionRespectsEnvOverride 19.14 diff --git a/.github/scripts/select-internal-api-shard.sh b/.github/scripts/select-internal-api-shard.sh new file mode 100755 index 0000000000..3a0cf0cd4f --- /dev/null +++ b/.github/scripts/select-internal-api-shard.sh @@ -0,0 +1,107 @@ +#!/usr/bin/env bash +# Print the internal/api top-level tests that belong to one CI shard. +# +# Usage: select-internal-api-shard.sh +# The full test list, in go test's own order, is read from stdin. +# +# Shards are contiguous runs of go test's order, because some internal/api +# tests still depend on package state left by the tests just before them. +# Cut points are chosen by estimated run time instead of test count. Every +# test weighs its seconds from the weights file, or DEFAULT_WEIGHT when the +# file does not name it (new tests, renamed tests). The smallest per-shard +# capacity that lets the list be cut into at most contiguous +# runs is found by binary search, then the list is filled greedily up to that +# capacity. If fewer runs than shards would result, the tail is cut early so +# every shard still gets at least one test. +# +# The weights only move the cut points. Each test is assigned exactly one +# shard index in 0..count-1 by a single pass over the list, so a stale, +# missing or wrong weight can make shards uneven but can never drop or repeat +# a test. +set -euo pipefail + +if [ "$#" -ne 3 ]; then + echo "usage: $0 < tests" >&2 + exit 2 +fi +weights=$1 +count=$2 +index=$3 + +case "$count" in '' | *[!0-9]*) echo "shard count must be a positive integer" >&2; exit 2 ;; esac +case "$index" in '' | *[!0-9]*) echo "shard index must be a non-negative integer" >&2; exit 2 ;; esac +if [ "$count" -lt 1 ] || [ "$index" -ge "$count" ]; then + echo "shard index ${index} is outside 0..$((count - 1))" >&2 + exit 2 +fi +if [ ! -f "$weights" ]; then + echo "weights file ${weights} not found" >&2 + exit 2 +fi + +awk -v n="$count" -v want="$index" -v default_weight="${DEFAULT_WEIGHT:-0.05}" ' + function centis(seconds, c) { + c = int(seconds * 100 + 0.5) + return c < 1 ? 1 : c + } + # Number of contiguous runs a greedy fill up to capacity cap produces. + function runs(cap, k, used, groups) { + groups = 1 + used = 0 + for (k = 1; k <= total; k++) { + if (used > 0 && used + cost[k] > cap) { + groups++ + used = 0 + } + used += cost[k] + } + return groups + } + FILENAME == ARGV[1] { + if ($0 ~ /^[[:space:]]*(#|$)/) next + if (NF != 2 || $2 !~ /^[0-9]+(\.[0-9]+)?$/ || $2 + 0 <= 0) { + printf "invalid weights line %d: %s\n", FNR, $0 > "/dev/stderr" + bad = 1 + exit 1 + } + weight[$1] = $2 + 0 + next + } + NF { + total++ + name[total] = $1 + cost[total] = centis(($1 in weight) ? weight[$1] : default_weight) + sum += cost[total] + if (cost[total] > heaviest) heaviest = cost[total] + } + END { + if (bad) exit 1 + if (total < n) { + printf "%d tests cannot fill %d shards\n", total, n > "/dev/stderr" + exit 1 + } + lo = heaviest + hi = sum + while (lo < hi) { + mid = int((lo + hi) / 2) + if (runs(mid) <= n) hi = mid + else lo = mid + 1 + } + shard = 0 + used = 0 + for (k = 1; k <= total; k++) { + if (used > 0 && used + cost[k] > lo) { + shard++ + used = 0 + } + used += cost[k] + if (shard == want) print name[k] + # Once the tests left equal the shards left, give each its own shard + # so no shard is empty. + if (total - k > 0 && total - k == n - 1 - shard) { + shard++ + used = 0 + } + } + } +' "$weights" - diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml index b3419e5747..cc54b9a6d7 100644 --- a/.github/workflows/build-and-test.yml +++ b/.github/workflows/build-and-test.yml @@ -323,9 +323,9 @@ jobs: # internal/api alone took ~27 of the ~31 minutes of the old single # `Backend tests (api)` job under -race, which made it the critical path of # every pull request. Its top-level tests are split across these shards - # as contiguous runs of the package's own test order, and the - # `Backend tests (api)` job below keeps the required check name by passing - # only when every shard passed. + # as contiguous runs of the package's own test order, cut by estimated run + # time, and the `Backend tests (api)` job below keeps the required check + # name by passing only when every shard passed. backend-api: name: Backend tests (api-${{ matrix.index }}) needs: changes @@ -379,9 +379,15 @@ jobs: # tests still lean on package state left by the tests just before # them (for example admin bypass or session store globals), and an # interleaved split by name broke dozens of them, while contiguous - # slices keep those neighbours together. Listing with -race builds - # the same test binary the run then reuses from cache. Benchmarks - # are left out because plain go test never runs them. + # slices keep those neighbours together. The slices are cut by + # estimated run time from .github/scripts/internal-api-test-seconds.txt + # rather than by count, because a handful of tests take most of the + # time (one contract test runs about six minutes and the integration + # server tail about twenty seconds per test). Unlisted tests get a + # small default weight, and the weights only move the cut points, so + # coverage never depends on them. Listing with -race builds the same + # test binary the run then reuses from cache. Benchmarks are left out + # because plain go test never runs them. run: | set -euo pipefail listing=$(go test -race -list . ./internal/api) @@ -391,21 +397,33 @@ jobs: exit 1 fi total=$(printf '%s\n' "$tests" | wc -l | tr -d ' ') - selected=$(printf '%s\n' "$tests" | awk -v n="$API_SHARD_COUNT" -v i="$API_SHARD_INDEX" -v total="$total" \ - 'NR > int(total * i / n) && NR <= int(total * (i + 1) / n)') + selected=$(printf '%s\n' "$tests" | bash .github/scripts/select-internal-api-shard.sh \ + .github/scripts/internal-api-test-seconds.txt "$API_SHARD_COUNT" "$API_SHARD_INDEX") if [ -z "$selected" ]; then echo "::error::internal/api shard ${API_SHARD_INDEX} of ${API_SHARD_COUNT} resolved to an empty test list" exit 1 fi + others=$(printf '%s\n' "$tests" | grep -vxF -f <(printf '%s\n' "$selected") || true) + # Name whichever side is shorter. A shard holding most of the tests + # is passed as -skip of every other shard's tests, which runs the + # same set in the same order. pattern="^($(printf '%s\n' "$selected" | paste -sd '|' -))\$" + filter=(-run "$pattern") + if [ -n "$others" ]; then + skip="^($(printf '%s\n' "$others" | paste -sd '|' -))\$" + if [ "${#skip}" -lt "${#pattern}" ]; then + pattern=$skip + filter=(-run . -skip "$pattern") + fi + fi # Linux rejects a single argument over 128 KiB. Fail with a clear # instruction instead of an opaque exec error. if [ "${#pattern}" -gt 120000 ]; then echo "::error::internal/api shard pattern is ${#pattern} bytes; add a shard index and raise API_SHARD_COUNT" exit 1 fi - echo "internal/api shard ${API_SHARD_INDEX} of ${API_SHARD_COUNT} runs $(printf '%s\n' "$selected" | wc -l | tr -d ' ') of ${total} tests" - go test -race -timeout 50m -run "$pattern" ./internal/api + echo "internal/api shard ${API_SHARD_INDEX} of ${API_SHARD_COUNT} runs $(printf '%s\n' "$selected" | wc -l | tr -d ' ') of ${total} tests (${selected%%$'\n'*} .. ${selected##*$'\n'})" + go test -race -timeout 50m "${filter[@]}" ./internal/api # Required check name. Branch protection matches `Backend tests (api)` # exactly, so this job must keep that name and must fail unless every diff --git a/docs/release-control/v6/internal/subsystems/deployment-installability.md b/docs/release-control/v6/internal/subsystems/deployment-installability.md index 52aa9a4b17..1cf07668de 100644 --- a/docs/release-control/v6/internal/subsystems/deployment-installability.md +++ b/docs/release-control/v6/internal/subsystems/deployment-installability.md @@ -30,12 +30,27 @@ Build and Test runs the `internal/api` race tests as four `Backend tests (api-N)` shards instead of one job, because that one package took about 27 of the 31 minutes of the old `Backend tests (api)` job and set the critical path of every pull request. Each shard lists the package's tests with `go test --race -list` from the commit under test and runs one contiguous quarter of -that list in go test's own order, so the shards cover every test exactly once, +-race -list` from the commit under test and runs one contiguous slice of that +list in go test's own order, so the shards cover every test exactly once, including tests added later, and a shard that resolves no tests fails. The order is kept on purpose. Some `internal/api` tests depend on package state left by the tests just before them, and an interleaved split by name broke -dozens of them. The required check name +dozens of them. Slices are cut by estimated run time, not test count. +Equal-count quarters ran 1.9, 8.0, 1.5 and 10.3 minutes of tests, because one +contract mock chart test runs about six minutes on the runner and the +integration server tail about twenty seconds per test. +`.github/scripts/select-internal-api-shard.sh` weighs each test by its seconds +in `.github/scripts/internal-api-test-seconds.txt` (the slowest tests only, +with a small default for every unlisted, new or renamed test), finds the +smallest per-shard budget that fits the list into the shard count, and fills +contiguous slices up to it, cutting the tail early if needed so no shard is +empty. The weights only move cut points. Each test is assigned one shard in a +single pass, so a stale weights file can unbalance the shards but never drop +or repeat a test, and the file is refreshed from a `-race -json` run when one +shard drifts well past the others. A shard holding most of the tests passes +`-skip` of every other shard's tests instead of naming its own, which runs the +same tests in the same order and keeps the single argument under the 120,000 +byte ceiling. The required check name `Backend tests (api)` belongs to a verdict job that passes only when every shard succeeded, and `Backend tests (rest-0)` and `Backend tests (rest-1)` keep their names and package split. All shards still expand for documentation-only diff --git a/docs/release-control/v6/internal/subsystems/registry.json b/docs/release-control/v6/internal/subsystems/registry.json index 9271b4a1ba..fc356b735b 100644 --- a/docs/release-control/v6/internal/subsystems/registry.json +++ b/docs/release-control/v6/internal/subsystems/registry.json @@ -4511,8 +4511,10 @@ ".dockerignore", ".github/ISSUE_TEMPLATE/v6_rc_feedback.yml", ".github/scripts/check-demo-reachability.sh", + ".github/scripts/internal-api-test-seconds.txt", ".github/scripts/recover-demo-runtime.sh", ".github/scripts/resolve-demo-runtime-profile.sh", + ".github/scripts/select-internal-api-shard.sh", ".github/scripts/setup-demo-ssh.sh", ".github/workflows/backfill-release-assets.yml", ".github/workflows/build-and-test.yml", @@ -5035,6 +5037,8 @@ "label": "frontend dependency security proof", "match_prefixes": [], "match_files": [ + ".github/scripts/internal-api-test-seconds.txt", + ".github/scripts/select-internal-api-shard.sh", ".github/workflows/build-and-test.yml", "frontend-modern/package-lock.json", "frontend-modern/package.json" diff --git a/scripts/installtests/build_release_assets_test.go b/scripts/installtests/build_release_assets_test.go index 17d3165fe9..bb72037e38 100644 --- a/scripts/installtests/build_release_assets_test.go +++ b/scripts/installtests/build_release_assets_test.go @@ -3840,11 +3840,17 @@ func TestBackendAPIShardsKeepRequiredCheckExhaustive(t *testing.T) { // The list comes from the commit under test, so a new test cannot be // missed, and contiguous slices of go test's own order put it in // exactly one shard while keeping order-coupled neighbours together. + // The checked-in weights only choose where those slices are cut. "go test -race -list . ./internal/api", - `'NR > int(total * i / n) && NR <= int(total * (i + 1) / n)'`, + "bash .github/scripts/select-internal-api-shard.sh \\\n .github/scripts/internal-api-test-seconds.txt \"$API_SHARD_COUNT\" \"$API_SHARD_INDEX\")", "resolved to an empty test list", "go test -list found no tests in ./internal/api", - `go test -race -timeout 50m -run "$pattern" ./internal/api`, + // A shard holding most tests is named by skipping every other + // shard's tests, which keeps its argument under the exec limit. + `others=$(printf '%s\n' "$tests" | grep -vxF -f <(printf '%s\n' "$selected") || true)`, + `filter=(-run . -skip "$pattern")`, + `if [ "${#pattern}" -gt 120000 ]; then`, + `go test -race -timeout 50m "${filter[@]}" ./internal/api`, "PULSE_DATA_DIR: /tmp/pulse-test-data", } { if !strings.Contains(shards, required) { @@ -3873,6 +3879,95 @@ func TestBackendAPIShardsKeepRequiredCheckExhaustive(t *testing.T) { if got := strings.Count(workflow, "name: Backend tests (api)\n"); got != 1 { t.Fatalf("exactly one job may carry the required Backend tests (api) name, found %d", got) } + + assertInternalAPIShardSelectionExhaustive(t, want) +} + +// assertInternalAPIShardSelectionExhaustive runs the shard selector the way +// the workflow does and proves that, whatever the weights say, the shards are +// non-empty contiguous slices that together cover the list exactly once in +// order. +func assertInternalAPIShardSelectionExhaustive(t *testing.T, workflowShards int) { + t.Helper() + selector := repoFile(".github", "scripts", "select-internal-api-shard.sh") + weightsPath := repoFile(".github", "scripts", "internal-api-test-seconds.txt") + weightsContent, err := os.ReadFile(weightsPath) + if err != nil { + t.Fatal(err) + } + + // Weighted names from the checked-in file interleaved with unknown ones, + // which stand in for tests added after the weights were measured. + var weighted []string + for _, line := range strings.Split(string(weightsContent), "\n") { + fields := strings.Fields(line) + if len(fields) == 0 || strings.HasPrefix(fields[0], "#") { + continue + } + if len(fields) != 2 || !strings.HasPrefix(fields[0], "Test") { + t.Fatalf("internal/api weights line must be ` `, got %q", line) + } + if seconds, err := strconv.ParseFloat(fields[1], 64); err != nil || seconds <= 0 { + t.Fatalf("internal/api weight for %s must be positive seconds, got %q", fields[0], fields[1]) + } + weighted = append(weighted, fields[0]) + } + if len(weighted) == 0 { + t.Fatal("internal/api weights file lists no tests") + } + var tests []string + for i, name := range weighted { + tests = append(tests, name) + for j := 0; j < 1+i%7; j++ { + tests = append(tests, "TestUnweightedShardProbe"+strconv.Itoa(i)+"x"+strconv.Itoa(j)) + } + } + + emptyWeights := filepath.Join(t.TempDir(), "empty.txt") + if err := os.WriteFile(emptyWeights, nil, 0o644); err != nil { + t.Fatal(err) + } + skewedWeights := filepath.Join(t.TempDir(), "skewed.txt") + skewed := tests[len(tests)/3] + " 900\n" + tests[len(tests)-1] + " 450\n" + if err := os.WriteFile(skewedWeights, []byte(skewed), 0o644); err != nil { + t.Fatal(err) + } + + selectShard := func(weights string, count, index int, input []string) ([]string, error) { + cmd := exec.Command("bash", selector, weights, strconv.Itoa(count), strconv.Itoa(index)) + cmd.Stdin = strings.NewReader(strings.Join(input, "\n") + "\n") + out, err := cmd.Output() + if err != nil { + return nil, err + } + return strings.Fields(string(out)), nil + } + + for _, weights := range []string{weightsPath, emptyWeights, skewedWeights} { + for _, count := range []int{1, 2, workflowShards, workflowShards + 1, 9} { + var combined []string + for index := 0; index < count; index++ { + selected, err := selectShard(weights, count, index, tests) + if err != nil { + t.Fatalf("select shard %d of %d with %s: %v", index, count, filepath.Base(weights), err) + } + if len(selected) == 0 { + t.Fatalf("shard %d of %d with %s selected no tests", index, count, filepath.Base(weights)) + } + combined = append(combined, selected...) + } + if strings.Join(combined, "\n") != strings.Join(tests, "\n") { + t.Fatalf("%d shards with %s do not cover the test list exactly once in order", count, filepath.Base(weights)) + } + } + } + + if _, err := selectShard(weightsPath, workflowShards, workflowShards, tests); err == nil { + t.Fatal("shard selector must reject an index outside the shard count") + } + if _, err := selectShard(weightsPath, 3, 0, tests[:2]); err == nil { + t.Fatal("shard selector must fail when there are fewer tests than shards") + } } func TestFrontendDependencySecurityAuditsAreRequired(t *testing.T) {