From 6f6bc9f1d92a7a0274fb189816db3c09bcd0e411 Mon Sep 17 00:00:00 2001 From: "Jeremy D. Miller" Date: Tue, 4 Aug 2026 18:55:17 -0500 Subject: [PATCH] Burn down the remaining Category=Flaky exclusions (GH-3763) Seven classes, one real bug among them. With the two ASB classes handled on GH-3825 and GH-3826, this takes the Flaky exclusion list to zero. Polecat subscriptions_end_to_end -- the actual defect. Failed 1 of 3 class runs but 6 of 6 in isolation, so an in-class interaction rather than a flake. It is the daemon-outbox race: WolverineSubscriptionRunner stages envelopes in a Marten outbox and the daemon flushes them AFTER committing the page and its progress, so WaitForNonStaleData() returning does not mean the messages were published, and the session ends on the activity lull with stragglers unrecorded. The MartenSubscriptionTests twin of this file already carries the fix -- explicit WaitForExecutionOf(count) waiters plus a 60s timeout -- and the Polecat copy never inherited it. Copied test files do not inherit each other's later repairs. Two tests in the same class carried Skip = "Known TrackActivity race condition with publishing subscriptions - same failure in MartenSubscriptionTests" for that same cause. Both are now unskipped and covered by the same waiters: 7 passing + 2 skipped becomes 9 passing. The other six needed no code change: - ASB session_id_pinning, dead_letter_queue_recovery, and both Bug_2588 classes were each tagged in the very commit that introduced the feature they cover, with no note. They were never green rather than intermittently green. - SqliteTests multi_tenancy_with_multiple_files was excluded for hanging the 10-minute job; the test has since been rewritten around the bounded Poll() helper it now uses. Stale note, not wrong when written. - MartenTests using_tenant_specific_queues_and_subscriptions is green 5/5. OracleTests.LeaderElection leader_election is untagged provisionally. Its rationale was real -- TM/DML lock contention against the shared CI Oracle container -- but the DDL_LOCK_TIMEOUT plus per-table ORA-00054 retry it stood in for has since been added to beforeBuildingHost(). 15/15 on four consecutive runs locally, though that contention cannot be reproduced on a dev box. The comment says so; watch CIOracle. Measurements: Polecat 245/0 (8/8 runs on the class), Sqlite 162/0, MartenTests 548/0, Oracle 15/0 x4, ASB 315/0. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01WHAuhdWS3XeAk16swV9G8m --- .../leader_election.cs | 11 +++++----- ...enant_specific_queues_and_subscriptions.cs | 1 - .../Subscriptions/subscriptions_end_to_end.cs | 21 ++++++++++++++++--- .../multi_tenancy_with_multiple_files.cs | 9 ++++---- ...x_with_handler_and_conventional_routing.cs | 2 -- .../dead_letter_queue_recovery.cs | 1 - .../session_id_pinning.cs | 1 - 7 files changed, 28 insertions(+), 18 deletions(-) diff --git a/src/Persistence/LeaderElection/OracleTests.LeaderElection/leader_election.cs b/src/Persistence/LeaderElection/OracleTests.LeaderElection/leader_election.cs index b2c095fa1..2633f2b5f 100644 --- a/src/Persistence/LeaderElection/OracleTests.LeaderElection/leader_election.cs +++ b/src/Persistence/LeaderElection/OracleTests.LeaderElection/leader_election.cs @@ -11,11 +11,12 @@ namespace OracleTests.LeaderElection; // the OracleControlTransport / NodeControlEndpoint when running in Balanced // durability mode, otherwise leadership election cannot start. // -// Marked Flaky so CI does not run them by default. The compliance suite spins up -// 3-4 hosts per test and depends on TM/DML lock release between runs against a -// single Oracle instance — fine to run locally one-at-a-time, but unstable -// against the shared CI Oracle container. See #2618 (CI stabilization). -[Trait("Category", "Flaky")] +// Previously excluded from CI as Flaky because the compliance suite spins up 3-4 hosts per test +// and depends on TM/DML lock release between runs against a single Oracle instance (#2618). The +// beforeBuildingHost() teardown below has since grown the DDL_LOCK_TIMEOUT + per-table ORA-00054 +// retry that the exclusion was standing in for. Re-measured 2026-08-04: 15/15 green on four +// consecutive runs, 1m44s each. Watch CIOracle -- the shared-container contention this was +// hedging against cannot be reproduced on a dev box. public class leader_election : LeadershipElectionCompliance { public const string SchemaName = "WOLVERINE"; diff --git a/src/Persistence/MartenTests/MultiTenancy/using_tenant_specific_queues_and_subscriptions.cs b/src/Persistence/MartenTests/MultiTenancy/using_tenant_specific_queues_and_subscriptions.cs index fb9553767..7a364d8f2 100644 --- a/src/Persistence/MartenTests/MultiTenancy/using_tenant_specific_queues_and_subscriptions.cs +++ b/src/Persistence/MartenTests/MultiTenancy/using_tenant_specific_queues_and_subscriptions.cs @@ -21,7 +21,6 @@ namespace MartenTests.MultiTenancy; -[Trait("Category", "Flaky")] public class using_tenant_specific_queues_and_subscriptions : PostgresqlContext, IAsyncLifetime { private readonly List _receivers = new(); diff --git a/src/Persistence/PolecatTests/Subscriptions/subscriptions_end_to_end.cs b/src/Persistence/PolecatTests/Subscriptions/subscriptions_end_to_end.cs index 62c15eb8a..02dcc78b4 100644 --- a/src/Persistence/PolecatTests/Subscriptions/subscriptions_end_to_end.cs +++ b/src/Persistence/PolecatTests/Subscriptions/subscriptions_end_to_end.cs @@ -17,7 +17,6 @@ namespace PolecatTests.Subscriptions; -[Trait("Category", "Flaky")] public class subscriptions_end_to_end { /// @@ -233,7 +232,7 @@ public async Task use_inline_subscription_filtered() PcTotalsHandler.Handled.ShouldBe(['a', 'b', 'a', 'a', 'a', 'a', 'b', 'b']); } - [Fact(Skip = "Known TrackActivity race condition with publishing subscriptions - same failure in MartenSubscriptionTests")] + [Fact] public async Task use_unfiltered_publishing_subscription() { const string schema = "pc_subscriptions_pub"; @@ -276,8 +275,16 @@ public async Task use_unfiltered_publishing_subscription() await daemon.WaitForNonStaleData(30.Seconds()); }; + // The daemon flushes the subscription's staged outbox AFTER it commits the page and its + // progress, so WaitForNonStaleData() returning does NOT mean the messages have been + // published. Without explicit waiters the session ends on the activity lull and the + // stragglers are never recorded. Same fix as the MartenSubscriptionTests twin. var tracked = await host .TrackActivity() + .Timeout(60.Seconds()) + .WaitForExecutionOf>(6) + .WaitForExecutionOf(7) + .WaitForExecutionOf>(6) .ExecuteAndWaitAsync(writeEvents); tracked.Executed.MessagesOf>().Count().ShouldBe(6); @@ -285,7 +292,7 @@ public async Task use_unfiltered_publishing_subscription() tracked.Executed.MessagesOf>().Count().ShouldBe(6); } - [Fact(Skip = "Known TrackActivity race condition with publishing subscriptions - same failure in MartenSubscriptionTests")] + [Fact] public async Task use_filtered_publishing_subscription() { const string schema = "pc_subscriptions_pub_filt"; @@ -332,8 +339,12 @@ public async Task use_filtered_publishing_subscription() await daemon.WaitForNonStaleData(30.Seconds()); }; + // See use_unfiltered_publishing_subscription for why the explicit waiters are needed var tracked = await host .TrackActivity() + .Timeout(60.Seconds()) + .WaitForExecutionOf>(6) + .WaitForExecutionOf>(6) .ExecuteAndWaitAsync(writeEvents); tracked.Executed.MessagesOf>().Count().ShouldBe(6); @@ -391,8 +402,12 @@ public async Task use_transformed_publishing_subscription() await daemon.WaitForNonStaleData(60.Seconds()); }; + // See use_unfiltered_publishing_subscription for why the explicit waiters are needed var tracked = await host .TrackActivity() + .Timeout(60.Seconds()) + .WaitForExecutionOf>(6) + .WaitForExecutionOf(6) .ExecuteAndWaitAsync(writeEvents); tracked.Executed.MessagesOf>().Count().ShouldBe(6); diff --git a/src/Persistence/SqliteTests/Transport/multi_tenancy_with_multiple_files.cs b/src/Persistence/SqliteTests/Transport/multi_tenancy_with_multiple_files.cs index 3a7f85e7f..28b8d6752 100644 --- a/src/Persistence/SqliteTests/Transport/multi_tenancy_with_multiple_files.cs +++ b/src/Persistence/SqliteTests/Transport/multi_tenancy_with_multiple_files.cs @@ -9,11 +9,10 @@ namespace SqliteTests.Transport; -// CI marker: scheduled_messages_are_processed_in_tenant_files reliably hangs the -// 10-minute sqlite job, and the 2-attempt retry policy multiplies that into a -// guaranteed timeout. Until the test is rewritten with a hard wait-bound, run it -// only locally via the Flaky filter. See #2618 (CI stabilization). -[Trait("Category", "Flaky")] +// The hang that got this class excluded (scheduled_messages_are_processed_in_tenant_files +// eating the 10-minute sqlite job, see #2618) was cured when the test was rewritten around the +// bounded Poll() helper below -- it no longer waits on an unbounded condition. Re-measured +// 2026-08-04: 2 tests in 3s, five consecutive runs, and 1m15s for the whole SqliteTests project. [Collection("sqlite")] public class multi_tenancy_with_multiple_files : SqliteContext, IAsyncLifetime { diff --git a/src/Transports/Azure/Wolverine.AzureServiceBus.Tests/Bugs/Bug_2588_durable_outbox_with_handler_and_conventional_routing.cs b/src/Transports/Azure/Wolverine.AzureServiceBus.Tests/Bugs/Bug_2588_durable_outbox_with_handler_and_conventional_routing.cs index 64d620e2a..bf8abadcd 100644 --- a/src/Transports/Azure/Wolverine.AzureServiceBus.Tests/Bugs/Bug_2588_durable_outbox_with_handler_and_conventional_routing.cs +++ b/src/Transports/Azure/Wolverine.AzureServiceBus.Tests/Bugs/Bug_2588_durable_outbox_with_handler_and_conventional_routing.cs @@ -18,7 +18,6 @@ namespace Wolverine.AzureServiceBus.Tests.Bugs; /// short-circuit on endpoint.Subscriptions.Any() == false and never upgrade /// the endpoint mode to Durable. /// -[Trait("Category", "Flaky")] public class Bug_2588_durable_outbox_with_handler_and_conventional_routing : IAsyncLifetime { private IHost _host = null!; @@ -74,7 +73,6 @@ public void conventionally_routed_sender_should_be_durable_when_handler_is_also_ /// queue-based one. Both inherit from MessageRoutingConvention<,,,> /// and share the same fix path. /// -[Trait("Category", "Flaky")] public class Bug_2588_durable_outbox_with_handler_and_topic_broadcasting_routing : IAsyncLifetime { private IHost _host = null!; diff --git a/src/Transports/Azure/Wolverine.AzureServiceBus.Tests/dead_letter_queue_recovery.cs b/src/Transports/Azure/Wolverine.AzureServiceBus.Tests/dead_letter_queue_recovery.cs index 24be1e44e..9a96962dc 100644 --- a/src/Transports/Azure/Wolverine.AzureServiceBus.Tests/dead_letter_queue_recovery.cs +++ b/src/Transports/Azure/Wolverine.AzureServiceBus.Tests/dead_letter_queue_recovery.cs @@ -20,7 +20,6 @@ namespace Wolverine.AzureServiceBus.Tests; /// set). The background recovery listener drains the sub-queue and the dead letter ends up queryable /// in Wolverine's durable storage. /// -[Trait("Category", "Flaky")] public class dead_letter_queue_recovery : IAsyncLifetime { private readonly string _queueName = $"dlqrecovery{Guid.NewGuid():N}"; diff --git a/src/Transports/Azure/Wolverine.AzureServiceBus.Tests/session_id_pinning.cs b/src/Transports/Azure/Wolverine.AzureServiceBus.Tests/session_id_pinning.cs index 89899e95b..c11136e96 100644 --- a/src/Transports/Azure/Wolverine.AzureServiceBus.Tests/session_id_pinning.cs +++ b/src/Transports/Azure/Wolverine.AzureServiceBus.Tests/session_id_pinning.cs @@ -11,7 +11,6 @@ namespace Wolverine.AzureServiceBus.Tests; // GH-3533: pinning a session-enabled listener to specific session identifiers turns the session id // into a broker-enforced routing key on a shared queue, so a listener pinned to "A" never sees the // messages meant for "B". -[Trait("Category", "Flaky")] public class session_id_pinning : IAsyncLifetime { private IHost _host = null!;