diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index b3d9740d48..d48f807f31 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -2638,7 +2638,8 @@ jobs: - 'Darling/Darling.Tests/McpArgumentTypeRows.cs' - 'Darling/Darling.Tests/StatementScrubCanary.cs' - 'Darling/Darling.Tests/GuardStageScanner.cs' - - 'Darling/Darling.Tests/WpfStaGate.cs' + - 'Darling/Darling.Tests/StaTestThread.cs' + - 'Darling/Darling.Tests/TimingTestScanner.cs' - 'Darling/Darling.Tests/WpfExitSwitchTests.cs' # How much of the Lite suite THIS leg runs, decided from the filter answers alone (no build needed): # full - a Lite, Lite.Tests, core or root path changed, or a Darling.Tests file that Lite.Tests diff --git a/Darling/Darling.Tests/AvailabilityGroupsTabRefreshTests.cs b/Darling/Darling.Tests/AvailabilityGroupsTabRefreshTests.cs index 902fc182e2..af44a5fbd7 100644 --- a/Darling/Darling.Tests/AvailabilityGroupsTabRefreshTests.cs +++ b/Darling/Darling.Tests/AvailabilityGroupsTabRefreshTests.cs @@ -35,7 +35,7 @@ public void Render_SameRowsAcrossTwoCalls_KeepsTheSameCardInstances() /* The pin the ruling asks for: two refreshes over identical rows must not hand the ItemsControl a new object per card. Before the fix, Render assigns a brand new List (brand new instances) every call, so this fails on the pre-#4238 code — proven once, below. */ - OnStaThread(() => + StaTestThread.Run(() => { var tab = new AvailabilityGroupsTab(); @@ -56,7 +56,7 @@ public void Render_SameRowsAcrossTwoCalls_KeepsTheSameCardInstances() [Fact] public void Render_ChangedValue_UpdatesInPlace_SameInstanceNewValue() { - OnStaThread(() => + StaTestThread.Run(() => { var tab = new AvailabilityGroupsTab(); @@ -80,7 +80,7 @@ public void Render_ChangedValue_UpdatesInPlace_SameInstanceNewValue() [Fact] public void Render_AddedOrRemovedAg_AddsOrRemovesExactlyOneCard() { - OnStaThread(() => + StaTestThread.Run(() => { var tab = new AvailabilityGroupsTab(); @@ -129,7 +129,7 @@ public void Render_UnchangedRows_ReplacesNoCardReplicaOrDatabaseInstances() reconcile every card, replica and database IN PLACE and never rebuild any of them. A regression that tears down and rebuilds rows -- or an O(n^2) loop that amounts to the same thing -- shows up here as a nonzero replaced count, independent of how loaded the runner is. */ - OnStaThread(() => + StaTestThread.Run(() => { var tab = new AvailabilityGroupsTab(); var topology = BuildTopology(agCount: 42, replicasPerAg: 2, dbRowsPerAg: 9, extraDbRowsOnFirst: 20); @@ -253,23 +253,4 @@ private static AgTopologyDatabaseRow Database(int serverId, string serverName, s RedoRateKbPerSec = 0, }; - /// WPF objects require STA; same shape as RawWindowFloorViewerPortTests / Lite.Tests' MainWindowAccessKeyTests. - private static void OnStaThread(Action body) - { - Exception? error = null; - using var staGate = WpfStaGate.Enter(); - var thread = new Thread(() => - { - try { body(); } - catch (Exception ex) { error = ex; } - }); - thread.SetApartmentState(ApartmentState.STA); - thread.Start(); - thread.Join(); - - if (error is not null) - { - throw error; - } - } } diff --git a/Darling/Darling.Tests/BoundedCompressionDdlTests.cs b/Darling/Darling.Tests/BoundedCompressionDdlTests.cs index 764e33b719..62d904412f 100644 --- a/Darling/Darling.Tests/BoundedCompressionDdlTests.cs +++ b/Darling/Darling.Tests/BoundedCompressionDdlTests.cs @@ -23,6 +23,7 @@ namespace Darling.Tests; /// failed settings read. /// /* #1776 own-store: each live fact mints its own scratch database through ScratchPostgres. */ +[Collection("timing")] public sealed class BoundedCompressionDdlTests { private static string Storage() => diff --git a/Darling/Darling.Tests/ColumnFilterPersistenceTests.cs b/Darling/Darling.Tests/ColumnFilterPersistenceTests.cs index 8dc69c9700..443612500d 100644 --- a/Darling/Darling.Tests/ColumnFilterPersistenceTests.cs +++ b/Darling/Darling.Tests/ColumnFilterPersistenceTests.cs @@ -509,24 +509,6 @@ private sealed class LoginRow public string? Detail { get; set; } } - private static T OnStaThread(Func body) - { - T result = default!; - Exception? error = null; - using var staGate = WpfStaGate.Enter(); - var thread = new Thread(() => - { - try { result = body(); } - catch (Exception ex) { error = ex; } - }); - thread.SetApartmentState(ApartmentState.STA); - thread.Start(); - thread.Join(); - if (error is not null) - throw error; - return result; - } - private static DataGrid NewGrid(string? name, string? server) { var grid = new DataGrid { AutoGenerateColumns = false }; @@ -555,7 +537,7 @@ private static void Untick(IDataGridFilterManager manager, string column, string [Fact] public void A_text_match_on_a_query_text_column_lasts_the_session_but_is_not_stored() { - var kept = OnStaThread(() => + var kept = StaTestThread.Run(() => { var store = NewStore(); ColumnFilterStore.Current = store; @@ -591,7 +573,7 @@ public void A_text_match_on_a_query_text_column_lasts_the_session_but_is_not_sto [Fact] public void A_filter_survives_a_refresh_and_a_value_that_first_appears_later_still_shows() { - OnStaThread(() => + StaTestThread.Run(() => { var grid = NewGrid("G", null); var manager = new DataGridFilterManager(grid); @@ -610,7 +592,7 @@ public void A_filter_survives_a_refresh_and_a_value_that_first_appears_later_sti [Fact] public void The_value_list_comes_from_the_unfiltered_rows_not_the_ones_shown() { - OnStaThread(() => + StaTestThread.Run(() => { var grid = NewGrid("G", null); var manager = new DataGridFilterManager(grid); @@ -626,7 +608,7 @@ public void The_value_list_comes_from_the_unfiltered_rows_not_the_ones_shown() [Fact] public void Number_query_text_and_prose_columns_and_columns_the_row_lacks_get_no_list() { - OnStaThread(() => + StaTestThread.Run(() => { var grid = NewGrid("G", null); var manager = new DataGridFilterManager(grid); @@ -645,7 +627,7 @@ public void Number_query_text_and_prose_columns_and_columns_the_row_lacks_get_no [Fact] public void A_text_column_that_sorts_by_another_member_gets_no_list() { - OnStaThread(() => + StaTestThread.Run(() => { var grid = NewGrid("G", null); ((DataGridTextColumn)grid.Columns[0]).SortMemberPath = "SessionId"; @@ -660,7 +642,7 @@ public void A_text_column_that_sorts_by_another_member_gets_no_list() [Fact] public void A_column_with_a_value_over_256_characters_gets_no_list() { - OnStaThread(() => + StaTestThread.Run(() => { var grid = NewGrid("G", null); var manager = new DataGridFilterManager(grid); @@ -674,7 +656,7 @@ public void A_column_with_a_value_over_256_characters_gets_no_list() [Fact] public void A_filter_set_in_one_session_is_loaded_by_the_next_on_its_first_refresh() { - var shown = OnStaThread(() => + var shown = StaTestThread.Run(() => { var store = NewStore(); ColumnFilterStore.Current = store; @@ -700,7 +682,7 @@ public void A_filter_set_in_one_session_is_loaded_by_the_next_on_its_first_refre [Fact] public void A_cross_server_list_under_the_all_servers_scope_keeps_its_filters_across_a_restart() { - var shown = OnStaThread(() => + var shown = StaTestThread.Run(() => { var store = NewStore(); ColumnFilterStore.Current = store; @@ -723,7 +705,7 @@ public void A_cross_server_list_under_the_all_servers_scope_keeps_its_filters_ac [Fact] public void Another_servers_filters_do_not_apply_and_a_grid_without_a_name_or_scope_is_not_stored() { - OnStaThread(() => + StaTestThread.Run(() => { var store = NewStore(); ColumnFilterStore.Current = store; @@ -757,7 +739,7 @@ public void Another_servers_filters_do_not_apply_and_a_grid_without_a_name_or_sc [Fact] public void A_stored_filter_on_a_column_the_grid_no_longer_has_is_ignored() { - OnStaThread(() => + StaTestThread.Run(() => { var store = NewStore(); store.Save("srv-a", "G", new[] { Hide("RetiredColumn", "x"), Hide("LoginName", "job_svc") }); @@ -776,7 +758,7 @@ public void A_stored_filter_on_a_column_the_grid_no_longer_has_is_ignored() [Fact] public void Clear_all_filters_clears_the_grid_and_the_stored_copy_but_a_server_switch_does_not() { - OnStaThread(() => + StaTestThread.Run(() => { var store = NewStore(); ColumnFilterStore.Current = store; @@ -806,7 +788,7 @@ public void Clear_all_filters_clears_the_grid_and_the_stored_copy_but_a_server_s [Fact] public void A_server_switch_loads_the_new_servers_own_filters() { - OnStaThread(() => + StaTestThread.Run(() => { var store = NewStore(); store.Save("srv-b", "G", new[] { Hide("LoginName", "sa") }); @@ -828,7 +810,7 @@ public void A_server_switch_loads_the_new_servers_own_filters() [Fact] public void A_store_that_fails_never_blocks_the_grid() { - OnStaThread(() => + StaTestThread.Run(() => { File.WriteAllText(FilePath, "garbage"); ColumnFilterStore.Current = NewStore(); diff --git a/Darling/Darling.Tests/ColumnFilterPopupTests.cs b/Darling/Darling.Tests/ColumnFilterPopupTests.cs index 516e2fc500..0c0f2385a4 100644 --- a/Darling/Darling.Tests/ColumnFilterPopupTests.cs +++ b/Darling/Darling.Tests/ColumnFilterPopupTests.cs @@ -26,24 +26,6 @@ namespace Darling.Tests; /// public sealed class ColumnFilterPopupTests { - private static T OnStaThread(Func body) - { - T result = default!; - Exception? error = null; - using var staGate = WpfStaGate.Enter(); - var thread = new Thread(() => - { - try { result = body(); } - catch (Exception ex) { error = ex; } - }); - thread.SetApartmentState(ApartmentState.STA); - thread.Start(); - thread.Join(); - if (error is not null) - throw error; - return result; - } - private static T Find(DependencyObject root, Func where) where T : DependencyObject { foreach (var child in LogicalTreeHelper.GetChildren(root).OfType()) @@ -65,7 +47,7 @@ private static Button ButtonNamed(ColumnFilterPopup popup, string content) => [Fact] public void The_list_section_shows_only_for_a_column_with_a_catalog() { - OnStaThread(() => + StaTestThread.Run(() => { var popup = new ColumnFilterPopup(); var panel = Find(popup, p => p.Name == "ValueListPanel"); @@ -84,7 +66,7 @@ public void The_list_section_shows_only_for_a_column_with_a_catalog() [Fact] public void The_search_box_narrows_the_list_and_Select_All_covers_what_it_shows() { - OnStaThread(() => + StaTestThread.Run(() => { var popup = new ColumnFilterPopup(); popup.Initialize("LoginName", null, Catalog("sa", "app", "job_svc", null)); @@ -113,7 +95,7 @@ public void The_search_box_narrows_the_list_and_Select_All_covers_what_it_shows( [Fact] public void Nothing_ticked_shows_the_note_and_the_cap_note_shows_for_a_long_list() { - OnStaThread(() => + StaTestThread.Run(() => { var popup = new ColumnFilterPopup(); popup.Initialize("LoginName", null, Catalog("a", "b")); @@ -134,7 +116,7 @@ public void Nothing_ticked_shows_the_note_and_the_cap_note_shows_for_a_long_list [Fact] public void An_existing_value_filter_comes_back_as_the_ticks_and_the_text_match_beside_it() { - OnStaThread(() => + StaTestThread.Run(() => { var existing = new ColumnFilterState { ColumnName = "LoginName", ValueMode = ColumnValueMode.Hide, Operator = FilterOperator.StartsWith, Value = "s" }; existing.Values.Add("job_svc"); @@ -158,7 +140,7 @@ public void An_existing_value_filter_comes_back_as_the_ticks_and_the_text_match_ [Fact] public void A_column_without_a_list_keeps_a_stored_value_filter_when_the_text_match_is_applied() { - OnStaThread(() => + StaTestThread.Run(() => { var existing = new ColumnFilterState { ColumnName = "LoginName", ValueMode = ColumnValueMode.ShowOnly }; existing.Values.Add("sa"); @@ -178,7 +160,7 @@ public void A_column_without_a_list_keeps_a_stored_value_filter_when_the_text_ma [Fact] public void A_text_only_apply_keeps_the_stored_value_part_even_when_todays_rows_have_drifted_from_it() { - OnStaThread(() => + StaTestThread.Run(() => { /* Scenario A: stored ShowOnly {sa}, but the rows now hold only sa. Re-deriving the mode would find nothing unticked, give None, and drop the filter. */ @@ -214,7 +196,7 @@ public void A_text_only_apply_keeps_the_stored_value_part_even_when_todays_rows_ [Fact] public void Select_All_unticked_shows_the_no_values_note_at_once() { - OnStaThread(() => + StaTestThread.Run(() => { var popup = new ColumnFilterPopup(); popup.Initialize("LoginName", null, Catalog("a", "b")); @@ -234,7 +216,7 @@ public void Select_All_unticked_shows_the_no_values_note_at_once() [Fact] public void A_stored_value_part_on_a_column_with_no_list_shows_read_only_with_a_way_to_clear_it() { - OnStaThread(() => + StaTestThread.Run(() => { var existing = new ColumnFilterState { ColumnName = "LoginName", ValueMode = ColumnValueMode.Hide }; existing.Values.Add("a"); @@ -276,7 +258,7 @@ public void A_stored_value_part_on_a_column_with_no_list_shows_read_only_with_a_ [Fact] public void Clear_all_filters_shows_only_when_the_grid_has_a_filter_and_raises_its_event() { - OnStaThread(() => + StaTestThread.Run(() => { var popup = new ColumnFilterPopup(); var button = ButtonNamed(popup, "Clear all filters"); @@ -297,7 +279,7 @@ public void Clear_all_filters_shows_only_when_the_grid_has_a_filter_and_raises_i [Fact] public void Clear_applies_an_empty_filter_including_the_value_part() { - OnStaThread(() => + StaTestThread.Run(() => { var existing = new ColumnFilterState { ColumnName = "LoginName", ValueMode = ColumnValueMode.Hide }; existing.Values.Add("a"); @@ -316,7 +298,7 @@ public void Clear_applies_an_empty_filter_including_the_value_part() [Fact] public void Esc_closes_the_popup_from_anywhere_in_it_and_the_list_is_virtualised_and_tab_stops_once() { - OnStaThread(() => + StaTestThread.Run(() => { var popup = new ColumnFilterPopup(); popup.Initialize("LoginName", null, Catalog("a", "b")); diff --git a/Darling/Darling.Tests/ComposeStatementTimeoutV147MigrationLiveTests.cs b/Darling/Darling.Tests/ComposeStatementTimeoutV147MigrationLiveTests.cs index 32be927a44..f27d65198d 100644 --- a/Darling/Darling.Tests/ComposeStatementTimeoutV147MigrationLiveTests.cs +++ b/Darling/Darling.Tests/ComposeStatementTimeoutV147MigrationLiveTests.cs @@ -7,6 +7,7 @@ */ using System; +using System.Collections.Generic; using System.Linq; using System.Threading; using System.Threading.Tasks; @@ -30,8 +31,10 @@ namespace Darling.Tests; /// scope 2) the moment that rung landed. /// /* #1776 own-store: deliberately NOT [Collection("live-postgres")]. Each fact mints its own scratch database - through ScratchPostgres and never touches the shared store's tables, so it cannot race the live collection - and serializing it would be pure slowdown. */ + through ScratchPostgres and never touches the shared store's tables, so it cannot race the live collection. + #5602: the role re-assertion fact writes the cluster's FIXED viewer and mcp roles (CREATE ROLE, ALTER ROLE ... SET + statement_timeout), which every scratch database shares, so the class runs in the collection that runs alone. */ +[Collection("pg-cluster-roles")] public sealed class ComposeStatementTimeoutV147MigrationLiveTests { private const int RungVersion = 147; @@ -199,13 +202,48 @@ public async Task TheRoleReassertion_SetsBothRolesTo60sAnd5000ms_ThroughPgDbRole var ct = TestContext.Current.CancellationToken; await using var scratch = await ScratchPostgres.CreateAsync(baseConnectionString!, ct); + + /* #5602: viewer and mcp are the cluster's FIXED roles, shared with every scratch database. The ones this fact + makes it drops again: left behind, they made the later classes that provision the managed roles skip + ("a cluster-wide admin/viewer/mcp role already exists on this rig"). A role that was already there is not + ours to drop. */ + var madeRoles = new List(); + var bodySucceeded = false; + try + { + await ReassertAndReadBackAsync(scratch, madeRoles, ct); + bodySucceeded = true; + } + finally + { + await LiveStoreCleanup.RunAsync(scratch.ConnectionString, bodySucceeded, async (cleanup, cleanupCt) => + { + foreach (var role in madeRoles) + { + await ExecAsync(cleanup, cleanupCt, $"DROP OWNED BY {role}; DROP ROLE IF EXISTS {role}"); + } + }); + } + } + + private static async Task ReassertAndReadBackAsync(ScratchPostgres scratch, List madeRoles, CancellationToken ct) + { await using (var connection = new NpgsqlConnection(scratch.ConnectionString)) { await connection.OpenAsync(ct); await PgMigrations.MigrateAsync(connection, ct); - await ExecAsync(connection, ct, "DO $$ BEGIN IF NOT EXISTS (SELECT 1 FROM pg_roles WHERE rolname = 'viewer') THEN CREATE ROLE viewer LOGIN; END IF; END $$"); - await ExecAsync(connection, ct, "DO $$ BEGIN IF NOT EXISTS (SELECT 1 FROM pg_roles WHERE rolname = 'mcp') THEN CREATE ROLE mcp LOGIN; END IF; END $$"); + foreach (var role in new[] { "viewer", "mcp" }) + { + using var exists = new NpgsqlCommand("SELECT count(*) FROM pg_roles WHERE rolname = $1", connection); + exists.Parameters.AddWithValue(role); + if (Convert.ToInt64(await exists.ExecuteScalarAsync(ct)) == 0) + { + await ExecAsync(connection, ct, $"CREATE ROLE {role} LOGIN"); + madeRoles.Add(role); + } + } + /* Old shipped value, as if provisioning last ran before this build existed — the state the re-assertion must move forward. */ await ExecAsync(connection, ct, "ALTER ROLE viewer SET statement_timeout = '15s'"); diff --git a/Darling/Darling.Tests/DarlingCloudIdentityProbeTests.cs b/Darling/Darling.Tests/DarlingCloudIdentityProbeTests.cs index 213feaa418..bd4b40f8d5 100644 --- a/Darling/Darling.Tests/DarlingCloudIdentityProbeTests.cs +++ b/Darling/Darling.Tests/DarlingCloudIdentityProbeTests.cs @@ -27,6 +27,7 @@ namespace Darling.Tests; /// an EC2 success, an Azure success, neither found (a timeout within the probe's own budget), a 401, a 404, an /// oversized body, a redirect (never followed), and a value that fails the accept pattern. /// +[Collection("timing")] public sealed class DarlingCloudIdentityProbeTests { private static HttpResponseMessage Text(HttpStatusCode status, string body) => @@ -147,11 +148,29 @@ wait on the handler until CI's job timeout. */ throw new InvalidOperationException("unreachable"); }); + /* #5600: this failed again in the whole-suite guards job (about 28,000 tests in one process) at 1083 ms + after the request ended, with the probe unchanged. returnedAt was read HERE after + `await ...WaitAsync(...)`, so it included the hop from the probe task's completion back into this + test (WaitAsync's own completion, then the awaiter's continuation), and that hop belongs to the + machine, not the probe. The probe's completion is stamped instead by a synchronous continuation on + the probe task itself, which runs on whichever thread completes the probe, with no hop. The test now + also runs in the `timing` collection (TimingCollection), alone in the process, so the remaining + cancel-to-return step is not competing with other classes either. No stall allowance: the bar + stays NoPoolStepLimitMs. */ + var returnedAtMs = new long[] { -1 }; + clock.Start(); CloudIdentity identity; try { - identity = await DarlingCloudIdentityProbe.ProbeAsync(handler, CancellationToken.None).WaitAsync(HangGuard); + var probe = DarlingCloudIdentityProbe.ProbeAsync(handler, CancellationToken.None); + var stamp = probe.ContinueWith( + _ => Interlocked.CompareExchange(ref returnedAtMs[0], clock.ElapsedMilliseconds, -1), + CancellationToken.None, + TaskContinuationOptions.ExecuteSynchronously, + TaskScheduler.Default); + identity = await probe.WaitAsync(HangGuard); + await stamp.WaitAsync(HangGuard); } catch (TimeoutException) { @@ -159,7 +178,8 @@ wait on the handler until CI's job timeout. */ throw; } - var returnedAt = clock.ElapsedMilliseconds; + var resumedAt = clock.ElapsedMilliseconds; + var returnedAt = Interlocked.Read(ref returnedAtMs[0]); Assert.Equal(CloudIdentity.None, identity); var startedAt = Interlocked.Read(ref handlerStartedAtMs[0]); @@ -173,7 +193,7 @@ wait on the handler until CI's job timeout. */ $"the handler's request ended after {endedAt} ms, well before the {DarlingCloudIdentityProbe.ProbeBudget.TotalMilliseconds:0} ms budget: something other than the probe's budget ended it"); Assert.True( returnedAt - endedAt <= NoPoolStepLimitMs, - $"ProbeAsync returned {returnedAt - endedAt} ms after the handler's request ended (at {endedAt} ms), not within {NoPoolStepLimitMs} ms: the probe waited after the budget cancelled the request, and nothing bounds that wait"); + $"ProbeAsync returned {returnedAt - endedAt} ms after the handler's request ended (at {endedAt} ms; this test resumed at {resumedAt} ms), not within {NoPoolStepLimitMs} ms: the probe waited after the budget cancelled the request, and nothing bounds that wait"); } /// The number the timing test above no longer measures against a clock (#4741): the 200 ms total diff --git a/Darling/Darling.Tests/DarlingSecuritySplitLiveTests.cs b/Darling/Darling.Tests/DarlingSecuritySplitLiveTests.cs index 112e2d0bed..e216fa7554 100644 --- a/Darling/Darling.Tests/DarlingSecuritySplitLiveTests.cs +++ b/Darling/Darling.Tests/DarlingSecuritySplitLiveTests.cs @@ -32,7 +32,7 @@ namespace Darling.Tests; /// the schemas only (no REVOKE on a named database), so the test does not depend on the store's /// database name. Every object it creates is cleaned up. /// -[Collection("live-postgres")] +[Collection("pg-cluster-roles")] public sealed class DarlingSecuritySplitLiveTests { /* #4981: a role belongs to the whole cluster, so a constant name made two runs on one cluster share a role: the diff --git a/Darling/Darling.Tests/DarlingStoreHostProfileLiveTests.cs b/Darling/Darling.Tests/DarlingStoreHostProfileLiveTests.cs index 6b49a33cbb..e10257030f 100644 --- a/Darling/Darling.Tests/DarlingStoreHostProfileLiveTests.cs +++ b/Darling/Darling.Tests/DarlingStoreHostProfileLiveTests.cs @@ -40,7 +40,7 @@ namespace Darling.Tests; /// max_wal_size for goes through ALTER SYSTEM, /// which file attribution alone resolves — no reload needed there either. /// -[Collection("live-postgres")] +[Collection("pg-cluster-roles")] public sealed class DarlingStoreHostProfileLiveTests { [Fact] diff --git a/Darling/Darling.Tests/GetReadLatencyLiveTests.cs b/Darling/Darling.Tests/GetReadLatencyLiveTests.cs index f9233aca7d..ef5aaa5d7d 100644 --- a/Darling/Darling.Tests/GetReadLatencyLiveTests.cs +++ b/Darling/Darling.Tests/GetReadLatencyLiveTests.cs @@ -32,7 +32,10 @@ namespace Darling.Tests; /// pinned too, not just the reader. /// /* #1776 own-store: each fact mints its own scratch database through ScratchPostgres and never touches the - shared store's tables, so it cannot race the live collection. */ + shared store's tables, so it cannot race the live collection. + #5602: the facts that connect as viewer and mcp create and drop those FIXED cluster-wide roles, so the class runs + in the collection that runs alone. */ +[Collection("pg-cluster-roles")] public sealed class GetReadLatencyLiveTests { private static string? ConnectionString => Environment.GetEnvironmentVariable("DARLING_TEST_PG"); diff --git a/Darling/Darling.Tests/LivePostgresCollectionHygieneTests.cs b/Darling/Darling.Tests/LivePostgresCollectionHygieneTests.cs index 2eb9f118d8..f71f68db73 100644 --- a/Darling/Darling.Tests/LivePostgresCollectionHygieneTests.cs +++ b/Darling/Darling.Tests/LivePostgresCollectionHygieneTests.cs @@ -72,6 +72,12 @@ public sealed class LivePostgresCollectionHygieneTests private const string LiveCollectionAttribute = "[Collection(\"" + LiveCollectionName + "\")]"; + /// + /// #5602: the collection that runs ALONE (DisableParallelization), for the classes that write the cluster's + /// fixed roles or settings. It is a stronger decision than the live collection's turn-taking, so it counts as one. + /// + private const string ClusterRolesCollectionAttribute = "[Collection(\"pg-cluster-roles\")]"; + /// The recorded-exemption marker. Prose, deliberately: the point is that a human wrote down why. private const string OwnStoreMarker = "#1776 own-store"; @@ -293,7 +299,8 @@ private static bool CarriesLiveCollectionAttribute(string header) { foreach (var line in header.Split('\n')) { - if (line.TrimStart().StartsWith(LiveCollectionAttribute, StringComparison.Ordinal)) + if (line.TrimStart().StartsWith(LiveCollectionAttribute, StringComparison.Ordinal) + || line.TrimStart().StartsWith(ClusterRolesCollectionAttribute, StringComparison.Ordinal)) { return true; } diff --git a/Darling/Darling.Tests/MigrationLockUncontendedAcquireTimingTests.cs b/Darling/Darling.Tests/MigrationLockUncontendedAcquireTimingTests.cs new file mode 100644 index 0000000000..ee8833d737 --- /dev/null +++ b/Darling/Darling.Tests/MigrationLockUncontendedAcquireTimingTests.cs @@ -0,0 +1,89 @@ +/* + * Copyright (c) 2026 Erik Darling, Darling Data LLC + * + * This file is part of the SQL Server Performance Monitor. + * + * Licensed under the MIT License. See LICENSE file in the project root for full license information. + */ + +using System; +using System.Diagnostics; +using System.Threading; +using System.Threading.Tasks; +using Npgsql; +using PerformanceMonitor.Darling.Storage; +using Xunit; + +namespace Darling.Tests; + +/* #1776 own-store: the fact mints its own scratch database through ScratchPostgres. */ + +/// +/// #5602: the one check in that judges a clock, in the timing +/// collection so it runs alone, after the parallel classes. +/// +/// Why it left the contention class. That class sits in live-postgres (its other facts hold the real +/// migration lock on the shared store), and a live-postgres class runs in parallel with every non-live class, so a +/// 750 ms bar against a one second poll sleep was judged against the runner's load. This fact needs no shared store: the +/// advisory lock is per database, so a scratch database of its own has nobody to contend with, and the class can join +/// timing (TimingCollection does not carry the shared-store fixture). +/// +/// What it keeps. The same two-sided bar. The shipped poll interval is one second, so a ceiling below a +/// second separates "took the lock on the first attempt" from "slept once first" — which matters because moving the sleep +/// above the first attempt is a RELOCATION, and every occurrence count stays 1, so no structural check sees it and only +/// elapsed time can. The operation is a single round trip on a connection that is already open and already migrated, which +/// is sub-millisecond against a local store, so 750 ms leaves roughly three orders of magnitude of headroom. +/// +[Collection("timing")] +public sealed class MigrationLockUncontendedAcquireTimingTests +{ + /// Same budget the contention class uses: long enough to be several poll intervals, so a slept-first build would have time to loop. + private const int TestWaitBudgetSeconds = 3; + + private static readonly TimeSpan UncontendedAcquireCeiling = TimeSpan.FromMilliseconds(750); + + [Fact] + public async Task AcquireSucceedsFirstAttempt_WhenNobodyHoldsTheLock_AgainstDevPostgres() + { + var baseConnectionString = Environment.GetEnvironmentVariable("DARLING_TEST_PG"); + Assert.SkipWhen(string.IsNullOrEmpty(baseConnectionString), + "Set DARLING_TEST_PG to a Postgres connection string to run the live migration-lock tests."); + + var ct = TestContext.Current.CancellationToken; + await using var scratch = await ScratchPostgres.CreateAsync(baseConnectionString!, ct); + await using var connection = new NpgsqlConnection(scratch.ConnectionString); + await connection.OpenAsync(ct); + await PgMigrations.MigrateAsync(connection, ct); + + /* The uncontended path must cost nothing: polling is only a fallback shape, and if the first + attempt did not succeed outright then every ordinary service start pays a poll interval. */ + var started = Stopwatch.StartNew(); + Assert.True(await PgMigrations.TryAcquireMigrationLockForTestsAsync( + connection, logger: null, TestWaitBudgetSeconds, ct)); + started.Stop(); + + /* Release in a finally, not after the assertion: Npgsql pools, so disposing the connection hands the physical + session, advisory lock and all, back to the pool rather than closing it. */ + var bodySucceeded = false; + try + { + Assert.True( + started.Elapsed < UncontendedAcquireCeiling, + $"An uncontended acquire took {started.Elapsed.TotalMilliseconds:F0}ms, over the " + + $"{UncontendedAcquireCeiling.TotalMilliseconds:F0}ms ceiling — it slept a poll interval " + + "instead of taking the lock on its first attempt."); + bodySucceeded = true; + } + finally + { + await LiveStoreCleanup.RunOwnedAsync( + bodySucceeded, + async () => + { + using var command = new NpgsqlCommand("SELECT pg_advisory_unlock($1)", connection) { CommandTimeout = 30 }; + command.Parameters.AddWithValue(PgMigrations.MigrationLockKeyForTests); + await command.ExecuteNonQueryAsync(CancellationToken.None); + }); + } + } +} diff --git a/Darling/Darling.Tests/MigrationLockWaitContentionTests.cs b/Darling/Darling.Tests/MigrationLockWaitContentionTests.cs index 2cb5c8591c..85787468af 100644 --- a/Darling/Darling.Tests/MigrationLockWaitContentionTests.cs +++ b/Darling/Darling.Tests/MigrationLockWaitContentionTests.cs @@ -61,60 +61,8 @@ public sealed class MigrationLockWaitContentionTests /// private const int TestWaitBudgetSeconds = 3; - /// - /// Ceiling for an UNCONTENDED acquire. Two-sided, and the upper side is what does the work: the - /// shipped poll interval is one second, so any ceiling below a second separates "took the lock on the - /// first attempt" from "slept once first" — which matters because moving the sleep above the first - /// attempt is a RELOCATION, and every occurrence count stays 1, so no structural check sees it and - /// only elapsed time can. The lower side is headroom: the operation is a single round trip on a - /// connection that is already open and already migrated, which is sub-millisecond against a local - /// store, so 750 ms leaves roughly three orders of magnitude before a slow runner can flake it. - /// - private static readonly TimeSpan UncontendedAcquireCeiling = TimeSpan.FromMilliseconds(750); - private static string? ConnectionString => Environment.GetEnvironmentVariable("DARLING_TEST_PG"); - [Fact] - public async Task AcquireSucceedsFirstAttempt_WhenNobodyHoldsTheLock_AgainstDevPostgres() - { - var connectionString = ConnectionString; - Assert.SkipWhen(string.IsNullOrEmpty(connectionString), - "Set DARLING_TEST_PG to a Postgres connection string to run the live migration-lock tests."); - - await using var connection = new NpgsqlConnection(connectionString); - await connection.OpenAsync(TestContext.Current.CancellationToken); - await PgMigrations.MigrateAsync(connection, TestContext.Current.CancellationToken); - - /* The uncontended path must cost nothing: polling is only a fallback shape, and if the first - attempt did not succeed outright then every ordinary service start pays a poll interval. */ - var started = Stopwatch.StartNew(); - Assert.True(await PgMigrations.TryAcquireMigrationLockForTestsAsync( - connection, logger: null, TestWaitBudgetSeconds, TestContext.Current.CancellationToken)); - started.Stop(); - - /* Release in a finally, not after the assertion. Npgsql pools by default, so disposing the - connection hands the physical session — advisory lock and all — back to the pool rather than - closing it, and a lock leaked by a failed assertion then blocks every later class in this - collection until the pool prunes that connection. Measured: a leak here cost a sibling test - 290 s of waiting on a lock nothing was using. */ - var bodySucceeded = false; - try - { - Assert.True( - started.Elapsed < UncontendedAcquireCeiling, - $"An uncontended acquire took {started.Elapsed.TotalMilliseconds:F0}ms, over the " - + $"{UncontendedAcquireCeiling.TotalMilliseconds:F0}ms ceiling — it slept a poll interval " - + "instead of taking the lock on its first attempt."); - bodySucceeded = true; - } - finally - { - await LiveStoreCleanup.RunOwnedAsync( - bodySucceeded, - () => ReleaseAsync(connection, CancellationToken.None)); - } - } - [Fact] public async Task ExpiryReturnsFalseWithoutThrowing_WhenTheStoreIsAlreadyCurrent_AgainstDevPostgres() { diff --git a/Darling/Darling.Tests/PasswordKeyTablesLiveTests.cs b/Darling/Darling.Tests/PasswordKeyTablesLiveTests.cs index 8bf3f11916..135f268826 100644 --- a/Darling/Darling.Tests/PasswordKeyTablesLiveTests.cs +++ b/Darling/Darling.Tests/PasswordKeyTablesLiveTests.cs @@ -24,7 +24,7 @@ namespace Darling.Tests; /// unique current key, the one pending marker row and the two reads. The roles carry fixed cluster-wide /// names, so a cluster that already has them skips these facts (the same rule the security-split facts follow). /// -[Collection("live-postgres")] +[Collection("pg-cluster-roles")] public sealed class PasswordKeyTablesLiveTests { /// One statement per table and verb; each statement reaches the row trigger (or the TRUNCATE trigger) of its table. diff --git a/Darling/Darling.Tests/PgClusterObjectCensusTests.cs b/Darling/Darling.Tests/PgClusterObjectCensusTests.cs new file mode 100644 index 0000000000..c55736c229 --- /dev/null +++ b/Darling/Darling.Tests/PgClusterObjectCensusTests.cs @@ -0,0 +1,338 @@ +/* + * Copyright (c) 2026 Erik Darling, Darling Data LLC + * + * This file is part of the SQL Server Performance Monitor. + * + * Licensed under the MIT License. See LICENSE file in the project root for full license information. + */ + +using System; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using System.Reflection; +using System.Runtime.CompilerServices; +using System.Text.RegularExpressions; +using Xunit; + +namespace Darling.Tests; + +/// +/// #5602: a live test changes only Postgres objects that no other test can see, or it runs in the +/// pg-cluster-roles collection, which runs alone. +/// +/// Roles, role memberships, database-level grants, ALTER ROLE ... SET, ALTER DATABASE ... SET and +/// ALTER SYSTEM are stored for the whole cluster, not for the scratch database a test made. Two tests that +/// wrote one of them at the same time failed with "tuple concurrently updated" (#5560), and the product's +/// provisioning batch writes FIXED role names, so every test that runs it shares them. This census reads each +/// test class that reaches the DARLING_TEST_PG cluster (the same reach test +/// uses) and fails when its code (comments stripped) does one of +/// these and the class is neither in nor on : +/// +/// CREATE/ALTER/DROP ROLE, DROP OWNED BY, GRANT ... TO or REVOKE ... FROM +/// with a LITERAL role name. A name built with {...} from a per-run token is fine: nobody else writes +/// that catalog row. +/// ALTER SYSTEM SET/RESET, ALTER DATABASE x SET and GRANT/REVOKE ... ON DATABASE x +/// with a literal database name (a scratch name built with {...} is fine). +/// A call of a product entry point that writes the fixed roles: the provisioning methods, the +/// statement_timeout re-assert, or the DarlingManagedPostgres.*RoleName constants. +/// +/// A lone SQL string that a test only ASSERTS on (a line with Assert. or Regex on it) is not an +/// execution and is skipped. Every allow-list entry carries its reason, and an entry that no longer matches +/// anything fails too, so the list cannot go stale. +/// +/* #1776 own-store: this class reads test source files and never connects to a database; the quoted store names in it are + patterns it searches for. */ +[Trait("Stage", "Guard")] +public sealed class PgClusterObjectCensusTests +{ + private const string CollectionName = "pg-cluster-roles"; + + private static readonly string[] SharedClusterReach = { "\"DARLING_TEST_PG\"", "ScratchPostgres.CreateAsync(", "LivePostgresStoreFixture" }; + + /// Classes that reach the shared cluster, match a rule, and are safe. Every entry says why. + internal static readonly IReadOnlyDictionary AllowList = new Dictionary(StringComparer.Ordinal) + { + ["PgStatementTextScrubLiveTests"] = + "the ALTER ROLE text is a sample statement fed to the scrubber as data; the class runs no role DDL", + ["StatementAnalysisReadCutLiveTests"] = + "the CREATE ROLE text is a sample statement fed to the read cut as data; the class runs no role DDL", + ["StoreLogRemaskLiveTests"] = + "the ALTER ROLE text is a sample log line fed to the re-mask as data; the class runs no role DDL", + ["StoreStatementStatsLiveTests"] = + "the ALTER ROLE and ALTER SYSTEM text is sample statement text fed to the scrubber as data; every role and database this class really creates carries a per-run suffix", + ["MigrationUpgradeLadderLiveTests"] = + "creates the cluster's own bootstrap role name only when it is missing (CREATE ROLE darling ... IF NOT EXISTS); on a CI cluster it already exists, so the write never happens", + ["AlertHistoryDismissTests"] = + "builds the shipped provisioning text and retargets its viewer GRANT at a per-run role; the batch itself is never executed", + ["ViewerStoreLoginRawReadLiveTests"] = + "lifts two shipped GRANT statements out of the provisioning text and runs them retargeted at a per-run role", + ["ServerAddViewerRoleLiveTests"] = + "lifts the shipped viewer GRANT statements out of the provisioning text and runs them retargeted at a per-run role", + ["ServerEditLiveTests"] = + "lifts the shipped viewer GRANT statements out of the provisioning text and runs them retargeted at a per-run role", + ["ServerTagMcpRoleLiveTests"] = + "lifts the shipped mcp GRANT statements out of the provisioning text and runs them retargeted at a per-run role", + ["ServerTagViewerRoleLiveTests"] = + "lifts the shipped viewer GRANT statements out of the provisioning text and runs them retargeted at a per-run role", + ["ComposeTempFileLimitLiveTests"] = + "renders the shipped compose-store role statements and retargets them at a per-run role; the fixed roles are not written", + }; + + /// + /// The allow-listed classes whose only matches are sample statement text, pinned to the exact code lines that match + /// (trimmed). The class-level allow entry above would let a NEW literal-role statement into one of these files + /// unnoticed, so a match that is not on this list fails, and a listed line that no longer matches fails too (#5602). + /// + internal static readonly IReadOnlyDictionary PinnedSampleText = new Dictionary(StringComparer.Ordinal) + { + ["PgStatementTextScrubLiveTests"] = new string[] + { + @"private const string Secret = ""ALTER ROLE app PASSWORD 'secret-x'"";", + }, + ["StatementAnalysisReadCutLiveTests"] = new string[] + { + @"var marker = ""SELECT marker_pg_ssf"" + string.Concat(Enumerable.Repeat("", 1"", 800)) + "" ; CREATE ROLE zz_ssf"";", + }, + ["StoreLogRemaskLiveTests"] = new string[] + { + @"Prefix + ""ERROR: canceling statement due to statement timeout\n"" + Prefix + ""STATEMENT: ALTER ROLE admin PASSWORD 'Remask3915a'""),", + }, + ["StoreStatementStatsLiveTests"] = new string[] + { + @"(""ALTER ROLE app PASSWORD 'x'"", false, true),", + @"(""ALTER SYSTEM SET primary_conninfo = 'host=x password=hunter2'"", false, true),", + @"(""ALTER ROLE app PASSWORD $$x$$"", false, true),", + @"(""ALTER ROLE app PASSWORD E'x'"", false, true),", + }, + }; + + [Fact] + public void TheCollectionThatRunsAlone_IsDefinedWithParallelizationOff() + { + var definition = typeof(PgClusterRolesCollection).GetCustomAttribute(); + Assert.NotNull(definition); + Assert.Equal(CollectionName, definition!.Name); + Assert.True(definition.DisableParallelization, + $"The '{CollectionName}' collection exists so its members run alone, after every parallel collection (#5602). " + + "Without DisableParallelization it is only another collection that overlaps the rest, and the census below " + + "would be pointing tests at a lock that does not exist."); + } + + [Fact] + public void EveryLiveClassThatChangesAClusterWideObjectUnderAFixedName_RunsInThePgClusterRolesCollection_OrIsAllowListed() + { + var offenders = new List(); + var allowed = new HashSet(StringComparer.Ordinal); + foreach (var hit in ScanClasses()) + { + if (hit.InCollection) + { + continue; + } + + if (AllowList.ContainsKey(hit.ClassName)) + { + allowed.Add(hit.ClassName); + if (PinnedSampleText.TryGetValue(hit.ClassName, out var pinned) && !pinned.Contains(hit.Text, StringComparer.Ordinal)) + { + offenders.Add($"{hit.File}:{hit.Line} {hit.ClassName} [{hit.Rule}] {hit.Text} (NOT PINNED: this class is allowed only for the sample statement text listed in PgClusterObjectCensusTests.PinnedSampleText)"); + } + + continue; + } + + offenders.Add($"{hit.File}:{hit.Line} {hit.ClassName} [{hit.Rule}] {hit.Text}"); + } + + Assert.True(offenders.Count == 0, + "These test classes reach the DARLING_TEST_PG cluster and change a cluster-wide Postgres object (a role, a " + + "grant, a role or database setting, ALTER SYSTEM) under a fixed name, or run a product entry point that " + + "writes the fixed admin/viewer/mcp roles, yet they are not in the " + CollectionName + " collection:\n\n" + + string.Join("\n", offenders) + + "\n\nPick one, deliberately (#5602):\n" + + $" - Add [Collection(\"{CollectionName}\")] (a DisableParallelization collection: it runs alone) when the test must " + + "write the fixed roles or the cluster settings.\n" + + " - Give the object a name built from the scratch database name or a per-run token ({...}), so no other test can see it.\n" + + " - Or add the class to PgClusterObjectCensusTests.AllowList with the reason it is safe."); + } + + [Fact] + public void EveryAllowListEntry_StillMatchesARuleInAClassThatReachesTheCluster() + { + var matched = ScanClasses().Where(h => !h.InCollection).Select(h => h.ClassName).ToHashSet(StringComparer.Ordinal); + var stale = AllowList.Keys.Where(name => !matched.Contains(name)).OrderBy(name => name, StringComparer.Ordinal).ToList(); + + Assert.True(stale.Count == 0, + "These allow-list entries no longer match anything (the class was removed, joined the " + + CollectionName + " collection, or stopped doing the thing). Delete them:\n" + string.Join("\n", stale)); + + foreach (var (name, reason) in AllowList) + { + Assert.False(string.IsNullOrWhiteSpace(reason), $"allow-list entry {name} needs a reason"); + } + } + + [Fact] + public void EveryPinnedSampleStatement_StillMatchesInItsClass() + { + var seen = ScanClasses().Where(h => !h.InCollection).Select(h => (h.ClassName, h.Text)).ToHashSet(); + var stale = PinnedSampleText + .SelectMany(p => p.Value.Select(text => (Class: p.Key, Text: text))) + .Where(p => !seen.Contains((p.Class, p.Text))) + .Select(p => p.Class + " " + p.Text) + .ToList(); + + Assert.True(stale.Count == 0, + "These pinned sample statements no longer match a rule in their class (the line changed or went away). Update or delete them in " + + "PgClusterObjectCensusTests.PinnedSampleText:\n" + string.Join("\n", stale)); + + foreach (var name in PinnedSampleText.Keys) + { + Assert.True(AllowList.ContainsKey(name), $"pinned class {name} is not on the allow list"); + } + } + + // ---- rules ------------------------------------------------------------------------------------------- + + /// (rule name, pattern). A pattern matches one code line; a name built with { never matches. + private static readonly (string Name, Regex Pattern)[] Rules = + { + ("role DDL with a literal name", + new Regex(@"\b(?:CREATE|ALTER|DROP)\s+(?:ROLE|USER)\s+(?:IF\s+(?:NOT\s+)?EXISTS\s+)?(?!IF\b|NOT\b|EXISTS\b)[A-Za-z_]\w*", RegexOptions.Compiled)), + ("DROP OWNED BY a literal role", + new Regex(@"\bDROP\s+OWNED\s+BY\s+[A-Za-z_]\w*", RegexOptions.Compiled)), + ("GRANT/REVOKE to or from a literal role", + new Regex(@"\b(?:GRANT\b[^;""]*?\sTO|REVOKE\b[^;""]*?\sFROM)\s+(?!PUBLIC\b|pg_|CURRENT_USER\b|SESSION_USER\b)[A-Za-z_]\w*", RegexOptions.Compiled)), + ("ALTER SYSTEM", + new Regex(@"\bALTER\s+SYSTEM\s+(?:SET|RESET)\b", RegexOptions.Compiled)), + ("ALTER DATABASE with a literal name", + new Regex(@"\bALTER\s+DATABASE\s+[A-Za-z_]\w*\s+SET\b", RegexOptions.Compiled)), + ("database-level grant on a literal database", + new Regex(@"\bON\s+DATABASE\s+[A-Za-z_]\w*", RegexOptions.Compiled)), + ("product entry point that writes the fixed roles", + new Regex(@"\b(?:EnsureProvisionedAsync|EnsureComposeStoreProvisionedAsync|ProvisionComposeStoreAsync|ProvisionRolesAsync|ReassertComposeStatementTimeoutAsync|BuildProvisioningSql|BuildComposeStatementTimeoutSql)\s*\(", RegexOptions.Compiled)), + ("the fixed role-name constants", + new Regex(@"\bDarlingManagedPostgres\.(?:Admin|Viewer|Mcp)RoleName\b", RegexOptions.Compiled)), + }; + + private static readonly Regex ClassDeclaration = + new(@"^(?:public|internal)\s+(?:sealed\s+|static\s+|abstract\s+|partial\s+)*class\s+(\w+)", + RegexOptions.Compiled | RegexOptions.Multiline); + + private static readonly Regex BlockComment = new(@"/\*.*?\*/", RegexOptions.Compiled | RegexOptions.Singleline); + + private static readonly Regex LineComment = new(@"//[^\r\n]*", RegexOptions.Compiled); + + private sealed record Hit(string File, int Line, string ClassName, string Rule, string Text, bool InCollection); + + private static List ScanClasses() + { + var directory = FindTestProjectDirectory(); + Assert.True(directory is not null, + "Could not locate the Darling.Tests source directory (walked up from the test binary looking for " + + "PerformanceMonitor.sln). This test scans source, so it fails rather than skips: a guard that skips stops guarding."); + + var thisFile = Path.GetFileName(ThisSourceFile()); + var hits = new List(); + foreach (var file in Directory.EnumerateFiles(directory!, "*.cs", SearchOption.AllDirectories)) + { + if (HasBuildOutputSegment(Path.GetRelativePath(directory!, file)) + || string.Equals(Path.GetFileName(file), thisFile, StringComparison.Ordinal)) + { + continue; + } + + var text = StripComments(File.ReadAllText(file)); + if (!ReachesCluster(text)) + { + continue; + } + + var declarations = ClassDeclaration.Matches(text); + for (var i = 0; i < declarations.Count; i++) + { + var declaration = declarations[i]; + var end = i + 1 < declarations.Count ? declarations[i + 1].Index : text.Length; + var body = text[declaration.Index..end]; + if (!ReachesCluster(body)) + { + continue; + } + + var inCollection = HeaderAbove(text, declaration.Index).Contains($"[Collection(\"{CollectionName}\")]", StringComparison.Ordinal); + var firstLine = text.AsSpan(0, declaration.Index).Count('\n') + 1; + var lines = body.Split('\n'); + for (var n = 0; n < lines.Length; n++) + { + var line = lines[n]; + if (line.Contains("Assert.", StringComparison.Ordinal) || line.Contains("Regex", StringComparison.Ordinal)) + { + continue; + } + + foreach (var (name, pattern) in Rules) + { + if (pattern.IsMatch(line)) + { + hits.Add(new Hit(Path.GetFileName(file), firstLine + n, declaration.Groups[1].Value, name, line.Trim(), inCollection)); + break; + } + } + } + } + } + + return hits; + } + + private static bool ReachesCluster(string source) => SharedClusterReach.Any(token => source.Contains(token, StringComparison.Ordinal)); + + /// Blanks comments but keeps every newline, so a reported line number is the file's own. + private static string StripComments(string source) + { + var withoutBlocks = BlockComment.Replace(source, m => new string('\n', m.Value.Count(c => c == '\n'))); + return LineComment.Replace(withoutBlocks, string.Empty); + } + + private static string HeaderAbove(string text, int declarationIndex) + { + var start = declarationIndex; + for (var lines = 0; lines < 25 && start > 0; lines++) + { + var previous = text.LastIndexOf('\n', start - 1); + if (previous < 0) + { + start = 0; + break; + } + + start = previous; + } + + return text[start..declarationIndex]; + } + + private static bool HasBuildOutputSegment(string relativePath) => + relativePath.Split('/', '\\').Any(segment => + string.Equals(segment, "bin", StringComparison.OrdinalIgnoreCase) || string.Equals(segment, "obj", StringComparison.OrdinalIgnoreCase)); + + private static string ThisSourceFile([CallerFilePath] string? path = null) => path!; + + private static string? FindTestProjectDirectory() + { + var directory = new DirectoryInfo(AppContext.BaseDirectory); + for (var i = 0; i < 10 && directory is not null; i++) + { + if (File.Exists(Path.Combine(directory.FullName, "PerformanceMonitor.sln"))) + { + var source = Path.Combine(directory.FullName, "Darling", "Darling.Tests"); + return Directory.Exists(source) ? source : null; + } + + directory = directory.Parent; + } + + return null; + } +} diff --git a/Darling/Darling.Tests/PgClusterRolesCollection.cs b/Darling/Darling.Tests/PgClusterRolesCollection.cs new file mode 100644 index 0000000000..2547388e47 --- /dev/null +++ b/Darling/Darling.Tests/PgClusterRolesCollection.cs @@ -0,0 +1,39 @@ +/* + * Copyright (c) 2026 Erik Darling, Darling Data LLC + * + * This file is part of the SQL Server Performance Monitor. + * + * Licensed under the MIT License. See LICENSE file in the project root for full license information. + */ + +using Xunit; + +namespace Darling.Tests; + +/// +/// The one place that runs the live classes whose effect is not confined to their own scratch database (#5602). +/// +/// ScratchPostgres gives a test its own database, but a role, a role membership, a database-level +/// grant, an ALTER ROLE ... SET and an ALTER SYSTEM write live in the CLUSTER's catalogs and +/// postgresql.auto.conf. Two tests that wrote the same one of them at the same time made Postgres refuse +/// the second with "tuple concurrently updated" (#5560). The product provisions FIXED role names +/// (admin, viewer, mcp), so any test that runs the product's provisioning batch, or drops or +/// alters those roles, shares them with every other such test whatever scratch database it aims at, and a test +/// cannot rename them without a product change. +/// +/// Tests whose roles carry a per-run name (a GUID suffix, or the scratch database's name) need nothing: +/// nobody else can see the catalog row they write. This collection is for the rest. It carries +/// DisableParallelization, so xUnit runs it alone, after every parallel collection has finished: nothing +/// else touches the cluster while a member runs. That is stronger than live-postgres, whose members only +/// take turns with each other and still overlap every class outside the collection. +/// +/// It deliberately has no collection fixture: its members build their own scratch database (or change +/// only cluster settings), so none of them reads the shared store's migrated tables, and a second +/// would run its residue check against a store the parallel collections +/// are still writing. fails when a live class changes a cluster-wide +/// object under a fixed name and is neither here nor on its allow list. +/// +[CollectionDefinition("pg-cluster-roles", DisableParallelization = true)] +public sealed class PgClusterRolesCollection +{ +} diff --git a/Darling/Darling.Tests/PgLogEventsPipelineTests.cs b/Darling/Darling.Tests/PgLogEventsPipelineTests.cs index 9aaac9d472..d1cb9f1cbd 100644 --- a/Darling/Darling.Tests/PgLogEventsPipelineTests.cs +++ b/Darling/Darling.Tests/PgLogEventsPipelineTests.cs @@ -38,6 +38,7 @@ namespace Darling.Tests; /// (its own fixtures are the source of the %Q-glued query id and the managed %t:%r:%u@%d:[%p]: /// prefix). Identifiers are synthesised; shapes are verbatim. /// +[Collection("timing")] public sealed class PgLogEventsPipelineTests { /* ---- fixtures ------------------------------------------------------------------------------------ */ diff --git a/Darling/Darling.Tests/PgSensitiveStatementFilterLiveTests.cs b/Darling/Darling.Tests/PgSensitiveStatementFilterLiveTests.cs index dbdf36c6f2..3e8303adbd 100644 --- a/Darling/Darling.Tests/PgSensitiveStatementFilterLiveTests.cs +++ b/Darling/Darling.Tests/PgSensitiveStatementFilterLiveTests.cs @@ -36,6 +36,7 @@ namespace Darling.Tests; /// #1776 own-store — mints its own scratch database () rather than /// sharing the live fixture, so it is deliberately NOT in the live-postgres collection. /// +[Collection("timing")] public sealed class PgSensitiveStatementFilterLiveTests { /// diff --git a/Darling/Darling.Tests/PgServerConfigPendingRestartLiveTests.cs b/Darling/Darling.Tests/PgServerConfigPendingRestartLiveTests.cs index 0a36c5e850..3a706982ac 100644 --- a/Darling/Darling.Tests/PgServerConfigPendingRestartLiveTests.cs +++ b/Darling/Darling.Tests/PgServerConfigPendingRestartLiveTests.cs @@ -26,7 +26,7 @@ namespace Darling.Tests; /// collector query against the SAME state (the fix), rather than asserting against a canned fixture — the /// point of #4251 is that the two disagree. /// -[Collection("live-postgres")] +[Collection("pg-cluster-roles")] public sealed class PgServerConfigPendingRestartLiveTests { private static string? ConnectionString => Environment.GetEnvironmentVariable("DARLING_TEST_PG"); diff --git a/Darling/Darling.Tests/PgSettingRedactionLivePostgresTests.cs b/Darling/Darling.Tests/PgSettingRedactionLivePostgresTests.cs index 2a7c036725..bfea3b5716 100644 --- a/Darling/Darling.Tests/PgSettingRedactionLivePostgresTests.cs +++ b/Darling/Darling.Tests/PgSettingRedactionLivePostgresTests.cs @@ -28,7 +28,7 @@ namespace Darling.Tests; /// Cluster-wide state (ALTER SYSTEM is not per-database), so this is serialized with the rest of /// the live-postgres collection and restores the setting in a finally that runs even on failure. /// -[Collection("live-postgres")] +[Collection("pg-cluster-roles")] public sealed class PgSettingRedactionLivePostgresTests { /* #4981: a role belongs to the whole cluster, so the name is unique to the run (8 lowercase hex characters). */ diff --git a/Darling/Darling.Tests/PgSettingRedactorTests.cs b/Darling/Darling.Tests/PgSettingRedactorTests.cs index 7062bec516..5f61ba18a1 100644 --- a/Darling/Darling.Tests/PgSettingRedactorTests.cs +++ b/Darling/Darling.Tests/PgSettingRedactorTests.cs @@ -16,11 +16,10 @@ namespace Darling.Tests; /// primary_conninfo, which can carry a replication password in plain text. These pin /// 's rules directly, without a database, so the corpus runs on every build. /// -/// The class runs in [Collection("pg-setting-redactor-timing")], which no other class shares and -/// which does not run in parallel with any other collection: its scaling check compares wall-clock times, and +/// The class runs in [Collection("timing")], which does not run in parallel with any other collection: its scaling check compares wall-clock times, and /// on a busy machine the rest of the suite could slow every repeat of the larger size past the bar (#4788). /// -[Collection("pg-setting-redactor-timing")] +[Collection("timing")] public sealed class PgSettingRedactorTests { /// diff --git a/Darling/Darling.Tests/PgSettingRedactorTimingCollection.cs b/Darling/Darling.Tests/PgSettingRedactorTimingCollection.cs deleted file mode 100644 index 7b11aa3318..0000000000 --- a/Darling/Darling.Tests/PgSettingRedactorTimingCollection.cs +++ /dev/null @@ -1,24 +0,0 @@ -/* - * Copyright (c) 2026 Erik Darling, Darling Data LLC - * - * This file is part of the SQL Server Performance Monitor. - * - * Licensed under the MIT License. See LICENSE file in the project root for full license information. - */ - -using Xunit; - -namespace Darling.Tests; - -/// -/// Runs PgSettingRedactorTests with no other test class running (#4788). Its scaling check times -/// PgSettingRedactor.Redact at two input sizes and fails when the larger one takes 64 times as long as -/// the smaller. Run in parallel with the rest of a 17,000-test suite, a busy machine can slow every repeat of -/// the larger size past that bar, and a guard that fails on a busy machine gets re-run instead of believed. -/// gives the timed calls a quiet process, so -/// the bar stays at 64 times and a real quadratic pattern still fails it. -/// -[CollectionDefinition("pg-setting-redactor-timing", DisableParallelization = true)] -public sealed class PgSettingRedactorTimingCollection -{ -} diff --git a/Darling/Darling.Tests/PlanViewerRuntimeSummaryNestingTests.cs b/Darling/Darling.Tests/PlanViewerRuntimeSummaryNestingTests.cs index e0d3d665f2..b72d5c528c 100644 --- a/Darling/Darling.Tests/PlanViewerRuntimeSummaryNestingTests.cs +++ b/Darling/Darling.Tests/PlanViewerRuntimeSummaryNestingTests.cs @@ -43,7 +43,7 @@ public sealed class PlanViewerRuntimeSummaryNestingTests [Fact] public void RuntimeSummary_EarlyAbortUnderOptimization_IndentsItsLabelOnly() { - OnStaThread(() => + StaTestThread.Run(() => { var control = new PlanViewerControl(); try @@ -85,7 +85,7 @@ public void RuntimeSummary_EarlyAbortUnderOptimization_IndentsItsLabelOnly() [Fact] public void RuntimeSummary_EarlyAbortWithNoOptimizationRow_IsNotIndented() { - OnStaThread(() => + StaTestThread.Run(() => { var control = new PlanViewerControl(); try @@ -125,23 +125,4 @@ private static (TextBlock Label, TextBlock Value) RowOf(Grid grid, string label) return (labelText, valueText); } - /// WPF objects require STA; same shape as the other WPF tests here. - private static void OnStaThread(Action body) - { - Exception? error = null; - using var staGate = WpfStaGate.Enter(); - var thread = new Thread(() => - { - try { body(); } - catch (Exception ex) { error = ex; } - }); - thread.SetApartmentState(ApartmentState.STA); - thread.Start(); - thread.Join(); - - if (error is not null) - { - throw error; - } - } } diff --git a/Darling/Darling.Tests/ProvisioningSerializationTests.cs b/Darling/Darling.Tests/ProvisioningSerializationTests.cs index 5eb957fc7e..5751ec261b 100644 --- a/Darling/Darling.Tests/ProvisioningSerializationTests.cs +++ b/Darling/Darling.Tests/ProvisioningSerializationTests.cs @@ -652,7 +652,7 @@ never beside them. */ /// scratch store: it must apply in full inside one explicit transaction (nothing in it may need to run outside one), and /// it must be re-runnable, since a second service re-runs it right after the first. /// -[Collection("live-postgres")] +[Collection("pg-cluster-roles")] public sealed class ProvisioningBatchLockedTransactionLiveTests { private static string? ConnectionString => Environment.GetEnvironmentVariable("DARLING_TEST_PG"); diff --git a/Darling/Darling.Tests/QueryStoreIntervalBrinLiveTests.cs b/Darling/Darling.Tests/QueryStoreIntervalBrinLiveTests.cs index dcb5dd5be3..dce3c18a14 100644 --- a/Darling/Darling.Tests/QueryStoreIntervalBrinLiveTests.cs +++ b/Darling/Darling.Tests/QueryStoreIntervalBrinLiveTests.cs @@ -28,6 +28,7 @@ namespace Darling.Tests; /* #1776 own-store: deliberately NOT [Collection("live-postgres")]. Every test reaches DARLING_TEST_PG only to CREATE and DROP its own database through ScratchPostgres and works entirely inside it, so it cannot race live collection. */ +[Collection("timing")] public sealed class QueryStoreIntervalBrinLiveTests { private const string Brin = "ix_query_store_interval_wide_collection_time_brin"; diff --git a/Darling/Darling.Tests/RawRetentionPurgeJobLiveTests.cs b/Darling/Darling.Tests/RawRetentionPurgeJobLiveTests.cs index b7b2ad6c5d..884a281526 100644 --- a/Darling/Darling.Tests/RawRetentionPurgeJobLiveTests.cs +++ b/Darling/Darling.Tests/RawRetentionPurgeJobLiveTests.cs @@ -30,6 +30,7 @@ namespace Darling.Tests; /// #1776 own-store: deliberately NOT [Collection("live-postgres")]. Goes through /// and never touches the shared database's tables. /// +[Collection("timing")] public sealed class RawRetentionPurgeJobLiveTests { private const string Raw = "query_stats"; diff --git a/Darling/Darling.Tests/RawWindowFloorViewerPortTests.cs b/Darling/Darling.Tests/RawWindowFloorViewerPortTests.cs index c12aa5c229..527784c1cd 100644 --- a/Darling/Darling.Tests/RawWindowFloorViewerPortTests.cs +++ b/Darling/Darling.Tests/RawWindowFloorViewerPortTests.cs @@ -33,7 +33,7 @@ public sealed class RawWindowFloorViewerPortTests [Fact] public void UpdateTruncationBanner_ShowsSinceEffectiveStart_WhenTheFloorIsPastTheSlack() { - OnStaThread(() => + StaTestThread.Run(() => { var banner = new TextBlock(); var floor = RequestedStart.AddDays(3); @@ -48,7 +48,7 @@ public void UpdateTruncationBanner_ShowsSinceEffectiveStart_WhenTheFloorIsPastTh [Fact] public void UpdateTruncationBanner_StaysCollapsed_WhenTheFloorIsInsideTheSlack() { - OnStaThread(() => + StaTestThread.Run(() => { /* 30 minutes after the requested start — inside DurationTrendRouting.TruncationSlack's 90-minute allowance, so a normal cadence-start lag, not a retention cut (the #2364 / #4231 ruling). Seeded @@ -65,7 +65,7 @@ public void UpdateTruncationBanner_StaysCollapsed_WhenTheFloorIsInsideTheSlack() [Fact] public void UpdateTruncationBanner_StaysCollapsed_WhenTheProbeFoundNoFloorAtAll() { - OnStaThread(() => + StaTestThread.Run(() => { var banner = new TextBlock(); @@ -75,27 +75,6 @@ public void UpdateTruncationBanner_StaysCollapsed_WhenTheProbeFoundNoFloorAtAll( }); } - /// WPF objects require STA; same shape as Lite.Tests' MainWindowAccessKeyTests / Dashboard.Tests' - /// DataGridExport tests. - private static void OnStaThread(Action body) - { - Exception? error = null; - using var staGate = WpfStaGate.Enter(); - var thread = new Thread(() => - { - try { body(); } - catch (Exception ex) { error = ex; } - }); - thread.SetApartmentState(ApartmentState.STA); - thread.Start(); - thread.Join(); - - if (error is not null) - { - throw error; - } - } - private static string ViewerFile(string file) => ReadRepoFile("Darling", "PerformanceMonitor.Darling.Viewer", file); [Theory] @@ -115,7 +94,7 @@ makes the same check on the MCP reader. */ [Fact] public void UpdateTruncationBanner_TableServed_NamesTheEffectiveStartAndTheBound_AndTheRawSlicerFloor() { - OnStaThread(() => + StaTestThread.Run(() => { var wideStart = RequestedStart.AddDays(1); var plan = new QueryStoreIntervalWide.WideReadPlan( @@ -151,7 +130,7 @@ public void UpdateTruncationBanner_TableServed_NamesTheEffectiveStartAndTheBound [Fact] public void UpdateTruncationBanner_HourlyServed_NamesTheGridsStart_AndRawsFloorAsTheSlicers() { - OnStaThread(() => + StaTestThread.Run(() => { const string Suffix = " - aggregated hourly"; var banner = new TextBlock(); diff --git a/Darling/Darling.Tests/ScratchPostgresExitDrainTests.cs b/Darling/Darling.Tests/ScratchPostgresExitDrainTests.cs index e397f30cd2..8f7f1ed352 100644 --- a/Darling/Darling.Tests/ScratchPostgresExitDrainTests.cs +++ b/Darling/Darling.Tests/ScratchPostgresExitDrainTests.cs @@ -18,6 +18,7 @@ namespace Darling.Tests; /// /* #1776 own-store: the dead-cluster cases never connect to a store, and the live case creates one scratch database through ScratchPostgres, so it does not read or write the shared store's tables. */ +[Collection("timing")] public sealed class ScratchPostgresExitDrainTests { /// A port nothing listens on: bind to port 0, read the port the OS chose, release it. diff --git a/Darling/Darling.Tests/ScratchPostgresQuiesceTests.cs b/Darling/Darling.Tests/ScratchPostgresQuiesceTests.cs index a91816fd58..56b514144c 100644 --- a/Darling/Darling.Tests/ScratchPostgresQuiesceTests.cs +++ b/Darling/Darling.Tests/ScratchPostgresQuiesceTests.cs @@ -19,6 +19,7 @@ namespace Darling.Tests; /// WITH (FORCE), so no TimescaleDB policy worker is mid-run in the database being dropped (CI saw a worker die with /// 0xC0000005 twice, both with such a drop in flight). It is best-effort: it never throws and never blocks the drop. /// +[Collection("timing")] public sealed class ScratchPostgresQuiesceLiveTests { [Fact] diff --git a/Darling/Darling.Tests/SensitiveStatementAutoParamTests.cs b/Darling/Darling.Tests/SensitiveStatementAutoParamTests.cs index 988f47c8ad..d3449daaff 100644 --- a/Darling/Darling.Tests/SensitiveStatementAutoParamTests.cs +++ b/Darling/Darling.Tests/SensitiveStatementAutoParamTests.cs @@ -31,6 +31,7 @@ namespace Darling.Tests; /// string or binary literal; and pwd= as a connection-string key), with ASCII word boundaries. The real judge /// runs over the captured fixtures in the wiring wave. /// +[Collection("timing")] public class SensitiveStatementAutoParamTests { private const string P = "<

>"; diff --git a/Darling/Darling.Tests/SensitiveStatementsParityLiveTests.cs b/Darling/Darling.Tests/SensitiveStatementsParityLiveTests.cs index 726aa2b75f..d513102ea8 100644 --- a/Darling/Darling.Tests/SensitiveStatementsParityLiveTests.cs +++ b/Darling/Darling.Tests/SensitiveStatementsParityLiveTests.cs @@ -27,6 +27,7 @@ namespace Darling.Tests; /// #1776 own-store - mints its own scratch database () rather than /// sharing the live fixture, so it is deliberately NOT in the live-postgres collection. /// +[Collection("timing")] public sealed class SensitiveStatementsParityLiveTests { [Fact] diff --git a/Darling/Darling.Tests/ServerTabToolbarAndTooltipsTests.cs b/Darling/Darling.Tests/ServerTabToolbarAndTooltipsTests.cs index 7a46f4b68f..5ae2751fc7 100644 --- a/Darling/Darling.Tests/ServerTabToolbarAndTooltipsTests.cs +++ b/Darling/Darling.Tests/ServerTabToolbarAndTooltipsTests.cs @@ -155,27 +155,6 @@ public void FinOpsFindingsGrid_DetailAndSavingsHeaders_HaveATooltip(string app) } } - private static T OnStaThread(Func body) - { - T result = default!; - Exception? error = null; - using var staGate = WpfStaGate.Enter(); - var thread = new Thread(() => - { - try { result = body(); } - catch (Exception ex) { error = ex; } - }); - thread.SetApartmentState(ApartmentState.STA); - thread.Start(); - thread.Join(); - if (error is not null) - { - System.Runtime.ExceptionServices.ExceptionDispatchInfo.Capture(error).Throw(); - } - - return result; - } - private static PlanNode Scan() => new() { PhysicalOp = "Clustered Index Scan", @@ -193,7 +172,7 @@ public void PlanOperatorBox_ExposesItsTooltipSummaryToAScreenReader() { var node = Scan(); - var (name, help, isControl) = OnStaThread(() => + var (name, help, isControl) = StaTestThread.Run(() => { var box = new PlanNodeBorder(); System.Windows.Automation.AutomationProperties.SetName(box, PlanNodeBorder.OperatorName(node)); diff --git a/Darling/Darling.Tests/StaTestThread.cs b/Darling/Darling.Tests/StaTestThread.cs new file mode 100644 index 0000000000..41585d1229 --- /dev/null +++ b/Darling/Darling.Tests/StaTestThread.cs @@ -0,0 +1,277 @@ +/* + * Copyright (c) 2026 Erik Darling, Darling Data LLC + * + * This file is part of the SQL Server Performance Monitor. + * + * Licensed under the MIT License. See LICENSE file in the project root for full license information. + */ + +using System; +using System.Collections.Generic; +using System.Runtime.ExceptionServices; +using System.Threading; +using System.Threading.Tasks; +using System.Windows; +using System.Windows.Threading; +using Xunit; + +///

+/// #5602: every test that needs WPF runs its body on ONE shared STA thread per test process, instead of starting a thread of its +/// own. WPF keeps process-wide state that is not thread-safe (#5559: the DependencyPropertyDescriptor cache and its +/// unsynchronised AddValueChanged dictionary) and per-thread state that is expensive to leave behind (#5596: a weak-event +/// table and a dispatcher per thread, 300 ms each at process exit). Fifty test files each starting their own thread meant both. +/// +/// One body at a time, end to end. A body that pumps (Dispatcher.PushFrame, a modal loop) lets a second queued +/// Invoke run inside it, which is #5559 again. So the CALLER takes a process-wide gate before it queues its body and +/// releases it after the body and its drain have returned; the dispatcher never has two bodies queued. The wait for the gate is +/// not charged to the hang guard. +/// +/// +/// Drain. After each body the thread closes any window the body left open and runs the dispatcher to application-idle, +/// so work one test queued does not run during the next. An exception thrown by queued work (a timer tick, a Loaded handler) +/// does not end the thread and the process: it is handed to the test whose body or drain ran it. +/// +/// +/// Hang guard. A body that does not return within fails the calling test, naming it. The shared +/// thread is then stuck inside that body and cannot be freed, so every later WPF test in the process fails at once with a +/// message naming the stuck test, instead of each waiting out its own 60 s. +/// +/// +/// Tests that must own a thread (a dispatcher shutdown, a thread that has to END while the test looks at the process) stay on +/// their own thread and take around it, so they cannot overlap a body running here. The allow list +/// is in StaThreadCensusTests, with a reason per file. +/// +/// This file lives in Darling.Tests and is compiled into Lite.Tests by a link (one host, two suites). It is in the global +/// namespace so the Lite classes, which sit in a different namespace, need no extra using. +/// +internal static class StaTestThread +{ + private static readonly StaHost s_host = new(); + + /// How long a body (and its drain) may take before the calling test fails. Settable so a test can shorten it. + internal static TimeSpan HangGuard { get => s_host.HangGuard; set => s_host.HangGuard = value; } + + /// Runs on the shared STA thread and returns when it and its drain are done. Its exception comes back here with its own stack. + internal static void Run(Action body) => s_host.Run(body); + + /// Runs on the shared STA thread and returns its result. + internal static T Run(Func body) => s_host.Run(body); + + /// Runs an async on the shared STA thread: its continuations come back to that thread's dispatcher. + internal static void Run(Func body) => s_host.Run(body); + + /// + /// For a test that must own its thread: waits for the gate and returns the token that releases it. Not re-entrant, and not to + /// be called from a body already running in . + /// + internal static IDisposable EnterExclusive() => s_host.EnterExclusive(); +} + +/// +/// The machinery behind : one lazily started background STA thread running a dispatcher, and the gate. +/// The suites use the single instance behind ; the host's own tests make their own, so the hang guard +/// can be proven without leaving the shared thread stuck for the rest of the process. +/// +internal sealed class StaHost +{ + internal TimeSpan HangGuard { get; set; } = TimeSpan.FromSeconds(60); + + /* A SemaphoreSlim, not a Monitor: a test may be async, so the releasing thread is not promised to be the taking one. */ + private readonly SemaphoreSlim _gate = new(1, 1); + private readonly object _startLock = new(); + private readonly List _strays = new(); + private Dispatcher? _dispatcher; + private volatile string? _stuckTest; + + internal string? StuckTest => _stuckTest; + + /// The dispatcher of the host thread (starting the thread if it is not yet). For the host's own tests. + internal Dispatcher HostDispatcher => EnsureThread(); + + internal void Run(Action body) => RunCore(() => { WithoutSyncContext(body); return Task.CompletedTask; }); + + internal T Run(Func body) + { + T result = default!; + RunCore(() => { WithoutSyncContext(() => result = body()); return Task.CompletedTask; }); + return result; + } + + /// + /// A synchronous body runs with no synchronization context, as it did on a thread of its own that never ran a dispatcher. Inside the + /// dispatcher's own callback the current context is the dispatcher's, and a body that blocks on a task (.GetAwaiter().GetResult()) + /// whose awaits capture it would wait for the very thread it is blocking. A frame the body pushes installs its own context for the + /// frame's life and restores this one, so a pumping body is unaffected. + /// + private static void WithoutSyncContext(Action body) + { + var prior = SynchronizationContext.Current; + SynchronizationContext.SetSynchronizationContext(null); + try + { + body(); + } + finally + { + SynchronizationContext.SetSynchronizationContext(prior); + } + } + + internal void Run(Func body) => RunCore(body); + + internal IDisposable EnterExclusive() + { + _gate.Wait(); + return new Releaser(_gate); + } + + /// Ends the host thread's dispatcher. Only for a host a test made for itself. + internal void Shutdown() => _dispatcher?.BeginInvokeShutdown(DispatcherPriority.Send); + + private void RunCore(Func body) + { + FailIfStuck(); + _gate.Wait(); + try + { + FailIfStuck(); + var dispatcher = EnsureThread(); + ExceptionDispatchInfo? error = null; + lock (_strays) + { + _strays.Clear(); + } + + var finished = dispatcher.InvokeAsync(() => Job(body, e => error = e)).Task.Unwrap(); + if (!finished.Wait(HangGuard)) + { + var name = CurrentTestName(); + _stuckTest = name; + throw new TimeoutException( + $"WPF test body did not return within {HangGuard.TotalSeconds:0} s: {name}. The shared STA test thread is stuck inside it, " + + "so every later WPF test in this process fails at once naming this test (#5602)."); + } + + error?.Throw(); + lock (_strays) + { + if (_strays.Count > 0) + { + var strays = _strays.ToArray(); + _strays.Clear(); + throw new AggregateException("Work the test queued on the dispatcher threw while the shared STA thread ran it (#5602):", strays); + } + } + } + finally + { + _gate.Release(); + } + } + + /// The body, then the cleanup. Never throws: the body's exception is handed back for the caller to rethrow with its stack. + private async Task Job(Func body, Action fail) + { + try + { + await body(); + } + catch (Exception ex) + { + fail(ExceptionDispatchInfo.Capture(ex)); + } + + try + { + CloseLeftoverWindows(); + await Dispatcher.Yield(DispatcherPriority.ApplicationIdle); + } + catch (Exception ex) + { + fail(ExceptionDispatchInfo.Capture(ex)); + } + } + + /// A test that failed before its own Close() leaves a window (and its HWND) behind; the next test must not meet it. + private static void CloseLeftoverWindows() + { + foreach (PresentationSource source in PresentationSource.CurrentSources) + { + if (source.RootVisual is Window window && window.Dispatcher == Dispatcher.CurrentDispatcher) + { + window.Close(); + } + } + } + + private void FailIfStuck() + { + var stuck = _stuckTest; + if (stuck is not null) + { + throw new InvalidOperationException( + $"The shared WPF STA test thread is stuck inside a body that never returned: {stuck}. This test did not run (#5602)."); + } + } + + private static string CurrentTestName() => TestContext.Current.Test?.TestDisplayName ?? "(test name unavailable)"; + + private Dispatcher EnsureThread() + { + var existing = _dispatcher; + if (existing is not null) + { + return existing; + } + + lock (_startLock) + { + if (_dispatcher is not null) + { + return _dispatcher; + } + + using var ready = new ManualResetEventSlim(); + Dispatcher? made = null; + var thread = new Thread(() => + { + var dispatcher = Dispatcher.CurrentDispatcher; + dispatcher.UnhandledException += (_, e) => + { + lock (_strays) + { + _strays.Add(e.Exception); + } + + e.Handled = true; + }; + made = dispatcher; + ready.Set(); + Dispatcher.Run(); + }) + { + Name = "StaTestThread", + /* Background: a stuck body must not keep the test process alive after the run returns. */ + IsBackground = true, + }; + thread.SetApartmentState(ApartmentState.STA); + thread.Start(); + ready.Wait(); + _dispatcher = made; + return made!; + } + } + + private sealed class Releaser(SemaphoreSlim gate) : IDisposable + { + private int _released; + + public void Dispose() + { + if (Interlocked.Exchange(ref _released, 1) == 0) + { + gate.Release(); + } + } + } +} diff --git a/Darling/Darling.Tests/StaTestThreadTests.cs b/Darling/Darling.Tests/StaTestThreadTests.cs new file mode 100644 index 0000000000..a3ac451cbd --- /dev/null +++ b/Darling/Darling.Tests/StaTestThreadTests.cs @@ -0,0 +1,195 @@ +/* + * Copyright (c) 2026 Erik Darling, Darling Data LLC + * + * This file is part of the SQL Server Performance Monitor. + * + * Licensed under the MIT License. See LICENSE file in the project root for full license information. + */ + +using System; +using System.Collections.Generic; +using System.Linq; +using System.Runtime.CompilerServices; +using System.Threading; +using System.Threading.Tasks; +using System.Windows; +using System.Windows.Threading; +using Xunit; + +namespace Darling.Tests; + +/// +/// #5602: the shared STA test thread. Bodies run in order on one thread, an exception keeps its stack, a body that pumps does not +/// let a second one in, the hang guard fires and names the test, and the drain runs the work a body queued before the next body +/// starts. The hang-guard test uses a host of its own: it leaves that host's thread stuck, which the shared one must never be. +/// In the timing collection because it judges 200 to 300 ms limits (the hang guard, a gate that must hold a body out), which a +/// parallel run's load would stretch (#5602). +/// +[Collection("timing")] +public sealed class StaTestThreadTests +{ + [Fact] + public void Bodies_RunOneAfterAnother_OnTheSameBackgroundStaThread() + { + var first = StaTestThread.Run(Describe); + var second = StaTestThread.Run(Describe); + + Assert.Equal(first.Id, second.Id); + Assert.NotEqual(Environment.CurrentManagedThreadId, first.Id); + Assert.Equal(ApartmentState.STA, first.Apartment); + Assert.True(first.Background, "the shared thread must be a background thread, or a stuck body keeps the test process alive"); + } + + [Fact] + public void ABodysException_ComesBackToTheCaller_WithItsOwnStack() + { + var thrown = Assert.Throws(() => StaTestThread.Run(() => ThrowFromNamedFrame())); + + Assert.Equal("boom from the body", thrown.Message); + Assert.Contains(nameof(ThrowFromNamedFrame), thrown.StackTrace); + + /* The thread survives the failure: the next body still runs. */ + Assert.Equal(5, StaTestThread.Run(() => 5)); + } + + [Fact] + public void ABodyThatPumps_DoesNotLetASecondBodyIn() + { + var inside = 0; + var most = 0; + var errors = new List(); + var callers = Enumerable.Range(0, 3).Select(_ => new Thread(() => + { + try + { + for (var i = 0; i < 3; i++) + { + StaTestThread.Run(() => + { + var now = Interlocked.Increment(ref inside); + InterlockedMax(ref most, now); + + /* A nested frame: while it runs, anything else queued on this dispatcher would run inside it. */ + var frame = new DispatcherFrame(); + var timer = new DispatcherTimer(TimeSpan.FromMilliseconds(15), DispatcherPriority.Normal, (_, _) => frame.Continue = false, Dispatcher.CurrentDispatcher); + timer.Start(); + Dispatcher.PushFrame(frame); + timer.Stop(); + + Interlocked.Decrement(ref inside); + }); + } + } + catch (Exception ex) + { + lock (errors) + { + errors.Add(ex); + } + } + })).ToList(); + + callers.ForEach(t => t.Start()); + callers.ForEach(t => Assert.True(t.Join(TimeSpan.FromSeconds(60)), "a caller never finished")); + + Assert.Empty(errors); + Assert.Equal(1, most); + } + + [Fact] + public void ABodyThatNeverReturns_FailsItsOwnTest_ByName_AndEveryLaterCallFailsAtOnce() + { + var host = new StaHost { HangGuard = TimeSpan.FromMilliseconds(300) }; + using var release = new ManualResetEventSlim(); + try + { + var timedOut = Assert.Throws(() => host.Run(() => release.Wait())); + Assert.Contains(nameof(ABodyThatNeverReturns_FailsItsOwnTest_ByName_AndEveryLaterCallFailsAtOnce), timedOut.Message); + Assert.Contains(nameof(ABodyThatNeverReturns_FailsItsOwnTest_ByName_AndEveryLaterCallFailsAtOnce), host.StuckTest); + + var ran = false; + var started = Environment.TickCount64; + var refused = Assert.Throws(() => host.Run(() => ran = true)); + Assert.Contains(nameof(ABodyThatNeverReturns_FailsItsOwnTest_ByName_AndEveryLaterCallFailsAtOnce), refused.Message); + Assert.False(ran); + Assert.True(Environment.TickCount64 - started < 250, "a later call must fail at once, not wait out its own guard"); + } + finally + { + release.Set(); + host.Shutdown(); + } + } + + [Fact] + public void AfterABody_TheDispatcherIsDrained_AndAWindowItLeftOpenIsClosed() + { + var queuedRan = false; + Window? left = null; + StaTestThread.Run(() => + { + Dispatcher.CurrentDispatcher.BeginInvoke(DispatcherPriority.ApplicationIdle, new Action(() => queuedRan = true)); + left = new Window { Width = 50, Height = 50, ShowInTaskbar = false }; + left.Show(); + }); + + Assert.True(queuedRan, "work the body queued must run before the next test, not during it"); + Assert.False(StaTestThread.Run(() => left!.IsVisible), "a window the body left open must be closed by the drain"); + } + + [Fact] + public void AnAsyncBody_ResumesOnTheSharedThread_AfterEachAwait() + { + var ids = new List(); + StaTestThread.Run(async () => + { + ids.Add(Environment.CurrentManagedThreadId); + await Task.Delay(10); + ids.Add(Environment.CurrentManagedThreadId); + await Dispatcher.Yield(DispatcherPriority.Background); + ids.Add(Environment.CurrentManagedThreadId); + }); + + Assert.Single(ids.Distinct()); + Assert.NotEqual(Environment.CurrentManagedThreadId, ids[0]); + } + + [Fact] + public void ExceptionFromWorkTheBodyQueued_ComesBackToTheTest_AndDoesNotEndTheThread() + { + var thrown = Assert.Throws(() => StaTestThread.Run(() => + Dispatcher.CurrentDispatcher.BeginInvoke(DispatcherPriority.Background, new Action(() => throw new FormatException("queued work failed"))))); + + Assert.Contains(thrown.InnerExceptions, e => e is FormatException { Message: "queued work failed" }); + Assert.Equal(7, StaTestThread.Run(() => 7)); + } + + [Fact] + public async Task EnterExclusive_KeepsABodyOut_UntilItIsDisposed() + { + var ran = new ManualResetEventSlim(); + Task caller; + using (StaTestThread.EnterExclusive()) + { + caller = Task.Run(() => StaTestThread.Run(() => ran.Set())); + Assert.False(ran.Wait(TimeSpan.FromMilliseconds(200)), "a body ran while a thread that owns its own thread held the gate"); + } + + await caller.WaitAsync(TimeSpan.FromSeconds(30), TestContext.Current.CancellationToken); + Assert.True(ran.IsSet); + } + + private static (int Id, ApartmentState Apartment, bool Background) Describe() => + (Environment.CurrentManagedThreadId, Thread.CurrentThread.GetApartmentState(), Thread.CurrentThread.IsBackground); + + [MethodImpl(MethodImplOptions.NoInlining)] + private static void ThrowFromNamedFrame() => throw new InvalidOperationException("boom from the body"); + + private static void InterlockedMax(ref int target, int value) + { + int seen; + while ((seen = Volatile.Read(ref target)) < value && Interlocked.CompareExchange(ref target, value, seen) != seen) + { + } + } +} diff --git a/Darling/Darling.Tests/StaThreadCensusTests.cs b/Darling/Darling.Tests/StaThreadCensusTests.cs new file mode 100644 index 0000000000..cbacbd6f44 --- /dev/null +++ b/Darling/Darling.Tests/StaThreadCensusTests.cs @@ -0,0 +1,139 @@ +/* + * Copyright (c) 2026 Erik Darling, Darling Data LLC + * + * This file is part of the SQL Server Performance Monitor. + * + * Licensed under the MIT License. See LICENSE file in the project root for full license information. + */ + +using System; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using System.Runtime.CompilerServices; +using System.Text.RegularExpressions; +using Xunit; + +namespace Darling.Tests; + +/// +/// #5602: every test that needs WPF runs its body on the one shared STA thread (StaTestThread.Run), not on a thread of its +/// own. About fifty test files used to start their own STA thread behind a gate (#5559): each left a dispatcher and a weak-event +/// table behind (#5596's 300 ms per thread at process exit), and a body that pumped could still overlap another. A new test that +/// starts its own thread, defines its own OnStaThread-style helper, or runs Dispatcher.Run() brings that back, so +/// this fails and names the file and the rule. A test that really must own its thread goes on with the +/// reason, and must take StaTestThread.EnterExclusive() around the thread so it cannot overlap a body on the shared one. +/// +[Trait("Stage", "Guard")] +public sealed class StaThreadCensusTests +{ + /* Built from parts so this file does not contain the text it scans for. */ + private static readonly string s_sta = "ApartmentState" + ".STA"; + private static readonly string s_run = "Dispatcher" + ".Run("; + private const string Exclusive = "StaTestThread.EnterExclusive()"; + + /// + /// The host and its own tests start the thread by design. Every other entry is a test that must own a thread, with the reason. + /// + private static readonly Dictionary Allowed = new(StringComparer.Ordinal) + { + ["StaTestThread.cs"] = "the host: the one place the shared STA thread is started", + ["StaTestThreadTests.cs"] = "the host's own tests assert the thread's apartment state", + ["StaThreadCensusTests.cs"] = "this census names the text it scans for", + ["WpfExitSwitchTests.cs"] = "#5598: it needs a WPF STA thread that has ENDED, to time what the process-exit handler does for it; the shared " + + "thread never ends. It takes StaTestThread.EnterExclusive() so it cannot overlap a body on the shared thread.", + }; + + /// Anti-vacuity: the suite has well over this many files that use the host, so a scan that stopped seeing them fails. + private const int HostUserFloor = 20; + + /// A method named like the per-file helpers this replaced: OnStaThread, OnStaThread<T>, RunOnStaThread, OnDispatcher. + private static readonly Regex s_helperDefinition = new( + @"\bstatic\s+[\w<>\[\]?,\.\s()]+?\b(?:Run)?On(?:Sta(?:Thread)?|Dispatcher|UiThread)\s*(?:<\w+>)?\s*\(", RegexOptions.Compiled); + + [Fact] + public void NoTestFileOutsideTheAllowList_StartsItsOwnStaThread_DefinesAnStaHelper_OrRunsADispatcher() + { + var violations = new List(); + var hostUsers = 0; + foreach (var (rel, text) in TestFiles()) + { + if (text.Contains("StaTestThread.Run", StringComparison.Ordinal)) + { + hostUsers++; + } + + if (Allowed.ContainsKey(Path.GetFileName(rel))) + { + continue; + } + + /* Code only: a comment or a string that names the old helper, the dispatcher or the apartment state is not a use of it. */ + var code = CSharpSourceWalker.StripCommentsAndStrings(text); + if (code.Contains(s_sta, StringComparison.Ordinal)) + { + violations.Add(rel + ": starts its own STA thread; run the body in StaTestThread.Run(...) instead"); + } + + if (s_helperDefinition.IsMatch(code)) + { + violations.Add(rel + ": defines its own OnStaThread-style helper; call StaTestThread.Run(...) directly"); + } + + if (code.Contains(s_run, StringComparison.Ordinal)) + { + violations.Add(rel + ": runs its own dispatcher; use the async overload StaTestThread.Run(async () => ...)"); + } + } + + Assert.True( + violations.Count == 0, + "WPF tests share ONE STA thread per test process (#5602). Fix, or add the file to StaThreadCensusTests.Allowed with the reason it must " + + "own a thread:" + Environment.NewLine + string.Join(Environment.NewLine, violations)); + Assert.True(hostUsers >= HostUserFloor, $"only {hostUsers} test files use StaTestThread.Run; the scan has stopped seeing them"); + } + + [Fact] + public void ATestThatOwnsItsThread_TakesTheExclusiveGateAroundIt() + { + var missing = new List(); + foreach (var (rel, text) in TestFiles()) + { + var name = Path.GetFileName(rel); + if (Allowed.ContainsKey(name) && name is not ("StaTestThread.cs" or "StaTestThreadTests.cs" or "StaThreadCensusTests.cs") + && !text.Contains(Exclusive, StringComparison.Ordinal)) + { + missing.Add(rel); + } + } + + Assert.True( + missing.Count == 0, + "These tests own their STA thread but do not call " + Exclusive + " first, so they can overlap a body on the shared thread (#5559's race): " + + string.Join(", ", missing)); + } + + [Fact] + public void EveryAllowListEntry_SaysWhy() + { + Assert.All(Allowed, entry => Assert.True(entry.Value.Length >= 20, entry.Key + " is on the allow list without a reason")); + } + + private static IEnumerable<(string Rel, string Text)> TestFiles() + { + var dir = ProjectDir(); + foreach (var file in Directory.EnumerateFiles(dir, "*.cs", SearchOption.AllDirectories)) + { + var rel = Path.GetRelativePath(dir, file); + if (rel.StartsWith("bin" + Path.DirectorySeparatorChar, StringComparison.Ordinal) + || rel.StartsWith("obj" + Path.DirectorySeparatorChar, StringComparison.Ordinal)) + { + continue; + } + + yield return (rel, File.ReadAllText(file)); + } + } + + private static string ProjectDir([CallerFilePath] string thisFile = "") => Path.GetDirectoryName(thisFile)!; +} diff --git a/Darling/Darling.Tests/StatementFilterAlertTests.cs b/Darling/Darling.Tests/StatementFilterAlertTests.cs index d4f810f5d3..6c0e41e08f 100644 --- a/Darling/Darling.Tests/StatementFilterAlertTests.cs +++ b/Darling/Darling.Tests/StatementFilterAlertTests.cs @@ -25,6 +25,7 @@ namespace Darling.Tests; /// record it. These pins run the real engine against a fake deliverer, so an alert that carries a statement or a /// report in its details, its attachments or its short message reaches the deliverer withheld. /// +[Collection("timing")] public sealed class StatementFilterAlertTests { private const string Marker = SensitiveStatements.PlaceholderText; diff --git a/Darling/Darling.Tests/StatementFilterWarmUpWaitTests.cs b/Darling/Darling.Tests/StatementFilterWarmUpWaitTests.cs index 6aff5b6a90..c9f8665d15 100644 --- a/Darling/Darling.Tests/StatementFilterWarmUpWaitTests.cs +++ b/Darling/Darling.Tests/StatementFilterWarmUpWaitTests.cs @@ -19,21 +19,12 @@ namespace Darling.Tests; -/// -/// Serializes the classes that stage a fake start-up warm-up (#5484). The registered warm-up is process-wide, so a -/// fake that is still running while another class judges a large document would hold that class up for the wait. -/// -[CollectionDefinition("statement-filter-warm-up", DisableParallelization = true)] -public sealed class StatementFilterWarmUpCollection -{ -} - /// /// #5478, #5484: a call that judges a large document waits for a running start-up warm-up, but only until one deadline /// (3 s after the warm-up started), only when it judges at least 256 KB, and the alert engine's fire path awaits instead /// of blocking. The MCP and web sweep waits the same way. /// -[Collection("statement-filter-warm-up")] +[Collection("timing")] public sealed class StatementFilterWarmUpWaitTests { private static long Ticks(TimeSpan span) => (long)(span.TotalSeconds * Stopwatch.Frequency); diff --git a/Darling/Darling.Tests/StatementFilterWebTests.cs b/Darling/Darling.Tests/StatementFilterWebTests.cs index f153b7dd5f..dca3a93ebb 100644 --- a/Darling/Darling.Tests/StatementFilterWebTests.cs +++ b/Darling/Darling.Tests/StatementFilterWebTests.cs @@ -33,6 +33,7 @@ namespace Darling.Tests; /// web path) and reads back what a browser would get. The tests are RED on a writer with no sweep, because the canary /// then comes through. /// +[Collection("timing")] public sealed class StatementFilterWebTests { private static readonly string Canary = StatementScrubCanary.CanaryStatement; diff --git a/Darling/Darling.Tests/StatementScrubViewerPlanWithheldTests.cs b/Darling/Darling.Tests/StatementScrubViewerPlanWithheldTests.cs index eb9470d889..955ff4d97d 100644 --- a/Darling/Darling.Tests/StatementScrubViewerPlanWithheldTests.cs +++ b/Darling/Darling.Tests/StatementScrubViewerPlanWithheldTests.cs @@ -32,7 +32,7 @@ public sealed class StatementScrubViewerPlanWithheldTests [Fact] public void APlanWithheldWhole_ShowsTheWithheldSentenceInTheViewer_NotAParseError() { - OnStaThread(() => + StaTestThread.Run(() => { var control = new PlanViewerControl(); try @@ -150,22 +150,4 @@ public void SaveXmlToFile_RefusesAWithheldReportBeforeTheSaveDialog_AndTheViewer Assert.DoesNotContain("File.WriteAllText", export, StringComparison.Ordinal); } - private static void OnStaThread(Action body) - { - Exception? error = null; - using var staGate = WpfStaGate.Enter(); - var thread = new Thread(() => - { - try { body(); } - catch (Exception ex) { error = ex; } - }); - thread.SetApartmentState(ApartmentState.STA); - thread.Start(); - thread.Join(); - - if (error is not null) - { - throw error; - } - } } diff --git a/Darling/Darling.Tests/StoreCopyPhaseLivePostgresTests.cs b/Darling/Darling.Tests/StoreCopyPhaseLivePostgresTests.cs index f2aa211557..6a40569577 100644 --- a/Darling/Darling.Tests/StoreCopyPhaseLivePostgresTests.cs +++ b/Darling/Darling.Tests/StoreCopyPhaseLivePostgresTests.cs @@ -229,8 +229,8 @@ fault the runner itself raised. */ /// deadline is a bare carrying /// . Remove the bound and this test does not fail /// slowly or ambiguously: it waits for the connection's timeout and then fails on the type. The - /// elapsed range is asserted on both sides as well, so a deadline accidentally written in - /// milliseconds cannot satisfy the type and message alone. + /// elapsed floor is asserted as well, so a deadline accidentally written in + /// milliseconds cannot satisfy the type and message alone (#5602: the ceiling is the type and message). /// /// It really does cost the sweep deadline in wall clock, and that is the price of driving /// the SHIPPED private body rather than a copy: the production call site takes @@ -295,13 +295,17 @@ arrives as an NpgsqlException wrapping one of these and would satisfy an assigna from the re-attempt gate, and would be reported as an orderly stop. */ Assert.IsNotAssignableFrom(fault); - /* It fired on its own clock rather than the connection's. Both sides: the upper bound is what says - Npgsql's timeout did not do this, and the lower bound is what a deadline written in - milliseconds would fail. */ - Assert.InRange( - clock.Elapsed, - StoreCopyStartDeadline.Deadline - TimeSpan.FromSeconds(1), - TimeSpan.FromSeconds(commandTimeoutSeconds)); + /* It fired on its own clock rather than the connection's. #5602: the LOWER bound only, as a clock check. + What a deadline written in milliseconds would fail is this floor, and load can only lengthen an + elapsed time, never shorten it, so it cannot flake. The upper bound this used to carry (the + connection's command timeout) is now held by the work: Npgsql's own timeout arrives as an + NpgsqlException wrapping a TimeoutException, which the exact-type and exact-message assertions above + already reject, so a late-firing deadline on a loaded runner no longer fails and the bug the upper + bound was written for, "the connection's bound ended the stall", still does. */ + Assert.True( + clock.Elapsed >= StoreCopyStartDeadline.Deadline - TimeSpan.FromSeconds(1), + $"The start phase gave up after {clock.Elapsed.TotalSeconds:F1}s, before the sweep deadline " + + $"({StoreCopyStartDeadline.Deadline.TotalSeconds:F0}s) less a second: the deadline is not the sweep deadline."); /* And the consequences the phase axis carries, on this fault: it is a transport fault, it is start-phase, so the gate accepts it — a re-attempt after a start-phase stall is exactly-once and diff --git a/Darling/Darling.Tests/StoreServerPasswordRulesLiveTests.cs b/Darling/Darling.Tests/StoreServerPasswordRulesLiveTests.cs index 7622652861..59a41d5604 100644 --- a/Darling/Darling.Tests/StoreServerPasswordRulesLiveTests.cs +++ b/Darling/Darling.Tests/StoreServerPasswordRulesLiveTests.cs @@ -17,8 +17,8 @@ namespace Darling.Tests; /* #1776 own-store: this class mints its own scratch database through ScratchPostgres and touches nothing on the - shared one. The managed role names are cluster-wide, so it shares the live-postgres collection with the other - classes that provision them, and it skips when a set of those names already exists on the rig. */ + shared one. The managed role names are cluster-wide, so it runs in the pg-cluster-roles collection (#5602), alone, + with the other classes that provision them, and it skips when a set of those names already exists on the rig. */ /// /// The store's own password rules, as the roles that meet them. The shipped provisioning batch runs whole on a scratch @@ -27,7 +27,7 @@ namespace Darling.Tests; /// to neither, so a configuration-file seed and --add-server keep working, and a row already in the table is never /// touched. Each scenario runs against the managed batch's text and against the self-managed script's. /// -[Collection("live-postgres")] +[Collection("pg-cluster-roles")] public sealed class StoreServerPasswordRulesLiveTests { private const string ReferenceSentence = "Enter the password itself. References (env: or file:) can only be set in the configuration file."; diff --git a/Darling/Darling.Tests/StoreStatementStatsLiveTests.cs b/Darling/Darling.Tests/StoreStatementStatsLiveTests.cs index 56946e81a9..bed04ad82a 100644 --- a/Darling/Darling.Tests/StoreStatementStatsLiveTests.cs +++ b/Darling/Darling.Tests/StoreStatementStatsLiveTests.cs @@ -54,6 +54,13 @@ public sealed class StoreStatementStatsLiveTests private const string PlainOwnerPassword = "PlainOwnerLiteral3899"; private const string ScrubPassword = "ScrubLiteral3899"; + /* #5602: roles and databases are cluster-wide, so each run names its own and no other test can see them. */ + private static readonly string RunSuffix = Guid.NewGuid().ToString("N")[..8]; + private static readonly string Reader = "pgss_reader_" + RunSuffix; + private static readonly string Outsider = "pgss_outsider_" + RunSuffix; + private static readonly string PlainOwner = "pgss_plainowner_" + RunSuffix; + private static readonly string PlainDb = "pgss_plain_" + RunSuffix; + [Fact] public async Task OnTheProductsOwnConf_StatementsRankByRole_AndARolePasswordNeverReachesTheView_Gated() { @@ -88,22 +95,22 @@ tracking off. */ Assert.Equal("off", await ScalarAsync(c, "SHOW pg_stat_statements.track_utility", ct)); await ExecAsync(c, "CREATE SCHEMA IF NOT EXISTS config", ct); - await ExecAsync(c, $"CREATE ROLE pgss_reader LOGIN PASSWORD '{ReaderPassword}'", ct); - await ExecAsync(c, $"CREATE ROLE pgss_outsider LOGIN PASSWORD '{OutsiderPassword}'", ct); + await ExecAsync(c, $"CREATE ROLE {Reader} LOGIN PASSWORD '{ReaderPassword}'", ct); + await ExecAsync(c, $"CREATE ROLE {Outsider} LOGIN PASSWORD '{OutsiderPassword}'", ct); /* Schema USAGE for both, as provisioning gives the product's roles: the outsider's refusal below must come from the missing EXECUTE grant, not from the schema. */ - await ExecAsync(c, "GRANT USAGE ON SCHEMA config TO pgss_reader, pgss_outsider", ct); + await ExecAsync(c, $"GRANT USAGE ON SCHEMA config TO {Reader}, {Outsider}", ct); Assert.Equal( StoreStatementStats.SetupOutcome.Ready, - await StoreStatementStats.EnsureAsync(c, "config", ["pgss_reader", "pgss_absent"], NullLogger.Instance, ct)); + await StoreStatementStats.EnsureAsync(c, "config", [Reader, "pgss_absent"], NullLogger.Instance, ct)); /* Both CREATE ROLEs above carried a password literal, and neither was recorded. */ Assert.Equal(0L, await CountRecordedAsync(c, "Literal3899", ct)); } - var readerCs = new NpgsqlConnectionStringBuilder(ownerCs.ConnectionString) { Username = "pgss_reader", Password = ReaderPassword }; + var readerCs = new NpgsqlConnectionStringBuilder(ownerCs.ConnectionString) { Username = Reader, Password = ReaderPassword }; await using var reader = NpgsqlDataSource.Create(readerCs.ConnectionString); await using (var r = await reader.OpenConnectionAsync(ct)) { @@ -121,14 +128,14 @@ tracking off. */ var root0 = doc.RootElement; var statements = root0.GetProperty("statements").EnumerateArray().ToList(); var probe = Assert.Single(statements, s => QueryOf(s).Contains("pgss_probe_marker", StringComparison.Ordinal)); - Assert.Equal("pgss_reader", probe.GetProperty("role").GetString()); + Assert.Equal(Reader, probe.GetProperty("role").GetString()); Assert.Equal(3L, probe.GetProperty("calls").GetInt64()); Assert.Contains("$1 AS pgss_probe_marker", QueryOf(probe), StringComparison.Ordinal); Assert.Equal(JsonValueKind.String, probe.GetProperty("query_id").ValueKind); Assert.True(long.TryParse(probe.GetProperty("query_id").GetString(), out _)); Assert.DoesNotContain(statements, s => QueryOf(s).Contains("Literal3899", StringComparison.Ordinal)); Assert.Contains(root0.GetProperty("by_role").EnumerateArray(), row => row.GetProperty("role").GetString() == "owner"); - Assert.Contains(root0.GetProperty("by_role").EnumerateArray(), row => row.GetProperty("role").GetString() == "pgss_reader"); + Assert.Contains(root0.GetProperty("by_role").EnumerateArray(), row => row.GetProperty("role").GetString() == Reader); Assert.False(string.IsNullOrEmpty(root0.GetProperty("stats_since").GetString())); Assert.False(root0.GetProperty("utility_statements_tracked").GetBoolean()); Assert.False(root0.GetProperty("connected_as_owner").GetBoolean()); @@ -146,7 +153,7 @@ tracking off. */ } /* A role outside the grant is told what it lacks, from the catalog, not handed an error. */ - var outsiderCs = new NpgsqlConnectionStringBuilder(ownerCs.ConnectionString) { Username = "pgss_outsider", Password = OutsiderPassword }; + var outsiderCs = new NpgsqlConnectionStringBuilder(ownerCs.ConnectionString) { Username = Outsider, Password = OutsiderPassword }; await using (var outsider = NpgsqlDataSource.Create(outsiderCs.ConnectionString)) { var denied = await DarlingMcpStoreQueryStatsTools.GetStoreQueryStats(outsider); @@ -187,7 +194,7 @@ await DarlingMcpStoreQueryStatsTools.GetStoreQueryStats(reader, top: 1000), await using (var c = await OpenAsync(ownerCs.ConnectionString, ct)) { await ExecAsync(c, "SET pg_stat_statements.track_utility = on", ct); - await ExecAsync(c, $"ALTER ROLE pgss_outsider PASSWORD '{ScrubPassword}'", ct); + await ExecAsync(c, $"ALTER ROLE {Outsider} PASSWORD '{ScrubPassword}'", ct); await ExecAsync(c, "RESET pg_stat_statements.track_utility", ct); Assert.Equal(1L, await CountRecordedAsync(c, ScrubPassword, ct)); } @@ -226,11 +233,11 @@ await DarlingMcpStoreQueryStatsTools.GetStoreQueryStats(reader, top: 1000, full_ await ExecAsync(c, "SET pg_stat_statements.track_utility = on", ct); Assert.Equal( StoreStatementStats.SetupOutcome.Ready, - await StoreStatementStats.EnsureAsync(c, "config", ["pgss_reader"], firstPass, ct)); + await StoreStatementStats.EnsureAsync(c, "config", [Reader], firstPass, ct)); Assert.Equal(0L, await CountRecordedAsync(c, ScrubPassword, ct)); Assert.Equal( StoreStatementStats.SetupOutcome.Ready, - await StoreStatementStats.EnsureAsync(c, "config", ["pgss_reader"], secondPass, ct)); + await StoreStatementStats.EnsureAsync(c, "config", [Reader], secondPass, ct)); } Assert.Contains("Warning: Statement statistics: pg_stat_statements.track_utility is on", firstPass.Joined, StringComparison.Ordinal); @@ -244,29 +251,29 @@ await DarlingMcpStoreQueryStatsTools.GetStoreQueryStats(reader, top: 1000, full_ and what fixes it, until pg_read_all_stats is granted. */ await using (var c = await OpenAsync(ownerCs.ConnectionString, ct)) { - await ExecAsync(c, $"CREATE ROLE pgss_plainowner LOGIN NOSUPERUSER PASSWORD '{PlainOwnerPassword}'", ct); - await ExecAsync(c, "CREATE DATABASE pgss_plain OWNER pgss_plainowner", ct); + await ExecAsync(c, $"CREATE ROLE {PlainOwner} LOGIN NOSUPERUSER PASSWORD '{PlainOwnerPassword}'", ct); + await ExecAsync(c, $"CREATE DATABASE {PlainDb} OWNER {PlainOwner}", ct); } - var plainSuperCs = new NpgsqlConnectionStringBuilder(ownerCs.ConnectionString) { Database = "pgss_plain" }; + var plainSuperCs = new NpgsqlConnectionStringBuilder(ownerCs.ConnectionString) { Database = PlainDb }; await using (var c = await OpenAsync(plainSuperCs.ConnectionString, ct)) { await ExecAsync(c, "CREATE EXTENSION pg_stat_statements", ct); - await ExecAsync(c, "CREATE SCHEMA config AUTHORIZATION pgss_plainowner", ct); - await ExecAsync(c, "GRANT USAGE ON SCHEMA config TO pgss_reader", ct); + await ExecAsync(c, $"CREATE SCHEMA config AUTHORIZATION {PlainOwner}", ct); + await ExecAsync(c, $"GRANT USAGE ON SCHEMA config TO {Reader}", ct); await ScalarAsync(c, "SELECT 3899 AS pgss_hidden_marker", ct); } - var plainOwnerCs = new NpgsqlConnectionStringBuilder(plainSuperCs.ConnectionString) { Username = "pgss_plainowner", Password = PlainOwnerPassword }; + var plainOwnerCs = new NpgsqlConnectionStringBuilder(plainSuperCs.ConnectionString) { Username = PlainOwner, Password = PlainOwnerPassword }; await using (var c = await OpenAsync(plainOwnerCs.ConnectionString, ct)) { Assert.False(await ScalarAsync(c, "SELECT pg_catalog.pg_has_role(current_user, 'pg_read_all_settings', 'USAGE')", ct)); Assert.Equal( StoreStatementStats.SetupOutcome.Ready, - await StoreStatementStats.EnsureAsync(c, "config", ["pgss_reader"], NullLogger.Instance, ct)); + await StoreStatementStats.EnsureAsync(c, "config", [Reader], NullLogger.Instance, ct)); } - var plainReaderCs = new NpgsqlConnectionStringBuilder(readerCs.ConnectionString) { Database = "pgss_plain" }; + var plainReaderCs = new NpgsqlConnectionStringBuilder(readerCs.ConnectionString) { Database = PlainDb }; await using (var plainReader = NpgsqlDataSource.Create(plainReaderCs.ConnectionString)) { using (var doc = JsonDocument.Parse(await DarlingMcpStoreQueryStatsTools.GetStoreQueryStats(plainReader, top: 1000, full_text: true))) @@ -277,7 +284,7 @@ await DarlingMcpStoreQueryStatsTools.GetStoreQueryStats(reader, top: 1000, full_ await using (var c = await OpenAsync(ownerCs.ConnectionString, ct)) { - await ExecAsync(c, "GRANT pg_read_all_stats TO pgss_plainowner", ct); + await ExecAsync(c, $"GRANT pg_read_all_stats TO {PlainOwner}", ct); } using (var doc = JsonDocument.Parse(await DarlingMcpStoreQueryStatsTools.GetStoreQueryStats(plainReader, top: 1000, full_text: true))) diff --git a/Darling/Darling.Tests/TimeRangePickerControlTests.cs b/Darling/Darling.Tests/TimeRangePickerControlTests.cs index bb2efac59d..5666656333 100644 --- a/Darling/Darling.Tests/TimeRangePickerControlTests.cs +++ b/Darling/Darling.Tests/TimeRangePickerControlTests.cs @@ -29,7 +29,7 @@ private static TimeRangePicker Build() [Fact] public void Constructs_WithTheShortPresetsAndTheCalendarPeriods() { - OnStaThread(() => + StaTestThread.Run(() => { var picker = Build(); Assert.Equal(9, picker.RollingPanel.Children.Count); @@ -44,7 +44,7 @@ public void Constructs_WithTheShortPresetsAndTheCalendarPeriods() [Fact] public void Value_RendersWithoutRaisingTheEvent_AndSelectRaisesIt() { - OnStaThread(() => + StaTestThread.Run(() => { var picker = Build(); var raised = 0; @@ -67,7 +67,7 @@ public void Value_RendersWithoutRaisingTheEvent_AndSelectRaisesIt() [Fact] public void Select_RefusesARangeUnderTheMinimum_AndHoldsTheOldOne() { - OnStaThread(() => + StaTestThread.Run(() => { var picker = Build(); picker.NowProvider = () => new DateTime(2026, 10, 8, 4, 2, 0, DateTimeKind.Unspecified); @@ -88,7 +88,7 @@ public void Select_RefusesARangeUnderTheMinimum_AndHoldsTheOldOne() [Fact] public void Resolve_FollowsTheZoneProvider() { - OnStaThread(() => + StaTestThread.Run(() => { var picker = Build(); picker.Value = TimeRangeSpec.ForPeriod(CalendarPeriod.Yesterday); @@ -104,7 +104,7 @@ public void Resolve_FollowsTheZoneProvider() [Fact] public void Notes_ShowTheDataStartAndTheCollectorInterval() { - OnStaThread(() => + StaTestThread.Run(() => { var picker = Build(); picker.Value = TimeRangePresets.Find("1w")!; @@ -127,7 +127,7 @@ public void Notes_ShowTheDataStartAndTheCollectorInterval() [Fact] public void Compact_HidesTheDetailBesideTheButton_AndKeepsItInTheTooltip() { - OnStaThread(() => + StaTestThread.Run(() => { var picker = Build(); picker.Compact = true; @@ -141,7 +141,7 @@ public void Compact_HidesTheDetailBesideTheButton_AndKeepsItInTheTooltip() [Fact] public void TypingARange_PreviewsItBeforeApply_AndAnErrorDisablesApply() { - OnStaThread(() => + StaTestThread.Run(() => { var picker = Build(); picker.InputBox.Text = "last month"; @@ -157,7 +157,7 @@ public void TypingARange_PreviewsItBeforeApply_AndAnErrorDisablesApply() [Fact] public void OpeningThePopup_ThemesTheCalendarFromTheHostBrushes_AndAPickedRangeFillsTheTextBox() { - OnStaThread(() => + StaTestThread.Run(() => { var dark = new SolidColorBrush(Color.FromRgb(0x11, 0x12, 0x17)); var light = new SolidColorBrush(Color.FromRgb(0xE4, 0xE6, 0xEB)); @@ -196,24 +196,4 @@ public void OpeningThePopup_ThemesTheCalendarFromTheHostBrushes_AndAPickedRangeF } }); } - - /// WPF objects require STA; same shape as AvailabilityGroupsTabRefreshTests. - private static void OnStaThread(Action body) - { - Exception? error = null; - using var staGate = WpfStaGate.Enter(); - var thread = new Thread(() => - { - try { body(); } - catch (Exception ex) { error = ex; } - }); - thread.SetApartmentState(ApartmentState.STA); - thread.Start(); - thread.Join(); - - if (error is not null) - { - throw error; - } - } } diff --git a/Darling/Darling.Tests/TimingCollection.cs b/Darling/Darling.Tests/TimingCollection.cs new file mode 100644 index 0000000000..989f86a08b --- /dev/null +++ b/Darling/Darling.Tests/TimingCollection.cs @@ -0,0 +1,32 @@ +/* + * Copyright (c) 2026 Erik Darling, Darling Data LLC + * + * This file is part of the SQL Server Performance Monitor. + * + * Licensed under the MIT License. See LICENSE file in the project root for full license information. + */ + +using Xunit; + +namespace Darling.Tests; + +/// +/// The one collection for every test class that times its own work and judges the time against a limit (#5602). +/// xunit runs a collection on its own, after the +/// parallel classes have finished, so the timed calls get a quiet process instead of sharing CPU with the other +/// 17,000 tests (#4788: the setting redactor's scaling check; #5484: the statement filter's warm-up waits; #5561: a +/// timing test on a 27 MB plan; #5600: the cloud identity probe's step limit). The bars stay where they were: a +/// real regression still fails them, and a busy runner no longer does. +/// +/// This collection replaced pg-setting-redactor-timing and statement-filter-warm-up. The warm-up +/// wait tests stage a process-wide fake warm-up; running them in this collection keeps them apart from every other +/// class too, which is all their own collection did. +/// +/// A class that times work and cannot join it (it needs the live-postgres collection's shared store) or +/// does not judge the time (a hang guard, a diagnostic) is named on the allow list of +/// with its reason. +/// +[CollectionDefinition("timing", DisableParallelization = true)] +public sealed class TimingCollection +{ +} diff --git a/Darling/Darling.Tests/TimingTestCensusTests.cs b/Darling/Darling.Tests/TimingTestCensusTests.cs new file mode 100644 index 0000000000..425f6029db --- /dev/null +++ b/Darling/Darling.Tests/TimingTestCensusTests.cs @@ -0,0 +1,168 @@ +/* + * Copyright (c) 2026 Erik Darling, Darling Data LLC + * + * This file is part of the SQL Server Performance Monitor. + * + * Licensed under the MIT License. See LICENSE file in the project root for full license information. + */ + +using System; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using System.Reflection; +using System.Runtime.CompilerServices; +using Xunit; + +namespace Darling.Tests; + +/// +/// A Darling.Tests class that starts a clock runs in the timing collection or is on the allow list with its +/// reason (#5602). xunit runs test classes in parallel, so a class that times its work and fails past a limit is +/// judging the whole runner's load: #5561 (a 27 MB plan) and #5600 (the cloud probe's step limit, 1083 ms against a +/// 1000 ms bar in the whole-suite guards job) were that. is declared with +/// DisableParallelization, so xunit runs it alone, after the parallel classes. +/// +/// "Starts a clock" is 's rule: Stopwatch.StartNew, GetTimestamp, +/// GetElapsedTime, new Stopwatch, a DateTime.UtcNow subtraction or Environment.TickCount in +/// code (comments and string literals do not count). That is wider than "asserts on the elapsed time", so a class that +/// only reports a time is named too, and one more assertion cannot slip in unseen. The allow list is per file; every +/// entry carries its reason, and an entry whose file no longer starts a clock outside the collection fails, so the +/// list only shrinks. +/// +[Trait("Stage", "Guard")] +public sealed class TimingTestCensusTests +{ + private const string HangGuard = "hang guard: the bound is a generous ceiling that only stops a stuck test, so load cannot trip it and a regression is not what it measures"; + + private const string PollLoop = "poll loop: the clock only bounds a wait for a state that the test then asserts on; no assertion is made on the time"; + + private const string Diagnostic = "diagnostic: the time is written to the test output and nothing asserts on it"; + + private const string LiveStore = "live-postgres member (#5602): it needs that collection's shared migrated store, and a class is in one collection only. The bound is wide (a server-side timeout or a hang guard of 10 s and up, against work that takes milliseconds), so runner load does not trip it. It stays: a second copy of the shared-store fixture in the timing collection would read the process-wide LiveCleanupBatch ledger again and report every live class's residue twice"; + + /// Anti-vacuity: the timing collection holds well over this many classes, so a scan that stopped seeing it fails. + private const int TimingMemberFloor = 15; + + private static readonly Dictionary s_allowed = new(StringComparer.Ordinal) + { + ["CaggGroupIndexDropUpgradeLiveTests.cs"] = "lower bound only: it asserts the migration waited out a ~8 s lock hold (elapsed >= 7 s); load makes the wait longer, never shorter", + ["CollectionLogDrainForensicsStoreTests.cs"] = "the clock is handed to a counting reader under test; the assertions are on the readings it records (rows, bytes, a last-read stamp >= 0), with no upper bound on any time", + ["CollectionLogSegmentByLiveTests.cs"] = LiveStore + "; the 30 s ceiling is a hang guard on a lock wait", + ["ComposeTimeoutPhaseLiveTests.cs"] = HangGuard + " (60 s against a 5 s compose timeout)", + ["DarlingManagedPostgresTests.cs"] = PollLoop + " (a retry of a file operation the OS may still hold)", + ["MoveDirectoryOnceReleasedTests.cs"] = "lower bound only: it asserts the retry waited its patience before giving up; load makes the wait longer, never shorter", + ["PerDatabaseFaultPathSplitTests.cs"] = "the clock is a running slice the code under test reads; the test spins until it passes a minimum and asserts it keeps running and freezes when stopped, with no upper bound", + ["PgWaitSamplerLiveTests.cs"] = LiveStore + "; the bound is the sampling window plus 30 s, with the window as the lower bound", + ["PlanRegressionDailyTriggerCostRig.cs"] = Diagnostic + " (a cost rig, never asserted)", + ["ProvisioningSerializationTests.cs"] = "lower bounds on a deliberate wait plus ceilings of 20 s and 90 s over it, which are hang guards; the same file also holds a live-postgres member", + ["QueryStoreBackfillCutChunkLiveTests.cs"] = Diagnostic, + ["QueryStoreIntervalConvergenceLiveTests.cs"] = Diagnostic + " (each step's duration is recorded for the failure message)", + ["QueryStoreIntervalPurgeRowCappedTests.cs"] = HangGuard + " (120 s for a purge drain)", + ["RdsLogStateConcurrencyTests.cs"] = "the clock sets how long the test hammers the code (rounds for one second); no assertion is made on a time", + ["ReadScopeStatementCaptureLiveTests.cs"] = "live-postgres member; lower bound only (the recorded duration must reach 550 ms after a 600 ms delay)", + ["RollupBackfillLiveTests.cs"] = PollLoop + " (a held-chunk wait with a limit)", + ["SensitiveStatementJsonTests.cs"] = Diagnostic + " (median of 20 identity walks)", + ["SensitiveStatementOutputFilterTests.cs"] = "every time bound goes through TimingClaim.AtMost, which asserts only when DARLING_TIMING_TESTS=1 and otherwise records the reading; the best of three is what it judges", + ["SensitiveStatementSessionTests.cs"] = Diagnostic + " (throughput records)", + ["SensitiveStatementXmlTests.cs"] = Diagnostic + " (best-of-seven records; the only assertion is that the pass took time)", + ["SensitiveStatementsTests.cs"] = "time bounds go through TimingClaim.AtMost (asserts only when DARLING_TIMING_TESTS=1, best of several runs); the real assertions are lower bounds (a budgeted judge ran at least 60% of its limit) and fake-clock arithmetic", + ["StatementFilterBlockingReadsLiveTests.cs"] = LiveStore + "; the 10 s bound is a hang guard on a filter that judges 1 MB in tens of milliseconds", + ["StatementFilterPlanReadsLiveTests.cs"] = LiveStore + "; the 15 s bound is a hang guard on a filter that judges a plan in tens of milliseconds", + ["StatementScrubBlockingTests.cs"] = Diagnostic + " (scrub throughput records)", + ["StatementScrubSnapshotTests.cs"] = HangGuard + " (2 minutes for a worst-case snapshot read)", + ["StoreCopyPhaseLivePostgresTests.cs"] = "live-postgres member; lower bound only (the start phase must not give up before the sweep deadline less a second); the upper bound moved to the exact fault type and message asserts, which tell the deadline from Npgsql's own timeout (#5602)", + ["TrendStaleStatsLiveTests.cs"] = Diagnostic + " (the assertions are on the answer status)", + ["TuningDropLockRecoveryLiveTests.cs"] = HangGuard + " (60 s against a single blocked drop)", + ["WebExceptionTextCensusTests.cs"] = "the stopwatch's reading is passed as the elapsed-milliseconds argument of a fake tool's timeout result; no assertion is made on a time", + ["TuningReaderBudgetLiveTests.cs"] = LiveStore + "; the bound is half of each reader's 15 s or 30 s command deadline", + }; + + [Fact] + public void TheDetector_SeesAClockAndTheCollection_AndIgnoresProseAndStrings() + { + const string text = "namespace N;\n\n" + + "[Collection(\"timing\")]\npublic sealed class Timed\n{\n void A() { var w = Stopwatch.StartNew(); }\n}\n\n" + + "public sealed class Bare\n{\n void B() { long t = Stopwatch.GetTimestamp(); }\n}\n\n" + + "[Trait(\"Stage\", \"Guard\")]\n[Collection(\"timing\")]\npublic sealed class TwoAttributes\n{\n void C() { var d = DateTime.UtcNow - start; }\n}\n\n" + + "public sealed class Prose\n{\n // var w = Stopwatch.StartNew();\n /* new Stopwatch() */\n" + + " void D() { Assert.Contains(\"Stopwatch.GetTimestamp()\", source); var s = @\"a \"\"Stopwatch.StartNew()\"\" b\"; }\n" + + " /// Environment.TickCount\n void E() { }\n}\n\n" + + "public sealed class Interpolated\n{\n void F() => Log($\"x {Stopwatch.StartNew()}\");\n}\n"; + + var rows = TimingTestScanner.ScanText("t.cs", text).ToDictionary(r => r.Name); + + Assert.True(rows["Timed"].StartsClock); + Assert.True(rows["Timed"].InTimingCollection); + Assert.True(rows["Bare"].StartsClock); + Assert.False(rows["Bare"].InTimingCollection); + Assert.True(rows["TwoAttributes"].StartsClock); + Assert.True(rows["TwoAttributes"].InTimingCollection); + Assert.False(rows["Prose"].StartsClock); + Assert.False(rows["Interpolated"].StartsClock); + } + + [Fact] + public void TheDetector_ReportsAClockOutsideTheCollection_AndAStaleAllowEntry() + { + var rows = TimingTestScanner.ScanText( + "Bare.cs", + "public sealed class Bare\n{\n void B() { var w = new Stopwatch(); }\n}\n"); + + var problems = TimingTestScanner.Problems(rows, new Dictionary()); + Assert.Single(problems); + Assert.Contains("Bare.cs", problems[0], StringComparison.Ordinal); + + Assert.Empty(TimingTestScanner.Problems(rows, new Dictionary { ["Bare.cs"] = "reason" })); + Assert.Equal(new[] { "Gone.cs" }, TimingTestScanner.Stale(rows, new Dictionary { ["Bare.cs"] = "reason", ["Gone.cs"] = "reason" })); + } + + [Fact] + public void EveryClassThatStartsAClock_RunsInTheTimingCollection_OrIsAllowedWithAReason() + { + var rows = TimingTestScanner.Scan(TestsRoot()); + + var problems = TimingTestScanner.Problems(rows, s_allowed); + Assert.True( + problems.Count == 0, + "A test class starts a clock outside the `timing` collection (#5602). xunit runs classes in parallel, so a time limit " + + "judged there is judged against the runner's load. Add [Collection(\"timing\")] above the class (TimingCollection runs " + + "alone, after the parallel classes), assert on the work instead of the clock, or, for a hang guard, a poll loop or a " + + "diagnostic, name the file in TimingTestCensusTests.s_allowed with the reason:\n " + string.Join("\n ", problems)); + + var stale = TimingTestScanner.Stale(rows, s_allowed); + Assert.True( + stale.Count == 0, + "These allow-list entries no longer name a file that starts a clock outside the timing collection; delete them:\n " + + string.Join("\n ", stale)); + + var members = rows.Count(r => r.InTimingCollection); + Assert.True( + members >= TimingMemberFloor, + $"only {members} Darling.Tests classes are in the timing collection; the scan has stopped seeing them"); + } + + [Fact] + public void EveryAllowListEntry_NamesARealFile_AndGivesAReason() + { + foreach (var (file, reason) in s_allowed) + { + Assert.True(File.Exists(Path.Combine(TestsRoot(), file)), $"allow-list entry {file} is not a file under Darling.Tests"); + Assert.False(string.IsNullOrWhiteSpace(reason), $"allow-list entry {file} has no reason"); + } + } + + [Fact] + public void TheTimingCollection_IsNotParallel_AndIsTheOnlyOneOfItsName() + { + var definitions = typeof(TimingTestCensusTests).Assembly.GetTypes() + .Select(t => (Type: t, Attribute: t.GetCustomAttribute())) + .Where(d => d.Attribute is not null && string.Equals(d.Attribute.Name, TimingTestScanner.CollectionName, StringComparison.Ordinal)) + .ToList(); + + var definition = Assert.Single(definitions); + Assert.True(definition.Attribute!.DisableParallelization, "the timing collection must run alone, or it protects nothing"); + } + + private static string TestsRoot([CallerFilePath] string thisFile = "") => Path.GetDirectoryName(thisFile)!; +} diff --git a/Darling/Darling.Tests/TimingTestScanner.cs b/Darling/Darling.Tests/TimingTestScanner.cs new file mode 100644 index 0000000000..539f55bc89 --- /dev/null +++ b/Darling/Darling.Tests/TimingTestScanner.cs @@ -0,0 +1,275 @@ +/* + * Copyright (c) 2026 Erik Darling, Darling Data LLC + * + * This file is part of the SQL Server Performance Monitor. + * + * Licensed under the MIT License. See LICENSE file in the project root for full license information. + */ + +using System; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using System.Text; +using System.Text.RegularExpressions; + +namespace Darling.Tests; + +/// +/// Finds the test classes that start a clock, for the timing census of each test suite (#5602). +/// +/// Why. xunit runs test classes in parallel, so a class that times its own work and fails when the +/// time passes a limit is judging the whole runner's load, not the code. #5561 and #5600 were that. The fix is a +/// single collection, timing, declared with DisableParallelization: xunit runs it alone, after the +/// parallel classes. The census (TimingTestCensusTests in each project) fails when a class starts a clock +/// outside that collection and is not on the allow list with its reason. +/// +/// What counts as starting a clock. A statement in a class body, after comments and string contents are +/// blanked, that calls Stopwatch.StartNew, Stopwatch.GetTimestamp or Stopwatch.GetElapsedTime, +/// constructs a Stopwatch, subtracts a DateTime.UtcNow/Now reading from a variable named like a start time (started, start, began; a heuristic, so "N minutes ago" arithmetic is not caught), or reads +/// Environment.TickCount. Source pins that quote those calls inside a string literal do not count. The rule is +/// wider than "asserts on the elapsed time": a class that only reports a time has to say so on the allow list, which +/// keeps one more place from growing an assertion unnoticed. +/// +/// Linked into Lite.Tests (see Lite.Tests.csproj and the lite_linked_shard filter in build.yml). +/// +internal static class TimingTestScanner +{ + /// The collection name every timing class carries. + internal const string CollectionName = "timing"; + + internal const string CollectionAttributeText = "[Collection(\"" + CollectionName + "\")]"; + + private static readonly Regex s_clock = new( + @"\bStopwatch\s*\.\s*(?:StartNew|GetTimestamp|GetElapsedTime)\s*\(|\bnew\s+(?:System\.Diagnostics\.)?Stopwatch\s*\(|\b(?:DateTime|DateTimeOffset)\s*\.\s*(?:UtcNow|Now)\s*-\s*(?:started|startedAt|start|startUtc|began|begun|t0)\b|\bEnvironment\s*\.\s*TickCount(?:64)?\b", + RegexOptions.CultureInvariant); + + private static readonly Regex s_classHeader = new( + @"^(?:(?:public|internal)\s+)?(?:(?:sealed|static|abstract|partial)\s+)*(?:class|record)\s+(?\w+)", + RegexOptions.CultureInvariant | RegexOptions.Multiline); + + /// One top-level class: its file, its name, whether it starts a clock, whether it is in timing. + internal sealed record Row(string File, string Name, bool StartsClock, bool InTimingCollection); + + /// Every top-level class under (build output skipped), one row per declaration. + internal static IReadOnlyList Scan(string testsRoot) + { + var rows = new List(); + foreach (var path in Directory.EnumerateFiles(testsRoot, "*.cs", SearchOption.AllDirectories).OrderBy(p => p, StringComparer.Ordinal)) + { + var relative = Path.GetRelativePath(testsRoot, path).Replace('\\', '/'); + if (relative.StartsWith("bin/", StringComparison.Ordinal) || relative.StartsWith("obj/", StringComparison.Ordinal)) + { + continue; + } + + rows.AddRange(ScanText(relative, File.ReadAllText(path))); + } + + return rows; + } + + /// The rows of one file's text. + internal static IReadOnlyList ScanText(string file, string text) + { + var withStrings = Blank(text, blankStrings: false); + var code = Blank(text, blankStrings: true); + var headers = s_classHeader.Matches(code); + var rows = new List(); + for (var i = 0; i < headers.Count; i++) + { + var start = headers[i].Index; + var end = i + 1 < headers.Count ? headers[i + 1].Index : code.Length; + + // The attribute lines directly above the declaration belong to it; the clock lines never sit there. + var attrStart = start; + while (attrStart > 0) + { + var lineEnd = attrStart - 1; + var lineStart = withStrings.LastIndexOf('\n', Math.Max(lineEnd - 1, 0)) + 1; + var line = withStrings.Substring(lineStart, lineEnd - lineStart).Trim(); + if (line.StartsWith('[') && line.EndsWith(']')) + { + attrStart = lineStart; + continue; + } + + break; + } + + var attributes = withStrings.Substring(attrStart, start - attrStart); + rows.Add(new Row( + file, + headers[i].Groups["name"].Value, + s_clock.IsMatch(code.AsSpan(start, end - start)), + attributes.Contains(CollectionAttributeText, StringComparison.Ordinal))); + } + + return rows; + } + + /// The problems with : a clock outside timing, off the allow list. + internal static List Problems(IReadOnlyList rows, IReadOnlyDictionary allowed) + { + var problems = new List(); + foreach (var row in rows.Where(r => r.StartsClock && !r.InTimingCollection)) + { + if (!allowed.ContainsKey(row.File)) + { + problems.Add($"{row.File}: {row.Name} starts a clock outside {CollectionAttributeText} and is not on the allow list"); + } + } + + return problems; + } + + /// The allow-list entries that no longer name a file with a clock outside timing: the list only shrinks. + internal static List Stale(IReadOnlyList rows, IReadOnlyDictionary allowed) + { + var live = rows.Where(r => r.StartsClock && !r.InTimingCollection).Select(r => r.File).ToHashSet(StringComparer.Ordinal); + return allowed.Keys.Where(k => !live.Contains(k)).OrderBy(k => k, StringComparer.Ordinal).ToList(); + } + + /// + /// with comments, and optionally the contents of string and character literals, replaced by + /// spaces. Length and line breaks are kept, so an index into the result is an index into the source. + /// + internal static string Blank(string text, bool blankStrings) + { + var output = new StringBuilder(text.Length); + var i = 0; + while (i < text.Length) + { + var c = text[i]; + var next = i + 1 < text.Length ? text[i + 1] : '\0'; + + if (c == '/' && next == '/') + { + while (i < text.Length && text[i] != '\n') + { + output.Append(text[i] == '\r' ? '\r' : ' '); + i++; + } + + continue; + } + + if (c == '/' && next == '*') + { + output.Append(" "); + i += 2; + while (i < text.Length && !(text[i] == '*' && i + 1 < text.Length && text[i + 1] == '/')) + { + output.Append(text[i] is '\n' or '\r' ? text[i] : ' '); + i++; + } + + if (i < text.Length) + { + output.Append(" "); + i += 2; + } + + continue; + } + + if (c == '"' && next == '"' && i + 2 < text.Length && text[i + 2] == '"') + { + // Raw string literal: three or more quotes to the same count. + var quotes = 0; + while (i + quotes < text.Length && text[i + quotes] == '"') + { + quotes++; + } + + var closer = new string('"', quotes); + var close = text.IndexOf(closer, i + quotes, StringComparison.Ordinal); + var stop = close < 0 ? text.Length : close + quotes; + CopyString(text, i, stop, output, blankStrings, keep: quotes); + i = stop; + continue; + } + + if (c == '"' || ((c == '@' || c == '$') && IsStringStart(text, i))) + { + var verbatim = false; + var j = i; + while (text[j] is '@' or '$') + { + verbatim |= text[j] == '@'; + j++; + } + + // text[j] is the opening quote. + var k = j + 1; + while (k < text.Length) + { + if (verbatim && text[k] == '"' && k + 1 < text.Length && text[k + 1] == '"') + { + k += 2; + continue; + } + + if (!verbatim && text[k] == '\\') + { + k += 2; + continue; + } + + if (text[k] == '"' || (!verbatim && text[k] == '\n')) + { + break; + } + + k++; + } + + var stop = Math.Min(k + 1, text.Length); + CopyString(text, i, stop, output, blankStrings, keep: (j - i) + 1); + i = stop; + continue; + } + + if (c == '\'') + { + var k = i + 1; + while (k < text.Length && text[k] != '\'' && text[k] != '\n') + { + k += text[k] == '\\' ? 2 : 1; + } + + var stop = Math.Min(k + 1, text.Length); + CopyString(text, i, stop, output, blankStrings, keep: 1); + i = stop; + continue; + } + + output.Append(c); + i++; + } + + return output.ToString(); + } + + private static bool IsStringStart(string text, int index) + { + var j = index; + while (j < text.Length && text[j] is '@' or '$') + { + j++; + } + + return j < text.Length && text[j] == '"'; + } + + /// Appends text[from..to); with the interior (past opening characters and the closing one) becomes spaces. + private static void CopyString(string text, int from, int to, StringBuilder output, bool blank, int keep) + { + for (var p = from; p < to; p++) + { + var ch = text[p]; + var isEdge = p - from < keep || p == to - 1; + output.Append(!blank || isEdge || ch is '\n' or '\r' ? ch : ' '); + } + } +} diff --git a/Darling/Darling.Tests/TuningHourlyCreateLiveTests.cs b/Darling/Darling.Tests/TuningHourlyCreateLiveTests.cs index 4dd0e12ca4..3b157ea480 100644 --- a/Darling/Darling.Tests/TuningHourlyCreateLiveTests.cs +++ b/Darling/Darling.Tests/TuningHourlyCreateLiveTests.cs @@ -25,6 +25,7 @@ namespace Darling.Tests; /// CREATE that is issued must be bounded by a lock_timeout. Own scratch database per test. /// /* #1776 own-store: each live fact mints its own scratch database through ScratchPostgres. */ +[Collection("timing")] public sealed class TuningHourlyCreateLiveTests { private static readonly string[] TunedIndexes = diff --git a/Darling/Darling.Tests/ViewerBlockedProcessReportsCoverageLiveTests.cs b/Darling/Darling.Tests/ViewerBlockedProcessReportsCoverageLiveTests.cs index d40a631785..4fc8ab4524 100644 --- a/Darling/Darling.Tests/ViewerBlockedProcessReportsCoverageLiveTests.cs +++ b/Darling/Darling.Tests/ViewerBlockedProcessReportsCoverageLiveTests.cs @@ -457,7 +457,7 @@ public static async Task CreateAsync(int rangeEndsDaysAgo, CancellationTo await probe.WaitAsync(ct); string? text = null; - OnStaThread(() => + StaTestThread.Run(() => { ViewerTimeHelper.CurrentDisplayMode = TimeDisplayMode.UTC; /* Seeded visible, so a no-op cannot pass as a hidden banner. */ @@ -629,22 +629,4 @@ public async ValueTask DisposeAsync() } /* WPF objects require STA; same shape as ViewerEventDataStartTests. */ - private static void OnStaThread(Action body) - { - Exception? error = null; - using var staGate = WpfStaGate.Enter(); - var thread = new Thread(() => - { - try { body(); } - catch (Exception ex) { error = ex; } - }); - thread.SetApartmentState(ApartmentState.STA); - thread.Start(); - thread.Join(); - - if (error is not null) - { - throw error; - } - } } diff --git a/Darling/Darling.Tests/ViewerClutterAndMemoryPressureDataStartTests.cs b/Darling/Darling.Tests/ViewerClutterAndMemoryPressureDataStartTests.cs index bf0668e3dc..10ffd6207b 100644 --- a/Darling/Darling.Tests/ViewerClutterAndMemoryPressureDataStartTests.cs +++ b/Darling/Darling.Tests/ViewerClutterAndMemoryPressureDataStartTests.cs @@ -398,7 +398,7 @@ await Assert.ThrowsAsync(() => ViewerServerTab.AwaitR private static string? ReadBanner(Action raise) { string? text = null; - OnStaThread(() => + StaTestThread.Run(() => { ViewerTimeHelper.CurrentDisplayMode = TimeDisplayMode.UTC; var banner = new TextBlock { Visibility = Visibility.Visible, Text = "stale" }; @@ -411,22 +411,4 @@ await Assert.ThrowsAsync(() => ViewerServerTab.AwaitR } /* WPF objects require STA; same shape as ViewerQueryHeatmapDataStartTests. */ - private static void OnStaThread(Action body) - { - Exception? error = null; - using var staGate = WpfStaGate.Enter(); - var thread = new Thread(() => - { - try { body(); } - catch (Exception ex) { error = ex; } - }); - thread.SetApartmentState(ApartmentState.STA); - thread.Start(); - thread.Join(); - - if (error is not null) - { - throw error; - } - } } diff --git a/Darling/Darling.Tests/ViewerCollectionHealthLoadTests.cs b/Darling/Darling.Tests/ViewerCollectionHealthLoadTests.cs index 8f7a04959b..87ba94bc73 100644 --- a/Darling/Darling.Tests/ViewerCollectionHealthLoadTests.cs +++ b/Darling/Darling.Tests/ViewerCollectionHealthLoadTests.cs @@ -73,48 +73,13 @@ private static List Runs(int count, DateTime first) => private static readonly string[] AllFour = ["health", "log", "chart", "caveats"]; - /// Runs on an STA thread with a dispatcher, as the tab's own load runs, and rethrows what it threw. - private static void OnDispatcher(Func body) - { - Exception? error = null; - using var staGate = WpfStaGate.Enter(); - var thread = new Thread(() => - { - var dispatcher = Dispatcher.CurrentDispatcher; - dispatcher.BeginInvoke(new Action(async () => - { - try - { - ViewerTimeHelper.CurrentDisplayMode = TimeDisplayMode.UTC; - await body(); - } - catch (Exception ex) - { - error = ex; - } - finally - { - dispatcher.BeginInvokeShutdown(DispatcherPriority.Background); - } - })); - Dispatcher.Run(); - }) - { - IsBackground = true, - }; - thread.SetApartmentState(ApartmentState.STA); - thread.Start(); - - if (!thread.Join(TimeSpan.FromSeconds(60))) + /// Runs on the shared STA thread, whose dispatcher resumes its awaits as the tab's own load does (#5602), with the viewer's display mode set to UTC. + private static void OnTabDispatcher(Func body) => + StaTestThread.Run(async () => { - throw new TimeoutException("the tab's step did not finish: it is waiting on something it should not wait for"); - } - - if (error is not null) - { - ExceptionDispatchInfo.Capture(error).Throw(); - } - } + ViewerTimeHelper.CurrentDisplayMode = TimeDisplayMode.UTC; + await body(); + }); /* The chart's read fails (here a real read against a store nothing listens on): the wrapper answers an empty list and logs it, the join the load runs does not throw, and the grids, the caveats and the note all draw, with the chart drawn empty. The coverage @@ -131,7 +96,7 @@ public async Task AChartReadThatFails_BlanksOnlyTheChart_TheGridsTheNoteAndTheCa await Task.WhenAll(logTask, trendTask); var drawn = new Drawn(); - OnDispatcher(async () => + OnTabDispatcher(async () => { drawn.Start(); await Step(drawn, logTask.Result, trendTask.Result, Task.FromResult(RangeStart.AddDays(2))); @@ -161,7 +126,7 @@ public void TheChartAndTheCaveats_DrawBeforeTheNoteAwaitsItsProbe() var probe = new TaskCompletionSource(); var drawn = new Drawn(); - OnDispatcher(async () => + OnTabDispatcher(async () => { drawn.Start(); var step = Step(drawn, Runs(3, RangeStart.AddDays(3)), [], probe.Task); @@ -187,7 +152,7 @@ public void AFullPage_NamesItsOldestRun_WithoutWaitingForAProbeThatNeverAnswers( var probe = new TaskCompletionSource(); var drawn = new Drawn(); - OnDispatcher(async () => + OnTabDispatcher(async () => { drawn.Start(); var step = Step(drawn, Runs(ViewerDataService.CollectionLogRowCap, oldest), [], probe.Task); @@ -208,7 +173,7 @@ public void AFullPage_StillLogsAProbeThatFailsLater() var probe = new TaskCompletionSource(); var warned = new List(); - OnDispatcher(async () => + OnTabDispatcher(async () => { var drawn = new Drawn(); drawn.Start(); diff --git a/Darling/Darling.Tests/ViewerCollectionLogDataStartLiveTests.cs b/Darling/Darling.Tests/ViewerCollectionLogDataStartLiveTests.cs index c6bbe3bd29..4d1cd9958b 100644 --- a/Darling/Darling.Tests/ViewerCollectionLogDataStartLiveTests.cs +++ b/Darling/Darling.Tests/ViewerCollectionLogDataStartLiveTests.cs @@ -220,7 +220,7 @@ private async Task TabAsync(int serverId, DateTime startUtc, DateTime await probe.WaitAsync(ct); string? text = null; - DataStartBannerReadout.OnStaThread(() => + StaTestThread.Run(() => { ViewerTimeHelper.CurrentDisplayMode = TimeDisplayMode.UTC; /* Seeded visible, so a no-op cannot pass as a hidden banner. */ @@ -243,39 +243,13 @@ private DrillAnswer Drill(int serverId, string collector, DateTime? asOfUtc) var viewer = _store.Viewer!; DrillAnswer? answer = null; - Exception? error = null; - using var staGate = WpfStaGate.Enter(); - var thread = new Thread(() => + StaTestThread.Run(async () => { - var dispatcher = Dispatcher.CurrentDispatcher; - dispatcher.BeginInvoke(new Action(async () => - { - try - { - ViewerTimeHelper.CurrentDisplayMode = TimeDisplayMode.UTC; - var banner = new TextBlock { Visibility = Visibility.Visible, Text = "stale" }; - var rows = await CollectionLogWindow.ReadDrillAsync(viewer, serverId, collector, banner, asOfUtc); - answer = new DrillAnswer(rows, banner.Visibility == Visibility.Visible ? banner.Text : null); - } - catch (Exception ex) - { - error = ex; - } - finally - { - dispatcher.BeginInvokeShutdown(DispatcherPriority.Background); - } - })); - Dispatcher.Run(); + ViewerTimeHelper.CurrentDisplayMode = TimeDisplayMode.UTC; + var banner = new TextBlock { Visibility = Visibility.Visible, Text = "stale" }; + var rows = await CollectionLogWindow.ReadDrillAsync(viewer, serverId, collector, banner, asOfUtc); + answer = new DrillAnswer(rows, banner.Visibility == Visibility.Visible ? banner.Text : null); }); - thread.SetApartmentState(ApartmentState.STA); - thread.Start(); - thread.Join(); - - if (error is not null) - { - throw error; - } return answer!; } diff --git a/Darling/Darling.Tests/ViewerCollectorDurationHoverTests.cs b/Darling/Darling.Tests/ViewerCollectorDurationHoverTests.cs index 109c4cd7a8..7929df183f 100644 --- a/Darling/Darling.Tests/ViewerCollectorDurationHoverTests.cs +++ b/Darling/Darling.Tests/ViewerCollectorDurationHoverTests.cs @@ -43,39 +43,25 @@ private static List TwentyBucketsWithOneBreak() => Enumerable.Range(0, 20).Select(k => new CollectorDurationBucket( "wait_stats", Start.AddMinutes(10 * k + (k >= 5 ? 60 : 0)), 100 + 3 * k, 50 + k, k + 1)).ToList(); - /// Runs on an STA thread in US English (the chart is a WPF control), and rethrows what it threw. - private static T OnStaThread(Func body) - { - T result = default!; - Exception? error = null; - using var staGate = WpfStaGate.Enter(); - var thread = new Thread(() => + /// Runs on the shared STA thread in US English (the chart is a WPF control), then puts the thread's culture back so the next body does not inherit it (#5602). + private static T InEnglish(Func body) => + StaTestThread.Run(() => { + var saved = CultureInfo.CurrentCulture; try { CultureInfo.CurrentCulture = English; - result = body(); + return body(); } - catch (Exception ex) + finally { - error = ex; + CultureInfo.CurrentCulture = saved; } }); - thread.SetApartmentState(ApartmentState.STA); - thread.Start(); - thread.Join(); - - if (error is not null) - { - ExceptionDispatchInfo.Capture(error).Throw(); - } - - return result; - } /// The hover text over each of the line's twenty points in turn (the mouse exactly on the point), in bucket order. private static List HoverOverEachBucket(List buckets) => - OnStaThread(() => + InEnglish(() => { var chart = new ScottPlot.WPF.WpfPlot(); var hover = new ChartHoverHelper(chart, "ms", displayZone: () => TimeZoneInfo.Utc); @@ -109,7 +95,7 @@ same strings through its own method (CollectorDurationDetailTextTests), so the t [InlineData(12L, 0.5, "Slowest of 12 runs; average 0.5 ms")] public void TheSharedText_SaysTheRunCountAndTheAverage_InTheseExactWords(long runs, double average, string expected) { - var text = OnStaThread(() => CollectorDurationHoverText.Detail(runs, average)); + var text = InEnglish(() => CollectorDurationHoverText.Detail(runs, average)); Assert.Equal(expected, text); } @@ -153,7 +139,7 @@ public void TheSeriesLines_AreTheSharedWords_OverEachBucketsOwnRunsAndAverage_Ke { var line = Assert.Single(CollectorDurationSeries.Build(TwentyBucketsWithOneBreak())); - var byX = OnStaThread(() => line.DetailsByX()); + var byX = InEnglish(() => line.DetailsByX()); Assert.Equal(20, byX.Count); for (var k = 0; k < 20; k++) @@ -188,7 +174,7 @@ public void ABucketAfterABreak_ShowsItsOwnRunsAndAverage_NotTheNextBucketsAndThe [Fact] public void TheLinesAreKeyedByX_SoEveryDrawnBucketFindsItsOwn_AndThePointTheBreakInsertsFindsNone() { - var (line, drawn) = OnStaThread(() => + var (line, drawn) = InEnglish(() => { var chart = new ScottPlot.WPF.WpfPlot(); var series = Assert.Single(CollectorDurationSeries.Build(TwentyBucketsWithOneBreak())); @@ -198,7 +184,7 @@ public void TheLinesAreKeyedByX_SoEveryDrawnBucketFindsItsOwn_AndThePointTheBrea return (series, scatter.Data.GetScatterPoints().ToList()); }); - var byX = OnStaThread(() => line.DetailsByX()); + var byX = InEnglish(() => line.DetailsByX()); Assert.Equal(20, byX.Count); Assert.Equal(21, drawn.Count); diff --git a/Darling/Darling.Tests/ViewerConfigChangesDataStartTests.cs b/Darling/Darling.Tests/ViewerConfigChangesDataStartTests.cs index 12dde354dc..523143d1a5 100644 --- a/Darling/Darling.Tests/ViewerConfigChangesDataStartTests.cs +++ b/Darling/Darling.Tests/ViewerConfigChangesDataStartTests.cs @@ -274,7 +274,7 @@ internal static class DataStartBannerReadout TimeDisplayMode mode = TimeDisplayMode.UTC, int? serverOffsetMinutes = null) { string? text = null; - OnStaThread(() => + StaTestThread.Run(() => { var savedMode = ViewerTimeHelper.CurrentDisplayMode; var savedClock = ViewerTimeHelper.ActiveServerClock; @@ -330,23 +330,4 @@ public static string Since(DateTime utc, TimeDisplayMode mode = TimeDisplayMode. } } - /// Runs on an STA thread (WPF objects require one) and rethrows what it threw. - public static void OnStaThread(Action body) - { - Exception? error = null; - using var staGate = WpfStaGate.Enter(); - var thread = new Thread(() => - { - try { body(); } - catch (Exception ex) { error = ex; } - }); - thread.SetApartmentState(ApartmentState.STA); - thread.Start(); - thread.Join(); - - if (error is not null) - { - throw error; - } - } } diff --git a/Darling/Darling.Tests/ViewerDataStartBannerTests.cs b/Darling/Darling.Tests/ViewerDataStartBannerTests.cs index f0b716aa1b..f28dc4fdd0 100644 --- a/Darling/Darling.Tests/ViewerDataStartBannerTests.cs +++ b/Darling/Darling.Tests/ViewerDataStartBannerTests.cs @@ -246,7 +246,7 @@ and the failure is logged with the surface and the cause. */ [Fact] public void AProbeThatThrows_HidesTheBanner_IsLogged_AndLetsTheLoadGoOn() { - OnStaThread(() => + StaTestThread.Run(() => { var banner = new TextBlock { Visibility = Visibility.Visible, Text = "Showing since 2026-09-04 00:00:00" }; var logged = new List<(string Source, string Message)>(); @@ -268,7 +268,7 @@ public void AProbeThatThrows_HidesTheBanner_IsLogged_AndLetsTheLoadGoOn() [Fact] public void AProbeThatAnswers_PassesTheAnswerOn_AndLogsNothing() { - OnStaThread(() => + StaTestThread.Run(() => { var banner = new TextBlock(); var logged = new List<(string Source, string Message)>(); @@ -296,7 +296,7 @@ public void TheBanner_NamesItsInstants_OnTheInvariantCulture(string cultureName) try { ViewerTimeHelper.CurrentDisplayMode = TimeDisplayMode.UTC; - OnStaThread(() => + StaTestThread.Run(() => { CultureInfo.CurrentCulture = CultureInfo.GetCultureInfo(cultureName); /* The premise: this culture's own formatting does not give the invariant text, so the pin is not vacuous. */ @@ -329,22 +329,4 @@ public void TheBanner_NamesItsInstants_OnTheInvariantCulture(string cultureName) } /* WPF objects require STA; same shape as RawWindowFloorViewerPortTests. */ - private static void OnStaThread(Action body) - { - Exception? error = null; - using var staGate = WpfStaGate.Enter(); - var thread = new Thread(() => - { - try { body(); } - catch (Exception ex) { error = ex; } - }); - thread.SetApartmentState(ApartmentState.STA); - thread.Start(); - thread.Join(); - - if (error is not null) - { - throw error; - } - } } diff --git a/Darling/Darling.Tests/ViewerDisabledListBoxThemeTests.cs b/Darling/Darling.Tests/ViewerDisabledListBoxThemeTests.cs index 280d6767c7..c3fdf86448 100644 --- a/Darling/Darling.Tests/ViewerDisabledListBoxThemeTests.cs +++ b/Darling/Darling.Tests/ViewerDisabledListBoxThemeTests.cs @@ -49,7 +49,7 @@ public sealed class ViewerDisabledListBoxThemeTests [InlineData("CoolBreezeTheme.xaml")] public void DisabledListBox_RendersTheThemedBackgroundDimmed_NotStockWhite(string themeFile) { - OnStaThread(() => + StaTestThread.Run(() => { var list = new ListBox { IsEnabled = false }; list.SetResourceReference(Control.BackgroundProperty, "BackgroundDarkBrush"); @@ -65,7 +65,7 @@ public void DisabledListBox_RendersTheThemedBackgroundDimmed_NotStockWhite(strin [InlineData("CoolBreezeTheme.xaml")] public void DisabledServerList_AsShipped_RendersTheThemedBackgroundDimmed(string themeFile) { - OnStaThread(() => + StaTestThread.Run(() => { var list = ParseServerList(); list.IsEnabled = false; @@ -80,7 +80,7 @@ public void DisabledServerList_AsShipped_RendersTheThemedBackgroundDimmed(string [InlineData("CoolBreezeTheme.xaml")] public void EnabledListBox_RendersIdenticallyToTheStockTemplate(string themeFile) { - OnStaThread(() => + StaTestThread.Run(() => { var themeStyle = Assert.IsType