Skip to content

Fix WhenOnDisposeAsyncSourceFails_ThenCallbackInvoked to actually assert callback invocation#128

Merged
ChrisPulman merged 2 commits intoCP_UpdatePackagesfrom
copilot/sub-pr-126
Feb 28, 2026
Merged

Fix WhenOnDisposeAsyncSourceFails_ThenCallbackInvoked to actually assert callback invocation#128
ChrisPulman merged 2 commits intoCP_UpdatePackagesfrom
copilot/sub-pr-126

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Feb 28, 2026

The test WhenOnDisposeAsyncSourceFails_ThenCallbackInvoked was asserting only on the completion result, not on whether the OnDispose callback itself was ever called — meaning the test would pass even if OnDispose was never invoked.

Change

Added a disposeCallbackInvoked flag set inside the OnDispose lambda, with an explicit assertion that it becomes true:

var disposeCallbackInvoked = false;
var source = ObservableAsync.Throw<int>(new InvalidOperationException("fail"))
    .OnDispose(() =>
    {
        disposeCallbackInvoked = true;
        return default;
    });

// ...

await Assert.That(completionResult!.Value.IsFailure).IsTrue();
await Assert.That(disposeCallbackInvoked).IsTrue(); // ← added

🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.

…Fails_ThenCallbackInvoked test

Co-authored-by: ChrisPulman <4910015+ChrisPulman@users.noreply.github.com>
Copilot AI changed the title [WIP] Update async library based on review feedback Fix WhenOnDisposeAsyncSourceFails_ThenCallbackInvoked to actually assert callback invocation Feb 28, 2026
@ChrisPulman ChrisPulman marked this pull request as ready for review February 28, 2026 14:45
@ChrisPulman ChrisPulman merged commit f799074 into CP_UpdatePackages Feb 28, 2026
@ChrisPulman ChrisPulman deleted the copilot/sub-pr-126 branch February 28, 2026 14:46
ChrisPulman added a commit that referenced this pull request Feb 28, 2026
* Bump packages; fix async test subscriptions

Update package versions in Directory.Packages.props (Polyfill -> 9.12.0, Microsoft.NET.Test.Sdk -> 18.3.0, TUnit -> 1.17.54, Verify.TUnit -> 31.13.2, Microsoft.Testing.Extensions.CodeCoverage -> 18.5.1, etc.) and remove some older test package entries.

Refactor ReactiveExtensionsTests to reliably track and await async subscription handlers: replace inline async Subscribe callbacks with a tasks list and a local HandleAsync wrapper that disposes the sync token in a finally block, then await Task.WhenAll(tasks) to avoid race conditions and flakiness in the tests.

* Add scheduler support to SyncTimer and tests

Change SyncTimer to key timers by both TimeSpan and IScheduler, add an overload SyncTimer(TimeSpan, IScheduler) and keep a fallback to Scheduler.Default. Validate scheduler is non-null and create the underlying Observable.Timer with the provided scheduler so timers can be shared per scheduler. Update tests to use TestScheduler (pass it into SyncTimer and OnErrorRetry), and replace async wait helpers with scheduler.AdvanceBy to make timing deterministic.

* Modernize async library and test fixes

Apply modern C# updates and various test fixes across the async library. Highlights:
- Update .github documentation target frameworks and add RCS1047 to NoWarn in Directory.Build.props.
- Bump copyright years from 2019-2025 to 2019-2026 in many source files.
- Add conditional ArgumentNullException.ThrowIfNull / ThrowIfLessThan checks for .NET 8+ and use Lock on newer frameworks where appropriate to improve null-safety and reliability.
- Expand ConcurrentObserverCallsException into a full exception class with standard constructors.
- Minor API/implementation cleanups: suppress CA2000 where needed, simplify nullable/collection initializations, expression-bodied test methods, additional disposals in tests, and other small refactors to reduce analyzer warnings and improve robustness.

* Add ArgumentExceptionHelper and centralize null checks

Introduce Internal/ArgumentExceptionHelper with ThrowIfNull and ThrowIfNullOrEmpty helpers and replace scattered NET8/legacy null-check conditionals across the Async and Reactive extensions with calls to ArgumentExceptionHelper.ThrowIfNull. This centralizes argument validation (removing many #if NET8_0_OR_GREATER blocks), streamlines a few checks (e.g. Timeout and WaitForCondition), and reduces duplication across numerous operator/subject/observable implementations.

* Add [Serializable] and serialization constructor to ConcurrentObserverCallsException (#127)

* Initial plan

* Add [Serializable] attribute and serialization constructor to ConcurrentObserverCallsException

Co-authored-by: ChrisPulman <4910015+ChrisPulman@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: ChrisPulman <4910015+ChrisPulman@users.noreply.github.com>

* Move RCS1047 suppression to .editorconfig

Set dotnet_diagnostic.RCS1047.severity = none in .editorconfig and remove RCS1047 from the NoWarn list in src/Directory.Build.props. This centralizes the analyzer configuration in .editorconfig and avoids duplicate suppression in MSBuild.

* Fix WhenOnDisposeAsyncSourceFails_ThenCallbackInvoked to actually assert callback invocation (#128)

* Initial plan

* Add disposeCallbackInvoked flag assertion in WhenOnDisposeAsyncSourceFails_ThenCallbackInvoked test

Co-authored-by: ChrisPulman <4910015+ChrisPulman@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: ChrisPulman <4910015+ChrisPulman@users.noreply.github.com>

---------

Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
Co-authored-by: ChrisPulman <4910015+ChrisPulman@users.noreply.github.com>
@github-actions
Copy link
Copy Markdown

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Mar 15, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants