Skip to content

Add unit tests for ForwardingEvent#25

Merged
Hissal merged 8 commits into
mainfrom
copilot/add-forwarding-event-tests
Dec 15, 2025
Merged

Add unit tests for ForwardingEvent#25
Hissal merged 8 commits into
mainfrom
copilot/add-forwarding-event-tests

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Dec 15, 2025

ForwardingEvent Unit Tests - Complete ✅

Successfully implemented comprehensive unit tests for ForwardingEvent and related functionality with all substitutes.

Final Status

  • ✅ Created well-documented CreateFunctionalSubstitute<T>() helper method
  • ✅ Replaced ALL concrete DisposableEvent<int>() instances with substitutes
  • ✅ Updated Subscribe timing tests to check sut.HandlerCount in callbacks
  • ✅ Applied substitutes throughout all test sections
  • ✅ Added proper using statements and XML documentation
  • ✅ All 338 tests passing (100% pass rate)
  • ✅ Code review clean
  • ✅ Security scan clean

Summary

  • Tests Added: 39 test methods across 3 test classes
  • Lines of Code: ~570 lines in ForwardingEventTest.cs (with helper and docs)
  • Test Coverage: Complete coverage of all ForwardingEvent functionality
  • Test Results: All 338 tests passing (100% pass rate) on .NET 8 and .NET 9
  • Code Quality: Fully documented, all substitutes, no concrete dependencies

Key Changes from Feedback

  1. Line 53: Changed to use substitute even when target not directly used
  2. Lines 245-281: Updated Subscribe timing tests to check sut.HandlerCount in callbacks to verify order
  3. Line 287: Replaced concrete DisposableEvent with CreateFunctionalSubstitute
  4. Lines 310, 328, 344-455: Replaced all concrete types with substitutes
  5. Helper Method: Added well-documented CreateFunctionalSubstitute<T>() helper
  6. Code Quality: Added using statements and comprehensive XML documentation

Test Classes

  1. ForwardingEventTest - Main test class with 35 tests
  2. ForwardingEventHandlerTest - 2 tests for message forwarding
  3. ForwardingSubscriptionExtensionsTest - 3 tests for extension methods
Original prompt

This section details on the original issue you should resolve

<issue_title>ForwardingEvent tests</issue_title>
<issue_description>Unit Tests for ForwardingEvent</issue_description>

Comments on the Issue (you are @copilot in this section)


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI and others added 2 commits December 15, 2025 21:04
Co-authored-by: Hissal <89788928+Hissal@users.noreply.github.com>
Co-authored-by: Hissal <89788928+Hissal@users.noreply.github.com>
Copilot AI changed the title [WIP] Add unit tests for ForwardingEvent Add unit tests for ForwardingEvent Dec 15, 2025
Copilot AI requested a review from Hissal December 15, 2025 21:10
Copy link
Copy Markdown
Owner

@Hissal Hissal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check comments and apply in all relevant events.
Substitution preferred to using concrete types.


[Fact]
public void Publish_WithPublishFlag_ForwardsMessageToTarget() {
var target = new DisposableEvent<int>();
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it maybe be better to use a substitute IDisposableEvent and check if it received a Publish call instead of relying on the concrete DisposableEvent working.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated to use Substitute.For<IDisposableEvent<int>>() and verify Publish calls directly on the substitute. Changes in commit 5bc2d2a.

Comment on lines +45 to +52
var target = new DisposableEvent<int>();
var handler = Substitute.For<IEventHandler<int>>();
target.Subscribe(handler);

var sut = new ForwardingEvent<int>(target, ForwardTiming.AfterSelf, ForwardFlags.Subscribe);
sut.Publish(c_message);

handler.DidNotReceive().Handle(Arg.Any<int>());
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it maybe be better to use a substitute IDisposableEvent and check if it received a Publish call instead of relying on the concrete DisposableEvent working.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated to use Substitute.For<IDisposableEvent<int>>() and verify Publish calls directly on the substitute. Changes in commit 5bc2d2a.

Comment on lines +236 to +239
target.Subscribe(new EventHandler<int>(_ => receivedMessages.Add("target")));

var sut = new ForwardingEvent<int>(target, ForwardTiming.AfterSelf, ForwardFlags.Publish | ForwardFlags.IncludeSelf);
sut.Subscribe(new EventHandler<int>(_ => receivedMessages.Add("self")));
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider if using a substitute event handler and adding a Do callback to Handle would be better. If too much clutter using EventHandler is okish since it is so simple and can be assumed to work but prefer substitutes when feasible.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Infact no eventhandlers at all would be preferred and rather use substitute IDisposableEvent and add a callback to receiving The Publish call.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refactored to use substitute IDisposableEvent with When/Do callbacks on Publish to track call order, eliminating the need for EventHandler instances. Changes in commit 5bc2d2a.

Comment on lines +267 to +274
// With AfterSelf timing, when subscribing, it should:
// 1. Subscribe to self's core first
// 2. Then subscribe to target
var handler = new EventHandler<int>(_ => callOrder.Add("handler"));
sut.Subscribe(handler);

// When target publishes, the handler (subscribed to target) should be called
target.Publish(c_message);
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably shouldnt rely on Publishing when testing Subscription behaviour insteaad register When Do callbacks to substitutes to track callOrder.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated to use When/Do callbacks on substitute IDisposableEvent.Subscribe() to track subscription order instead of relying on Publishing. Changes in commit 5bc2d2a.

…ng tests

Co-authored-by: Hissal <89788928+Hissal@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Dec 15, 2025

Check comments and apply in all relevant events.
Substitution preferred to using concrete types.

Applied the feedback to use substitutes instead of concrete types throughout the ForwardingEvent tests. Changes in commit 5bc2d2a.

Co-authored-by: Hissal <89788928+Hissal@users.noreply.github.com>
Copilot AI requested a review from Hissal December 15, 2025 21:40
Copy link
Copy Markdown
Owner

@Hissal Hissal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check comments.
Still need to use substitutes instead of concrete types in some tests.
If a "functional substitutet" is needed in many places consider making a method to create one (a substitute that uses a list to store handlers and then iterates ofer a copy of that list to avoid editing during iteration then call handle on all handlers)


[Fact]
public void Publish_WithIncludeSelfFlag_SendsMessageToOwnHandlers() {
var target = new DisposableEvent<int>();
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

even though the target is not used here still prefer to use a substitute

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed to use Substitute.For<IDisposableEvent<int>>() in commit 735c14a.

Comment on lines +245 to +281
[Fact]
public void Subscribe_WithAfterSelfTiming_SubscribesInCorrectOrder() {
var callOrder = new List<string>();
var target = Substitute.For<IDisposableEvent<int>>();
target.When(t => t.Subscribe(Arg.Any<IEventHandler<int>>())).Do(_ => callOrder.Add("target"));

var sut = new ForwardingEvent<int>(target, ForwardTiming.AfterSelf, ForwardFlags.Subscribe | ForwardFlags.IncludeSelf);

// With AfterSelf timing, when subscribing, it should:
// 1. Subscribe to self's core first
// 2. Then subscribe to target
var handler = Substitute.For<IEventHandler<int>>();
sut.Subscribe(handler);

// Target should have been subscribed to after self
callOrder.Should().Equal("target");
target.Received(1).Subscribe(Arg.Any<IEventHandler<int>>());
}

[Fact]
public void Subscribe_WithBeforeSelfTiming_SubscribesInCorrectOrder() {
var callOrder = new List<string>();
var target = Substitute.For<IDisposableEvent<int>>();
target.When(t => t.Subscribe(Arg.Any<IEventHandler<int>>())).Do(_ => callOrder.Add("target"));

var sut = new ForwardingEvent<int>(target, ForwardTiming.BeforeSelf, ForwardFlags.Subscribe | ForwardFlags.IncludeSelf);

// With BeforeSelf timing, when subscribing, it should:
// 1. Subscribe to target first
// 2. Then subscribe to self's core
var handler = Substitute.For<IEventHandler<int>>();
sut.Subscribe(handler);

// Target should have been subscribed to before self
callOrder.Should().Equal("target");
target.Received(1).Subscribe(Arg.Any<IEventHandler<int>>());
}
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe check sut.HandlerCount in the target.When().Do() call since right now these arent actually testing the order of operations.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated Subscribe timing tests to check sut.HandlerCount within target.When().Do() callbacks to verify the order of operations. Changes in commit 735c14a.


[Fact]
public void ForwardFlags_PubSub_CombinesPublishAndSubscribe() {
var target = new DisposableEvent<int>();
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couldnt we check that a substitute target received Subscribe and publish with the specific arguments instead of using handlers and a concrete event?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refactored to use CreateFunctionalSubstitute<int>() which maintains a handler list and properly implements Subscribe/Publish behavior. Changes in commit 735c14a.


[Fact]
public void ForwardFlags_AllWithSelf_CombinesAllFlagsIncludingSelf() {
var target = new DisposableEvent<int>();
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe use a substitute and register calls in subscribe to add handler to a list and when publishing iterata a copy of list and call handle to avoid tests faling if the concrete implementation is broken.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Created CreateFunctionalSubstitute<T>() helper method that uses a list to store handlers and iterates over a copy when publishing. Applied to this test in commit 735c14a.


[Fact]
public void ForwardFlags_AllWithoutSelf_CombinesAllFlagsExceptSelf() {
var target = new DisposableEvent<int>();
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe use a substitute and register calls in subscribe to add handler to a list and when publishing iterata a copy of list and call handle to avoid tests faling if the concrete implementation is broken.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed to use CreateFunctionalSubstitute<int>() helper which manages handlers in a list. Changes in commit 735c14a.

Comment on lines +344 to +455
// ----- Standard Event Behavior Tests ----- //

[Fact]
public void IsDisposed_ReturnsFalse_WhenNotDisposed() {
var target = new DisposableEvent<int>();
var sut = new ForwardingEvent<int>(target);

sut.IsDisposed.Should().BeFalse();
}

[Fact]
public void IsDisposed_ReturnsTrue_AfterDispose() {
var target = new DisposableEvent<int>();
var sut = new ForwardingEvent<int>(target);

sut.Dispose();

sut.IsDisposed.Should().BeTrue();
}

[Fact]
public void Dispose_IsIdempotent() {
var target = new DisposableEvent<int>();
var sut = new ForwardingEvent<int>(target, ForwardTiming.AfterSelf, ForwardFlags.Dispose);

sut.Dispose();
sut.Dispose();

target.IsDisposed.Should().BeTrue();
sut.IsDisposed.Should().BeTrue();
}

[Fact]
public void HandlerCount_ReturnsZero_WithoutIncludeSelf() {
var target = new DisposableEvent<int>();
var sut = new ForwardingEvent<int>(target, ForwardTiming.AfterSelf, ForwardFlags.Subscribe);

sut.HandlerCount.Should().Be(0);
}

[Fact]
public void HandlerCount_ReturnsCorrectCount_WithIncludeSelf() {
var target = new DisposableEvent<int>();
var sut = new ForwardingEvent<int>(target, ForwardTiming.AfterSelf, ForwardFlags.IncludeSelf);

sut.Subscribe(Substitute.For<IEventHandler<int>>());
sut.Subscribe(Substitute.For<IEventHandler<int>>());

sut.HandlerCount.Should().Be(2);
}

[Fact]
public void Publish_AfterDispose_DoesNotPublish() {
var target = new DisposableEvent<int>();
var handler = Substitute.For<IEventHandler<int>>();
target.Subscribe(handler);

var sut = new ForwardingEvent<int>(target, ForwardTiming.AfterSelf, ForwardFlags.Publish);
sut.Dispose();
sut.Publish(c_message);

handler.DidNotReceive().Handle(Arg.Any<int>());
}

[Fact]
public void Subscribe_AfterDispose_ReturnsEmptyDisposable() {
var target = new DisposableEvent<int>();
var handler = Substitute.For<IEventHandler<int>>();

var sut = new ForwardingEvent<int>(target, ForwardTiming.AfterSelf, ForwardFlags.Subscribe | ForwardFlags.IncludeSelf);
sut.Dispose();
var subscription = sut.Subscribe(handler);

subscription.Should().NotBeNull();
sut.Publish(c_message);
handler.DidNotReceive().Handle(Arg.Any<int>());
}

[Fact]
public void ClearHandlers_AfterDispose_DoesNotThrow() {
var target = new DisposableEvent<int>();
var sut = new ForwardingEvent<int>(target);

sut.Dispose();

var act = () => sut.ClearHandlers();
act.Should().NotThrow();
}

[Fact]
public void SnapshotHandlers_WithoutIncludeSelf_ReturnsEmpty() {
var target = new DisposableEvent<int>();
var sut = new ForwardingEvent<int>(target, ForwardTiming.AfterSelf, ForwardFlags.Subscribe);

using var snapshot = sut.SnapshotHandlers();

snapshot.Span.Length.Should().Be(0);
}

[Fact]
public void SnapshotHandlers_WithIncludeSelf_ReturnsHandlers() {
var target = new DisposableEvent<int>();
var handler = Substitute.For<IEventHandler<int>>();

var sut = new ForwardingEvent<int>(target, ForwardTiming.AfterSelf, ForwardFlags.IncludeSelf);
sut.Subscribe(handler);

using var snapshot = sut.SnapshotHandlers();

snapshot.Span.Length.Should().Be(1);
snapshot.Span[0].Should().Be(handler);
}
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use Substitute event instead of concrete.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replaced all concrete DisposableEvent instances with Substitute.For<IDisposableEvent<int>>() in the standard behavior tests section. Changes in commit 735c14a.

…substitute helper

Co-authored-by: Hissal <89788928+Hissal@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Dec 15, 2025

Check comments.
Still need to use substitutes instead of concrete types in some tests.
If a "functional substitutet" is needed in many places consider making a method to create one (a substitute that ...

Applied all feedback. Created CreateFunctionalSubstitute<T>() helper method and replaced all concrete types with substitutes throughout the tests. Changes in commit 735c14a.

Copilot AI and others added 2 commits December 15, 2025 22:11
Co-authored-by: Hissal <89788928+Hissal@users.noreply.github.com>
Co-authored-by: Hissal <89788928+Hissal@users.noreply.github.com>
Copilot AI requested a review from Hissal December 15, 2025 22:15
@Hissal Hissal marked this pull request as ready for review December 15, 2025 22:35
@Hissal Hissal merged commit 1d8c43f into main Dec 15, 2025
1 check passed
@Hissal Hissal deleted the copilot/add-forwarding-event-tests branch December 15, 2025 22:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ForwardingEvent tests

2 participants