Skip to content

test(integration): run the OAuth and MCP test classes in parallel with the shared collection - #1286

Merged
mforce merged 3 commits into
mainfrom
test/1271-parallel-oauth-mcp
Oct 13, 2026
Merged

mforce merged 3 commits into
mainfrom
test/1271-parallel-oauth-mcp

Conversation

@mforce

@mforce mforce commented Oct 12, 2026 •

Copy link
Copy Markdown
Owner

Part of #1271

Step 1 of #1271. Eight integration test classes leave the serial IntegrationCollection and each gets its own IClassFixture<CluckworkWebApplicationFactory>, the pattern eight classes already use. With #1002 each factory gets a template-cloned database. The serial collection is one thread and decides when the integration leg ends, so these classes now run on the idle cores instead.

Moved: OAuthFailClosedTests (5 partial files), Mcp.RecordDailyEntryToolTests, Mcp.McpAuthorizationTests, Mcp.McpToolErrorTests, Mcp.McpReadToolsTests, Mcp.McpCommerceToolsTests, OAuthServerTests, AccountLifecycleCommandTests.

No assertion or test body changes. Each moved class changes in two lines: the [Collection] attribute goes and the fixture interface comes in.

Two smaller changes:

  • tools/test-timing/summarize.py now matches partial classes. The old pattern dropped OAuthFailClosedTests from the shared-collection census.
  • The two metadata tests that drain the 5-minute per-URL budget now count on a frozen clock (see below).

Audit: why each class can leave the serial collection

Class Verdict Evidence
OAuthFailClosedTests Safe Each test builds its own host (Host(), OAuthFailClosedTests.cs:766), so IFixedWindowCounter, OpenIddict state and the Probe filter belong to that host. The factory configures no Redis, only a key namespace per factory. MetadataDocumentServer starts per test on port 0, and BudgetResolver gives each global-budget test a new counter. The one read outside its own tenant is the default account's AllowConnectedApps in ConnectedApps_AreOnByDefault…. No other class writes that value, and a fresh clone holds the migration default that the test asserts. Other counts filter by the test's own subject, client id or URL.
Mcp.RecordDailyEntryToolTests Safe Host per test (McpConnection.Host). Each test seeds its own farms. Every count is tenant-scoped or filters by the test's own account or flock id, including the IdempotencyRecords count. In its own database, the global purge sweeps of other classes cannot reach its rows.
Mcp.McpAuthorizationTests Safe Host per test, with per-host settings only (AllowedHosts=* at :140). Own farms.
Mcp.McpToolErrorTests Safe Host per test, with per-host service overrides (:156). Own farms.
Mcp.McpReadToolsTests Safe The Assert.Empty checks (:39–41) are MCP reads by the test's own tenant. A foreign farm it seeded must not appear. The reads are tenant-filtered, so an empty result means the same in any database.
Mcp.McpCommerceToolsTests Safe Same pattern as the read-tools class.
OAuthServerTests Safe Client ids are client-{Guid} (:248). The token read at :55 filters by the test's own user id. OAuthClientRegistrationTests stays in the shared collection and calls new OAuthServerTests(factory) with its own factory, so the two classes share helper code, not state.
AccountLifecycleCommandTests Safe Its subprocesses get ConnectionStrings__Default = factory.ConnectionString (:25), the class's own cloned database. They are run-then-exit verbs with no port and no hosted services. Slugs come from the seeded account id. missing-farm is a deliberate unknown code. All counts filter by the test's own account id.

Shared state checked for all eight:

  • src/ has no mutable process-wide statics.
  • The test project has one process-wide map, the refresh-cookie map at TestHarness.cs:29. It is keyed by a random token, so classes cannot collide in it.
  • EnvironmentMutatingCollection writes environment variables and disables parallelization. QueryShapeWarningInitializer.cs:9 writes one at module initialization, before any test runs.
  • The JWT and Data Protection test keys are read-only Lazy statics.
  • Each factory has its own database, so its DurableJobWorker leader lock and global sweeps see only that class's rows.
  • Tests inside one class still run one at a time.

#863 §D2 lists the seven classes that depend on shared-database rows. None of these eight is among them, and a grep agrees: the only SeedDefaults reference is the read described above.

Frozen counter clock for the two per-URL budget tests

FailingUrl_IsFetchedOnlyWithinItsBudget and RemovedDocument_IsNotRevived_ByDrainingTheBudget counted fetches on the host's real counter. That counter's 5-minute buckets align to the wall clock. If a loop crosses a bucket boundary, an eleventh fetch passes and both assertions fail. Parallel load makes the loops slower, so this PR fixes it. MetadataHostAsync takes an optional counterClock. Those two tests pass a FakeTimeProvider, and the host's IFixedWindowCounter becomes an InProcessFixedWindowCounter on that clock. #1258 used the same mechanism for the global-budget tests. Document expiry still reads the real clock. No production window changed, and no retry was added.

Mutation proof. The mutations were temporary and none is committed.

Mutation Before this fix With this fix
At attempt 5, each loop waits until a real 5-minute boundary passes (crossed at 19:20, 19:25, 19:30 and 19:35 UTC) Both tests fail: expected 10 fetches, got 11 Both tests pass
The override counter gets a clock that moves forward 5 minutes on every read (not applicable) Both tests fail with 10 expected and 11 actual, which proves the host counts with the override counter

Local results

All runs were on the shared 12-core host with no other test stacks running.

  • dotnet build: 0 warnings, 0 errors.
  • Each moved class alone: 81, 20, 29, 22, 14, 11, 8 and 24 tests, all passing.
  • --filter "FullyQualifiedName~OAuthFailClosedTests|FullyQualifiedName~Mcp.|FullyQualifiedName~OAuthServerTests|FullyQualifiedName~AccountLifecycleCommandTests", 3 runs: 334/334 each time, in 2m46s, 2m50s and 3m42s. In the trx, all eight classes start together at 11 s.
  • The same filter plus OAuthClientRegistrationTests, after the fix: 378/378, twice.
  • Peak Postgres connections, sampled every 0.5 s from pg_stat_activity: 58 to 65 against max_connections=208. That is headroom on a filtered run, not a worst-case bound for the full suite.

CI A/B

Method from #863 §U7. Only the Tests (integration) job was rerun, with gh run rerun --job.

  • PR side: run 38299783464 at 92bdee8c, attempts 1 to 11.
  • Baseline: run 38299935049 of draft chore: CI baseline for #1286 (throwaway, do not merge) #1287, attempts 1 to 10. That PR has one empty commit on 391036d0, this PR's base. Main had only one push run since ci: record per-test timing and runner CPU for the integration leg #1272, and rerunning main's integration job would also rerun publish and push :sha-391036d0 again. Both sides therefore ran as pull_request.
  • Leg is the job's duration. "Serial alone" is the time the shared collection ran with no other test running, read from each run's trx.
  • The PR side has 10 passing samples. Attempt 9 failed on McpToolErrorTests (see below), so attempt 11 replaced it.

All runners had 4 vCPUs. Seven CPU models appeared, in two speed tiers. Times are in seconds, median (range).

Tier (CPU models seen) Side n Leg Serial alone
Slow (EPYC 7763, Xeon 8370C, Xeon 8573C) Baseline 8 1010 (965–1025) 487 (435–529)
Slow This PR 7 731 (634–766) 0.1 (0–5)
Fast (EPYC 9V45, EPYC 9V74, Xeon 6973P-C) Baseline 2 752 (664–840) 345 (283–407)
Fast This PR 3 524 (514–586) 0 (0–1)
All Baseline 10 1001 (664–1025) 479 (283–529)
All This PR 10 730 (514–766) 0 (0–5)

On the one CPU both sides drew often, AMD EPYC 7763, the leg went from 1013 s (993–1025, n=7) to 731 s (730–766, n=5), 282 s or 28% less, and the two ranges do not overlap. Fast runners appeared too rarely on either side for a median to mean much. Per CPU: EPYC 9V45 went from 664 s to 514 s, and EPYC 9V74 from 840 s to 586 s. The one main push run since #1272 (38292467802, EPYC 9V45) took 676 s, with the serial collection alone for 320 s.

The shared collection no longer runs alone at the end: its solo time is 0–5 s on every PR sample. The leg now ends with the rest of the parallel work. Fast runners take 8.6–9.8 min, which is not yet the 6–7 min that #1271 aims for. Step 2 (reusing one host per class) cuts total CPU work and is the next lever.

Failure seen in the A/B (already existing, not fixed here)

Attempt 9 failed McpToolErrorTests.BadArgument_IsNamedWithItsShape_AndLogsNoError (job 115013721759). The tool refusal was correct (IsError = True, logged at Information). Then the /mcp POST of that same call ended in a TaskCanceledException in StreamableHttpPostTransport.HandlePostAsync, and Serilog's request log wrote it as a 500 at Error. This is the #1246 failure. #1249 drops late events from earlier requests by trace id. This event has the call's own trace id, so the filter keeps it. The likely cause is the MCP client closing the response stream after it reads the result, before the server finishes the POST. Extra CPU load from running in parallel makes that window wider. One failure in 11 PR samples, none in 10 baseline samples.

…h the shared collection

Move OAuthFailClosedTests, the five MCP tool/authorization classes,
OAuthServerTests and AccountLifecycleCommandTests off the serial
IntegrationCollection onto their own IClassFixture factory, each with its
own template-cloned database (#1002). No test body changes.

summarize.py now matches partial classes, so it no longer drops
OAuthFailClosedTests from the shared-collection census.

Part of #1271
…a drain tests

FailingUrl_IsFetchedOnlyWithinItsBudget and
RemovedDocument_IsNotRevived_ByDrainingTheBudget counted against the host's
wall-clock counter, so a loop that crossed a 5-minute bucket boundary let
an eleventh fetch through. Their metadata hosts now take a frozen counter
clock, as BudgetResolver and FrozenHost already do. Document expiry still
reads the real clock.

Part of #1271
@mforce
mforce merged commit 1204e8f into main Oct 13, 2026
18 checks passed
@mforce
mforce deleted the test/1271-parallel-oauth-mcp branch October 13, 2026 01:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant