Tests: the ManagedConf crash hook and the Lite time settings are no longer shared between parallel test classes (#4773, #4776)

[erikdarlingdata]

Fixes #4773
Fixes #4776

Two test-only fixes for tests that fail because another test class changes process-wide state while they run. Commits are grouped by issue.

#4773: the two ManagedConf migration classes shared a crash hook

Why

ManagedConfMigrationSteps.FailBetweenSteps is one static field for the whole test process. ManagedConfMigrationStepsTests and ManagedConfMigrationRunnerTests both set it to throw a simulated crash, and neither had a [Collection], so xUnit could run them at the same time. When they overlapped, a test in one class threw the other class's crash: in a full run ManagedConfMigrationRunnerTests.RunStepA_Mismatch_RestoresAndListsTheKey failed with "simulated crash between steps", a message that only exists in the steps class.

What changes

• New ManagedConfCrashHookCollection: [CollectionDefinition("managed-conf-crash-hook", DisableParallelization = true)], the same shape as PgFileSettingsStaticsCollection. Both classes now carry [Collection("managed-conf-crash-hook")]. DisableParallelization also keeps every other class off the hook while these run, including a class that reaches WriteTwoSteps only through DarlingManagedPostgres.
• New ManagedConfCrashHookCollectionTests (two tests, plus the theory in the next bullet): both classes carry that collection and the collection disables parallelization; and every test file in Darling.Tests that assigns a non-null value to FailBetweenSteps carries the collection, so a third class added later cannot arm the hook outside it.
• The pattern that second test scans with now counts only an assignment that arms the hook. It matched = null and == null too, although its doc says an assignment of anything but null: the whitespace after = backtracked to nothing, so the lookahead saw a space instead of null, and == was not excluded. A test file that only clears or compares the hook would have been counted as arming it and told to join the collection. The pattern is now FailBetweenSteps\s*=(?!=)\s*(?!\s|null\b), and a new theory, ArmsTheHook_MatchesAnAssignmentThatArms_NotOneThatClearsOrComparesTheHook (8 cases), runs it on strings: = null;, =null;, = null;, a line break before null and == null do not match; = () => throw new IOException(), = _ => throw new IOException() and a line break before the lambda do.
• No assertion in either ManagedConf class changed; only the attribute and a doc paragraph were added.

Reproduction

I ran the two classes together 10 times in one process before the fix (45 tests each). All 10 passed: the whole pair finishes in about 0.4 s, so the two rarely overlap. The failure is not reproduced here. Instead, the shared state was put back and the new guard shown to catch it (below).

RED plants

Each plant was applied on top of the fix, the guard class was run, and the plant was removed.

Plant	Failing output line
[Collection("managed-conf-crash-hook")] removed from ManagedConfMigrationRunnerTests (the shared state back)	2 of 2 fail. EveryTestFileThatArmsTheHook_CarriesTheCollection: These test files arm ManagedConfMigrationSteps.FailBetweenSteps without [Collection("managed-conf-crash-hook")], so they can crash another class's WriteTwoSteps call: ManagedConfMigrationRunnerTests.cs. TheClassesThatArmTheHook_ShareOneCollection_ThatRunsAlone: ManagedConfMigrationRunnerTests sets ManagedConfMigrationSteps.FailBetweenSteps, a static shared by the whole test process, so it must carry [Collection("managed-conf-crash-hook")]; found no [Collection] attribute.
DisableParallelization = true removed from the collection definition	1 of 2 fails. TheClassesThatArmTheHook_ShareOneCollection_ThatRunsAlone: The "managed-conf-crash-hook" collection must disable parallelization: without it the two classes are serialized against each other only, and any other class that reaches WriteTwoSteps can still meet a crash that was armed for someone else.

#4776: QueryWindowTruncationTests depended on another class's time settings

Why

SetWindowTruncatedBanner_Truncated_ShowsSinceEffectiveStart formats the same time twice with ServerTimeHelper.FormatServerTime: once inside ServerTab.SetWindowTruncatedBanner and once in its assertion. FormatServerTime reads ServerTimeHelper.UtcOffsetMinutes and ServerTimeHelper.CurrentDisplayMode, settings shared by the whole test process. The classes that change them are in [Collection("server-time-helper")], but QueryWindowTruncationTests was not, so a writer could run between the two calls and the two texts differed.

What changes

• QueryWindowTruncationTests now carries [Collection("server-time-helper")]. Every assertion stays.
• New ServerTimeHelperCollectionTests (two tests): QueryWindowTruncationTests carries the collection; and every Lite.Tests source file that uses ServerTimeHelper.UtcOffsetMinutes, CurrentDisplayMode, ToServerTime, ConvertForDisplay, DisplayTimeToServerTime, GetTimezoneLabel, FormatServerTime or FormatServerClock in code (comments are ignored) carries it. One file, ComparisonWindowUtcTests.cs, is allowed because it only names UtcOffsetMinutes inside a string it searches product source for; the allowance states why. The scan found QueryWindowTruncationTests.cs and no other file outside the collection.
• The guard's member list now also covers ConvertForDisplay, DisplayTimeToServerTime and GetTimezoneLabel. Each reads _utcOffsetMinutes (ConvertForDisplay in the local-time and UTC modes, GetTimezoneLabel in the server-time mode, the two-argument DisplayTimeToServerTime always) but was missing from the list, so a test class that called only one of them was never asked to join the collection. The pattern cannot tell the two DisplayTimeToServerTime overloads apart, and the three-argument one reads no setting, so a file that calls only that one belongs in the allowance; the doc comment on the pattern says so. No test file needed the attribute or an allowance: the only file that calls an added member, AlertBadgeServerOffsetTests, already carries the collection.

Reproduction

Before the fix, QueryWindowTruncationTests was run 10 times together with four classes that write the settings (ServerClockRendererArithmeticTests, McpServerLocalOffsetTests, AlertBadgeServerOffsetTests, TimeHonestyRungTests). One of the 10 runs (run 3) failed:

QueryWindowTruncationTests.SetWindowTruncatedBanner_Truncated_ShowsSinceEffectiveStart [FAIL]
  Assert.Equal() Failure: Strings differ
  Expected: "Showing since 2026-01-15 03:30:00"
  Actual:   "Showing since 2026-01-15 04:30:00"

Before the fix the new guard also failed on the real code (2 of 2):

• QueryWindowTruncationTests_IsInTheServerTimeHelperCollection: QueryWindowTruncationTests formats one time twice with ServerTimeHelper.FormatServerTime, which reads settings shared by the whole test process, so it must carry [Collection("server-time-helper")]; found no [Collection] attribute.
• EveryTestFileThatReadsTheSettings_CarriesTheCollection: These test files use ServerTimeHelper's shared settings without [Collection("server-time-helper")], so a class that changes them can run between two of their reads: QueryWindowTruncationTests.cs

After the fix the same 10-run loop, plus the two new guard tests (42 tests per run), passed 10 of 10.

Test plan

• Darling.Tests and Lite.Tests build with 0 warnings and 0 errors after merging origin/dev.
• Two ManagedConf migration test classes share a static crash hook and fail each other when run in parallel #4773 guard: both tests pass with the fix and fail under each plant above.
• Darling: ManagedConfMigrationStepsTests, ManagedConfMigrationRunnerTests, ManagedConfCrashHookCollectionTests and DocCommentHygiene together with the other ManagedConf* classes: 265 total, 0 failed, 9 skipped (live tests that need DARLING_TEST_PG or a runtime).
• QueryWindowTruncationTests can fail when another test class changes the server time settings #4776 before the fix: 1 of 10 loop runs failed. After the fix: 10 of 10 passed.
• Lite: QueryWindowTruncationTests, ServerTimeHelperCollectionTests and the four writer classes after merging origin/dev: 42 total, 0 failed.
• Full Lite.Tests run from this branch: 5670 total, 0 failed, 0 skipped.
• Two ManagedConf migration test classes share a static crash hook and fail each other when run in parallel #4773 pattern theory ArmsTheHook_MatchesAnAssignmentThatArms_NotOneThatClearsOrComparesTheHook. RED on the old pattern: 4 of 8 cases fail with Assert.Equal() Failure: Values differ, Expected: False, Actual: True ( = null;, = null;, a line break before null, and == null; =null; passed even then). After the change: 8 of 8 pass, and ManagedConfCrashHookCollectionTests passes 10 of 10.
• QueryWindowTruncationTests can fail when another test class changes the server time settings #4776 member list, RED with a temporary test class that calls ServerTimeHelper.GetTimezoneLabel and carries no collection: with the old member list ServerTimeHelperCollectionTests passed 2 of 2 (the guard could not see the class); with the new list EveryTestFileThatReadsTheSettings_CarriesTheCollection failed (1 of 2) and named only the temporary file: These test files use ServerTimeHelper's shared settings without [Collection("server-time-helper")], so a class that changes them can run between two of their reads: PlantedTimezoneLabelTemp.cs. The temporary file was deleted and is in no commit. Without it the guard passes 2 of 2.
• After merging origin/dev again: Darling.Tests and Lite.Tests build with 0 warnings and 0 errors. Darling ManagedConfCrashHookCollectionTests, ManagedConfMigrationStepsTests, ManagedConfMigrationRunnerTests and DocCommentHygiene: 132 total, 0 failed. Lite ServerTimeHelperCollectionTests: 2 total, 0 failed.
• Full Darling.Tests run from this branch: not made. Full Darling.Tests runs were made from the branch of Tests: the redactor scaling check holds on a busy machine (#4788) #4819 (17425 total, 0 failed) and from the branch of Store-host live test: compare chunk counts read while no chunk changed (#4807) #4815 (17434 total, 0 failed); neither includes this change. The Darling classes this change touches were run at class level, above.
• Full Lite.Tests run after the guard's member list was widened: not made. No test class or attribute changed and the widened guard flags no test file; the full run of 5670 above was made before this change.

CHANGELOG

SECTION: None