From 000c4ff2c947ebad6585100a865e2810647022d8 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 11 Nov 2026 04:24:57 +0000 Subject: [PATCH 1/3] Replace fixed sleeps in tests with signals where no app change is needed - EDDN uploader: wait on a TaskCompletionSource instead of an unbounded poll - EliteBGS: drop a 100 ms delay; callers join the in-flight request synchronously - Discord presence: advance the injected clock instead of Thread.Sleep - EDDN/Inara history tests: DisposeAsync drains, so the pre-dispose delay is gone - Inara: suppressed/disabled tests drain via DisposeAsync; accumulate test uses a dock as a sentinel - Settings: release the brief lock from a dedicated thread, not a pool timer - Twitch card: wait for the faulted PublishCompleted instead of a fixed delay Co-Authored-By: Claude Opus 5.5 --- .../Discord/DiscordPresenceServiceTests.cs | 7 +++---- .../EliteBgs/EliteBgsClientTests.cs | 2 +- .../Reporting/EddnBridgeBatchingTests.cs | 2 +- .../Reporting/EddnUploaderResilienceTests.cs | 13 +++++++++--- .../Reporting/InaraBridgeResilienceTests.cs | 3 ++- .../Reporting/InaraBridgeTests.cs | 20 ++++++++++++++++--- .../Settings/SettingsStorePersistenceTests.cs | 10 +++++++--- .../Twitch/TwitchStreamCardServiceTests.cs | 14 ++++++++++--- 8 files changed, 52 insertions(+), 19 deletions(-) diff --git a/tests/EDNexus.Tests/Discord/DiscordPresenceServiceTests.cs b/tests/EDNexus.Tests/Discord/DiscordPresenceServiceTests.cs index 78aa63e..4143f05 100644 --- a/tests/EDNexus.Tests/Discord/DiscordPresenceServiceTests.cs +++ b/tests/EDNexus.Tests/Discord/DiscordPresenceServiceTests.cs @@ -39,13 +39,12 @@ public void Construction_immediately_pushes_whatever_state_is_already_known() [Fact] public void A_relevant_change_outside_the_throttle_window_sends_immediately() { + var now = new DateTimeOffset(2026, 1, 1, 0, 0, 0, TimeSpan.Zero); var state = new CommanderState(); var client = new FakeDiscordRpcClient(); - // A near-zero window so the very first change (which follows the constructor's own send) - // isn't itself throttled by the earlier push. - using var service = new DiscordPresenceService(state, client, TimeSpan.FromMilliseconds(1)); + using var service = new DiscordPresenceService(state, client, TimeSpan.FromSeconds(15), clock: () => now); - System.Threading.Thread.Sleep(5); // clear the 1ms window from the constructor's initial send + now = now.AddMinutes(1); // past the window opened by the constructor's initial send state.StarSystem = "Colonia"; var last = Assert.Single(client.Sent, p => p.State == "Exploring Colonia"); diff --git a/tests/EDNexus.Tests/EliteBgs/EliteBgsClientTests.cs b/tests/EDNexus.Tests/EliteBgs/EliteBgsClientTests.cs index 17df88f..3778540 100644 --- a/tests/EDNexus.Tests/EliteBgs/EliteBgsClientTests.cs +++ b/tests/EDNexus.Tests/EliteBgs/EliteBgsClientTests.cs @@ -209,8 +209,8 @@ public async Task Concurrent_lookups_of_one_name_share_a_single_request() var handler = new CountingGateHandler(gate.Task, () => Interlocked.Increment(ref calls)); using var client = NewClient(null!, http: new HttpClient(handler)); + // Each call joins the in-flight request before it returns, so all eight share it by now. var tasks = Enumerable.Range(0, 8).Select(_ => client.GetSystemAsync("Sol")).ToArray(); - await Task.Delay(100); gate.SetResult(); var results = await Task.WhenAll(tasks); diff --git a/tests/EDNexus.Tests/Reporting/EddnBridgeBatchingTests.cs b/tests/EDNexus.Tests/Reporting/EddnBridgeBatchingTests.cs index 473bc2a..bf8e044 100644 --- a/tests/EDNexus.Tests/Reporting/EddnBridgeBatchingTests.cs +++ b/tests/EDNexus.Tests/Reporting/EddnBridgeBatchingTests.cs @@ -94,7 +94,7 @@ public async Task Historical_signals_are_never_uploaded() bus.Publish(Entry(Jump, historical: true)); bus.Publish(Entry(Signal("A"), historical: true)); bus.Publish(Entry("""{ "timestamp": "2020-01-01T00:00:06Z", "event": "Music", "MusicTrack": "x" }""", historical: true)); - await Task.Delay(100); + // Disposing flushes a trailing run and waits for the upload, so anything buffered would be sent here. await bridge.DisposeAsync(); Assert.Equal(0, handler.CallCount); diff --git a/tests/EDNexus.Tests/Reporting/EddnUploaderResilienceTests.cs b/tests/EDNexus.Tests/Reporting/EddnUploaderResilienceTests.cs index ef5b578..edc26c1 100644 --- a/tests/EDNexus.Tests/Reporting/EddnUploaderResilienceTests.cs +++ b/tests/EDNexus.Tests/Reporting/EddnUploaderResilienceTests.cs @@ -59,8 +59,10 @@ protected override Task SendAsync(HttpRequestMessage reques public async Task Queue_is_bounded_and_drops_the_oldest_queued_messages() { var gate = new TaskCompletionSource(); + var inFlight = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); var handler = new ScriptedHandler(async (_, _) => { + inFlight.TrySetResult(); await gate.Task; return new HttpResponseMessage(HttpStatusCode.OK); }); @@ -70,7 +72,7 @@ public async Task Queue_is_bounded_and_drops_the_oldest_queued_messages() var messages = Enumerable.Range(0, 10).Select(Message).ToList(); uploader.Enqueue(messages[0]); - while (handler.Calls == 0) await Task.Delay(5); // message 0 is now in flight, queue empty + await inFlight.Task.WaitAsync(TimeSpan.FromSeconds(10)); // message 0 is now in flight, queue empty foreach (var m in messages.Skip(1)) uploader.Enqueue(m); gate.SetResult(); @@ -104,11 +106,16 @@ public async Task Messages_that_waited_too_long_are_dropped_unsent() [Fact] public async Task Dispose_cancels_a_pending_retry_wait_promptly() { - var handler = new ScriptedHandler((_, _) => Task.FromResult(new HttpResponseMessage(HttpStatusCode.InternalServerError))); + var attempted = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var handler = new ScriptedHandler((_, _) => + { + attempted.TrySetResult(); + return Task.FromResult(new HttpResponseMessage(HttpStatusCode.InternalServerError)); + }); var uploader = new EddnUploader(Options(o => o.RetryDelay = TimeSpan.FromHours(1)), new HttpClient(handler)); uploader.Enqueue(Message(1)); - while (handler.Calls == 0) await Task.Delay(5); // first attempt failed; now sleeping the retry delay + await attempted.Task.WaitAsync(TimeSpan.FromSeconds(10)); // the first attempt is failing; the retry waits an hour var sw = Stopwatch.StartNew(); await uploader.DisposeAsync().AsTask().WaitAsync(TimeSpan.FromSeconds(10)); diff --git a/tests/EDNexus.Tests/Reporting/InaraBridgeResilienceTests.cs b/tests/EDNexus.Tests/Reporting/InaraBridgeResilienceTests.cs index 1afd506..7ea4239 100644 --- a/tests/EDNexus.Tests/Reporting/InaraBridgeResilienceTests.cs +++ b/tests/EDNexus.Tests/Reporting/InaraBridgeResilienceTests.cs @@ -102,7 +102,8 @@ public async Task Replayed_history_including_shutdown_uploads_nothing() bus.Publish(Entry("""{ "timestamp": "2020-01-01T00:00:03Z", "event": "Loadout", "Ship": "python", "ShipID": 7 }""", historical: true)); bus.Publish(Entry("""{ "timestamp": "2020-01-01T00:00:04Z", "event": "Reputation", "Empire": 10.0 }""", historical: true)); bus.Publish(Entry("""{ "timestamp": "2020-01-01T00:20:00Z", "event": "Shutdown" }""", historical: true)); - await Task.Delay(150); + // Disposing flushes whatever is pending and waits for the send chain, so anything the history + // had queued would be sent here. await bridge.DisposeAsync(); Assert.Equal(0, handler.CallCount); diff --git a/tests/EDNexus.Tests/Reporting/InaraBridgeTests.cs b/tests/EDNexus.Tests/Reporting/InaraBridgeTests.cs index e9a2084..6e4e202 100644 --- a/tests/EDNexus.Tests/Reporting/InaraBridgeTests.cs +++ b/tests/EDNexus.Tests/Reporting/InaraBridgeTests.cs @@ -59,8 +59,18 @@ public async Task Accumulating_events_do_not_send_without_a_trigger() bus.Publish(Live("""{ "timestamp": "2020-01-01T00:00:01Z", "event": "Loadout", "Ship": "python", "ShipID": 7 }""")); bus.Publish(Live("""{ "timestamp": "2020-01-01T00:00:02Z", "event": "Rank", "Combat": 3 }""")); - await Task.Delay(150); Assert.Equal(0, handler.CallCount); // nothing sent — no trigger occurred + + // A dock is a trigger, and flushes synchronously on this thread. Had any of the events above + // sent on its own, it would have gone out first and taken its event out of the batch; instead + // the first and only request carries all of them together with the dock. + bus.Publish(Live("""{ "timestamp": "2020-01-01T00:10:00Z", "event": "Docked", "StarSystem": "Sol", "StationName": "X", "MarketID": 1 }""")); + await bridge.DisposeAsync(); // waits for the send chain + + var body = Assert.Single(handler.Bodies); + Assert.Contains("setCommanderShip", body); + Assert.Contains("setCommanderRankPilot", body); + Assert.Contains("addCommanderTravelDock", body); } [Fact] @@ -110,7 +120,9 @@ public async Task Suppressed_reporter_never_sends_even_when_enabled() bus.Publish(Live("""{ "timestamp": "2020-01-01T00:00:00Z", "event": "LoadGame", "Commander": "Jameson", "FID": "F1", "Credits": 100 }""")); bus.Publish(Live("""{ "timestamp": "2020-01-01T00:10:00Z", "event": "Docked", "StarSystem": "Sol", "StationName": "X", "MarketID": 1 }""")); - await Task.Delay(150); + // The dock flushes synchronously, and disposing flushes again and waits for the send chain, so + // anything that was going to go out has gone out by now. + await bridge.DisposeAsync(); Assert.Equal(0, handler.CallCount); } @@ -125,7 +137,9 @@ public async Task Disabled_reporter_never_sends() bus.Publish(Live("""{ "timestamp": "2020-01-01T00:00:00Z", "event": "LoadGame", "Commander": "Jameson", "FID": "F1", "Credits": 100 }""")); bus.Publish(Live("""{ "timestamp": "2020-01-01T00:10:00Z", "event": "Docked", "StarSystem": "Sol", "StationName": "X", "MarketID": 1 }""")); - await Task.Delay(150); + // The dock flushes synchronously, and disposing flushes again and waits for the send chain, so + // anything that was going to go out has gone out by now. + await bridge.DisposeAsync(); Assert.Equal(0, handler.CallCount); } diff --git a/tests/EDNexus.Tests/Settings/SettingsStorePersistenceTests.cs b/tests/EDNexus.Tests/Settings/SettingsStorePersistenceTests.cs index 51b9cbd..05dad76 100644 --- a/tests/EDNexus.Tests/Settings/SettingsStorePersistenceTests.cs +++ b/tests/EDNexus.Tests/Settings/SettingsStorePersistenceTests.cs @@ -149,16 +149,20 @@ public void A_file_that_could_not_be_read_at_startup_is_not_overwritten_by_a_lat } [Fact] - public async Task A_brief_lock_on_startup_is_waited_out() + public void A_brief_lock_on_startup_is_waited_out() { Directory.CreateDirectory(Dir); File.WriteAllText(SettingsPath, """{"Reporting":{"Inara":{"ApiKey":"keep-me"}}}"""); + // The 150 ms lock is the scenario itself, well inside the store's ~600 ms of retries. It is + // released from a dedicated thread rather than a pool timer, so a busy thread pool on a loaded + // CI runner cannot stretch it past the retry budget. var locked = new FileStream(SettingsPath, FileMode.Open, FileAccess.ReadWrite, FileShare.None); - var release = Task.Run(async () => { await Task.Delay(150); locked.Dispose(); }); + var release = new Thread(() => { Thread.Sleep(150); locked.Dispose(); }) { IsBackground = true }; + release.Start(); var loaded = NewStore().Load(); - await release; + release.Join(); Assert.Equal("keep-me", loaded.Reporting.Inara.ApiKey); } diff --git a/tests/EDNexus.Tests/Twitch/TwitchStreamCardServiceTests.cs b/tests/EDNexus.Tests/Twitch/TwitchStreamCardServiceTests.cs index 58e9d2d..9fe6a8c 100644 --- a/tests/EDNexus.Tests/Twitch/TwitchStreamCardServiceTests.cs +++ b/tests/EDNexus.Tests/Twitch/TwitchStreamCardServiceTests.cs @@ -548,11 +548,19 @@ public async Task A_backend_fault_does_not_stop_publishing_the_way_a_revoked_tok }; using var service = Create(state, client); - service.ReauthRequired += () => reauth++; + service.ReauthRequired += () => Interlocked.Increment(ref reauth); + // ReauthRequired is raised before PublishCompleted for the same attempt, so once the fault + // has been reported, any reauth it was going to cause has already happened. + var faulted = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + service.PublishCompleted += r => + { + if (r.Status == StreamStatePublishStatus.BackendRejectedByTwitch) faulted.TrySetResult(); + }; service.RequestPublish(); - await Task.Delay(FastInterval * 6); + await faulted.Task.WaitAsync(TimeSpan.FromSeconds(5)); - Assert.Equal(0, reauth); // the commander's sign-in is not the problem + Assert.Equal(0, Volatile.Read(ref reauth)); // the commander's sign-in is not the problem + Assert.False(service.StoppedForReauth); } [Fact] From c1a3f87be085621123a7664730fe8f014bb5a4f4 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 11 Nov 2026 04:28:46 +0000 Subject: [PATCH 2/3] Radio start test: read the snapshot on a dedicated thread The 2 s watchdog failed on the Windows runner (CI run 42959125896): the pooled read can queue behind the blocked engine start without ever touching the lock. Read on its own thread with a 10 s bound; the start is held for 30 s, so a read blocked on the lock still fails the test. Co-Authored-By: Claude Opus 5.5 --- tests/EDNexus.Tests/Radio/RadioEngineStartTests.cs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/tests/EDNexus.Tests/Radio/RadioEngineStartTests.cs b/tests/EDNexus.Tests/Radio/RadioEngineStartTests.cs index f43f6c7..e4a9ffc 100644 --- a/tests/EDNexus.Tests/Radio/RadioEngineStartTests.cs +++ b/tests/EDNexus.Tests/Radio/RadioEngineStartTests.cs @@ -35,9 +35,12 @@ public async Task The_snapshot_stays_readable_while_the_engine_is_starting() try { // This is what the UI thread does every 250 ms; it must not wait for the native start. - var read = Task.Run(() => radio.Snapshot); - var finished = await Task.WhenAny(read, Task.Delay(TimeSpan.FromSeconds(2))); - Assert.Same(read, finished); + // Read on a dedicated thread, not the pool: the blocked start already holds a pool thread, + // and on a busy CI runner a pooled read can sit queued for seconds without ever touching + // the lock. The start is held for 30 s, so a read blocked on it cannot finish inside 10. + var read = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + new Thread(() => { _ = radio.Snapshot; read.SetResult(); }) { IsBackground = true }.Start(); + await read.Task.WaitAsync(TimeSpan.FromSeconds(10)); } finally { From 4974d7316a1952358448f7b89c877bd9649b1df0 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 11 Nov 2026 04:36:00 +0000 Subject: [PATCH 3/3] EDDN hand-off tests: don't let dispose's grace period cut the drain short Windows CI (run 42959504078) counted 1 of 12,000 uploaders as stranded. DisposeAsync waits only DisposeGrace (1.5 s) before cancelling, so a worker starved by the 16-way stress loop was cancelled before its second send. Pinned to one core with a 1 ms grace this reproduces as 6-9 'stranded' per run; with a one-minute grace it passes. Dispose still returns as soon as the worker finishes, so a truly stranded message is still caught. Co-Authored-By: Claude Opus 5.5 --- tests/EDNexus.Tests/Reporting/EddnUploaderHandOffTests.cs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/EDNexus.Tests/Reporting/EddnUploaderHandOffTests.cs b/tests/EDNexus.Tests/Reporting/EddnUploaderHandOffTests.cs index 9823423..2f0166c 100644 --- a/tests/EDNexus.Tests/Reporting/EddnUploaderHandOffTests.cs +++ b/tests/EDNexus.Tests/Reporting/EddnUploaderHandOffTests.cs @@ -23,6 +23,12 @@ public class EddnUploaderHandOffTests SoftwareName = "T", SoftwareVersion = "1", RetryDelay = TimeSpan.Zero, + // Dispose only waits DisposeGrace for the queue to drain before cancelling. These tests count + // what was sent after dispose, so a drain cut short by a starved thread pool (16-way parallel on + // a small CI runner) would look exactly like a stranded message. A generous grace removes that + // false positive; dispose still returns as soon as the worker finishes, so a stranded message + // (the worker exiting without it) is still caught. + DisposeGrace = TimeSpan.FromMinutes(1), }; ///