You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Fix - replace fixed sleeps in tests with signals - #271
Before: several tests waited on fixed sleeps or unbounded polls. Two EDDN uploader tests could hang forever if the handler was never reached, and the settings "brief lock" test released its lock from a pool timer that a busy CI runner could delay past the store's retry budget.
After: those waits are tied to real signals (a TaskCompletionSource, the injected clock, PublishCompleted, or DisposeAsync draining the send chain). Test-only; no app code changed.
EDDN uploader: two while (Calls == 0) await Task.Delay(5) loops (no deadline) now await a TCS set by the handler.
EliteBGS: dropped a 100 ms delay; every caller joins the in-flight request synchronously before GetSystemAsync returns.
Discord presence: Thread.Sleep(5) replaced by advancing the injected clock past the throttle window.
EDDN batching / Inara resilience (history): removed the delay before DisposeAsync; dispose flushes and awaits, which is a stronger check.
Inara bridge: suppressed/disabled tests drain via DisposeAsync instead of 150 ms; the accumulate test now publishes a dock as a sentinel and asserts the single request carries everything.
Settings store: the 150 ms lock (the scenario itself) is released from a dedicated thread.
Radio engine start: the snapshot read ran on the pool with a 2 s bound and failed on the Windows runner (the blocked start holds a pool thread). It now reads on a dedicated thread with a 10 s bound; the start is held 30 s, so a read blocked on the lock still fails.
EDDN hand-off stress tests: Windows counted 1 of 12,000 uploaders as stranded. The cause was dispose's 1.5 s grace cancelling a starved worker, not the hand-off race (pinned to one core with a 1 ms grace it reproduces 6-9 times per run). The tests now use a one-minute grace; dispose still returns as soon as the worker finishes.
Twitch card: backend-fault test waits for the faulted PublishCompleted (raised after ReauthRequired) instead of 6 intervals.
Left on purpose
Task.WhenAny(x, Task.Delay(N)) watchdogs: upper bounds, not sleeps.
Task.Delay(Infinite) / sleeps inside plugin bodies: they model hung I/O or slow plugins.
Deadline-bounded polling helpers.
"Wait and confirm nothing happened" checks in the Twitch card service (10), Inara retry caps (2) and Discord privacy (1). These can only false-pass, not flake. Making them deterministic needs a TimeProvider seam in those services (no behaviour change), which is app code and is not in this PR.
How it was verified
dotnet build EDNexus.slnx # 0 warnings
dotnet test EDNexus.slnx # all green on Linux# affected classes run 5x in a row: green# full suite pinned to one core (taskset -c 0, DOTNET_PROCESSOR_COUNT=1): green
Windows: via the CI matrix on this PR.
Checklist
Builds clean (dotnet build EDNexus.slnx), no new warnings
Feature code readsCommanderState; only StateTracker mutates it (no feature code touched)
Platform-specific code sits behind an interface with a graceful fallback (n/a)
Validated against real journal data where relevant (n/a, tests only)
No secrets, tokens, or personal journal files committed
- 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 <noreply@anthropic.com>
No actionable comments were generated in the recent review.
Walkthrough
Tests replace fixed delays and polling with controlled clocks, disposal-based synchronization, and explicit completion signals.
Changes: Test synchronization
Layer / File(s)
Summary
Clock and thread-based timing tests/EDNexus.Tests/Discord/DiscordPresenceServiceTests.cs, tests/EDNexus.Tests/Settings/SettingsStorePersistenceTests.cs, tests/EDNexus.Tests/Radio/RadioEngineStartTests.cs
The presence test advances an injected clock. The persistence test releases its lock from a background thread. The radio test reads the snapshot on a dedicated thread and waits for completion.
Reporter flush and disposal checks tests/EDNexus.Tests/Reporting/EddnBridgeBatchingTests.cs, tests/EDNexus.Tests/Reporting/EddnUploaderHandOffTests.cs, tests/EDNexus.Tests/Reporting/InaraBridgeResilienceTests.cs, tests/EDNexus.Tests/Reporting/InaraBridgeTests.cs
Bridge tests use disposal to flush or await pending work before checking requests. Uploader test options set a one-minute disposal grace period.
Lookup and uploader tests use explicit request-state signals. The Twitch test waits for the backend-rejection completion event before checking reauthentication state.
This change only makes tests less dependent on timing, so it has no production impact. No merge-blocking risk is evident from the supplied review.
Pre-merge checks: 4
Check name
Status
Explanation
Description Check
Check skipped - CodeRabbit’s high-level summary is enabled.
Title check
The title names the main change: replacing fixed sleeps in tests with signals or deterministic synchronization. The changeset replaces fixed delays in EDDN, Inara, EliteBGS, Discord, radio, settings, …
Linked Issues check
Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check
Check skipped because no linked issues were found for this pull request.
Full details: Title check
Explanation
The title names the main change: replacing fixed sleeps in tests with signals or deterministic synchronization. The changeset replaces fixed delays in EDDN, Inara, EliteBGS, Discord, radio, settings, and Twitch tests with signals, awaits, injected clock advances, and dedicated threads. The title uses the 'Fix -' prefix, which is a minor style point and does not affect the match. No application code changed, which the title does not claim otherwise.
Finishing Touches
Generate unit tests (beta)
Commit to this branch
Create a new PR
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.
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 <noreply@anthropic.com>
…hort
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 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What & why
Before: several tests waited on fixed sleeps or unbounded polls. Two EDDN uploader tests could hang forever if the handler was never reached, and the settings "brief lock" test released its lock from a pool timer that a busy CI runner could delay past the store's retry budget.
After: those waits are tied to real signals (a
TaskCompletionSource, the injected clock,PublishCompleted, orDisposeAsyncdraining the send chain). Test-only; no app code changed.Follow-up to #265 (Windows test leg).
Changes
while (Calls == 0) await Task.Delay(5)loops (no deadline) now await a TCS set by the handler.GetSystemAsyncreturns.Thread.Sleep(5)replaced by advancing the injected clock past the throttle window.DisposeAsync; dispose flushes and awaits, which is a stronger check.DisposeAsyncinstead of 150 ms; the accumulate test now publishes a dock as a sentinel and asserts the single request carries everything.PublishCompleted(raised afterReauthRequired) instead of 6 intervals.Left on purpose
Task.WhenAny(x, Task.Delay(N))watchdogs: upper bounds, not sleeps.Task.Delay(Infinite)/ sleeps inside plugin bodies: they model hung I/O or slow plugins.TimeProviderseam in those services (no behaviour change), which is app code and is not in this PR.How it was verified
Windows: via the CI matrix on this PR.
Checklist
dotnet build EDNexus.slnx), no new warningsCommanderState; onlyStateTrackermutates it (no feature code touched)Generated by Claude Code
Summary by CodeRabbit