fix flaky HeartbeatEndpointSettingsSyncHostedService tests by polling instead of fixed delay - #5756
Closed
soujay wants to merge 21 commits into
Closed
fix flaky HeartbeatEndpointSettingsSyncHostedService tests by polling instead of fixed delay#5756soujay wants to merge 21 commits into
soujay wants to merge 21 commits into
Conversation
… instead of fixed delay
Update SSH.NET to 2026.0.0 to address CVE
Relocates the Event Source creation and wait logic from RavenDB-specific folders to the base acceptance test projects. This ensures the setup is available for all persistence types and prevents race conditions when multiple tests attempt to create the same event source during parallel execution.
Move Event Source setup to shared acceptance test projects
…nd services This change continues the effort to consistently apply optional `CancellationToken` parameters to asynchronous methods across the monitoring project, including hosting commands, background services, and acceptance test infrastructure. Cancellation tokens are now propagated to underlying asynchronous operations, ensuring proper responsiveness to cancellation and retiring the remaining cancellation analyzer debt for the monitoring projects.
Propagate cancellation tokens through monitoring hosting and background services
…nfig app Retires the cancellation analyzer debt blocks in ServiceControlInstaller.Engine, ServiceControl.Management.PowerShell and ServiceControl.Config.Tests, and narrows the ServiceControl.Config block to the one boundary that cannot carry a token. Phase 7 of the propagation work. The engine change is what forces these projects to move together. AbstractCommandChecks' eight abstract members take tokens, the prompt callback becomes Func<PathInfo, CancellationToken, Task<bool>> throughout PathsValidator, the installable bases and the unattended installers, and ValidateNewInstance loses params so the token can be last. PowerShell's PowerShellCommandChecks and the three New-*Instance cmdlets follow, along with the Config app's ScmuCommandChecks, InstallerModule and add attachments. Two token drops are fixed in the Config app's Caliburn layer. RxScreen received a CancellationToken in ActivateAsync and DeactivateAsync and discarded it instead of passing it to OnInitialize, OnActivate and OnDeactivate; RxConductorBase did the same in ActivateItemAsync and DeactivateItemAsync. Both now forward it, so screen activation is cancellable. Command bodies reached through ReactiveCommand.CreateFromTask now take a required token, which binds its Func<CancellationToken, Task> overload and gives them a real one. Config's own Command.Create path bottoms out in System.Windows.Input.ICommand.Execute, which returns void and has no token to offer, so those command types keep a file-scoped block that says so, and their call sites pass CancellationToken.None explicitly. Caliburn's IClose.TryCloseAsync and IEventAggregator declare no token and cannot be changed, so those are inline pragmas rather than fixes.
Update IBodyStorage and its implementations to return a result object that distinguishes between not found, empty, and unavailable bodies. This allows the API and retry logic to handle these scenarios more accurately and provide better feedback.
…ests Refactor body storage retrieval to use explicit result states
… and acceptance test infrastructure Retires the cancellation analyzer debt blocks in six projects, which is Phase 7 of the propagation work. ServiceControl.Persistence.EFCore.SqlServer / .PostgreSql: the dialect implementations now match their interfaces, whose tokens are already optional. ServiceControl.RavenDB: EmbeddedDatabase.DeleteDatabase takes a token and forwards it, so deleting a database can be cancelled. Particular.LicensingComponent: IAuditQuery and IThroughputCollector and their implementations, plus BrokerThroughputCollectorHostedService's stoppingToken renamed to cancellationToken. Five catch blocks now catch OperationCanceledException filtered on the token before catching Exception, so shutdown cancellation is no longer reported as a failure to gather throughput or to reach the audit remotes. ServiceControl.AcceptanceTesting: the HTTP helpers and assertion helpers take and forward optional tokens. The scenario poll loop captured cancellation as a test failure on shutdown, so it now swallows its own cancellation and holds the linked token in a local rather than reading a disposed source. The project block is narrowed to the types that sit on NServiceBus.AcceptanceTesting extension points, which drive them without a token. ServiceControl.Audit.Persistence.RavenDB: the PS0020 carve-out on LicenseStatusCheck moves from the project editorconfig to an inline pragma. The filter is deliberate, since the try runs on a linked token that is always cancelled in the timeout case being mapped.
Propagate cancellation tokens through the installer engine and the Config app
…icensing Propagate cancellation tokens through persistence dialects, licensing and acceptance test infrastructure
Retires the PS0003, PS0006, PS0008, PS0017 and PS0019 debt across the test projects, and corrects the justification on what is left. Phase 7 of the propagation work. Stacked on the persistence and licensing change, whose optional parameters this depends on. Because the production tokens are now optional, 139 call sites that passed the default literal do not need CancellationToken.None in its place: they drop the argument entirely and read as ordinary calls. Tests that genuinely exercise cancellation keep using [Test, CancelAfter(...)] with the token NUnit injects. Test methods that take that injected token gain "= default", which is what PS0003 asks for on a non-private member and does not affect NUnit's injection. The remaining PS0018 and PS0013 blocks are reworded. The previous text claimed these were accepted exceptions because the rule would demand a token on every [Test] method. That is not what happens: Particular.Analyzers already exempts NUnit test methods, and measuring it showed 301 of the 306 sites are ordinary helper methods. The blocks now say what is actually left, which is threading tokens through the helper chain and the callback shapes the NServiceBus scenario API fixes, so it reads as scheduled work rather than a settled decision. Two shared container helpers rename ct to cancellationToken and rethrow their own cancellation before mapping container start failures. WatchdogTests does the same around its expected-exception assertion.
ListInstancesViewModel.AddAndRemoveInstances was a genuine async void, not an event handler. It returned at its first await, so HandleAsync(RefreshInstances) published PostRefreshInstances while the removals were still in flight. That is the exact ordering the method's own remarks say must not happen: deleting an instance in PowerShell could then error out a deleted instance viewmodel trying to refresh itself. It is now async Task and awaited. The only thing that kept it async void was the constructor call site, which cannot await; that call moves to RxScreen.OnInitialize, which takes a token and is awaited by IActivate.ActivateAsync. The list therefore populates on activation rather than construction. The CorruptInstanceConfiguration specs construct the viewmodel directly, so they now activate it before asserting. Deferred since Phase 0 of the cancellation work, and unblocked by OnInitialize gaining a CancellationToken.
Drop redundant cancellation token arguments from the test projects
Await the deployed instances refresh instead of firing it as async void
… instead of fixed delay
…ceControl into flaky-test-5747
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Background service work runs asynchronously on the thread pool. Waiting a fixed duration of 2 seconds before asserting is flaky on slower/loaded CI machines because the work may not have completed yet. Instead, this PR polls for the expected condition until it is met or a generous timeout elapses.