tests: Add unit tests for EF and SQL Server outbox components#104
Merged
tests: Add unit tests for EF and SQL Server outbox components#104
Conversation
samtrion
added
state:ready for merge
Contributor
WalkthroughAdded centralized package version entries for Microsoft.EntityFrameworkCore.InMemory across target-framework–scoped ItemGroups and introduced multiple new unit test projects and test suites for EntityFramework and SqlServer outbox implementations; adjusted two existing tests and updated the solution file to include the new test projects. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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. Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
tests/NetEvolve.Pulse.EntityFramework.Tests.Unit/NetEvolve.Pulse.EntityFramework.Tests.Unit.csproj (1)
4-4: Consider multi-targeting to match other test projects.This project targets only
net10.0, while other test projects use$(_TestTargetFrameworks)(net8.0;net9.0;net10.0). This is likely becauseMicrosoft.EntityFrameworkCore.InMemoryis only defined for .NET 10 inDirectory.Packages.props.If you want this test suite to run on all target frameworks, add
InMemorypackage versions for .NET 8 and .NET 9 inDirectory.Packages.props, then change this to:- <TargetFramework>net10.0</TargetFramework> + <TargetFrameworks>$(_TestTargetFrameworks)</TargetFrameworks>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/NetEvolve.Pulse.EntityFramework.Tests.Unit/NetEvolve.Pulse.EntityFramework.Tests.Unit.csproj` at line 4, The project currently sets a single TargetFramework element (TargetFramework) to net10.0; change it to multi-targeting by replacing TargetFramework with TargetFrameworks and set its value to the shared property $(_TestTargetFrameworks), and ensure Directory.Packages.props includes Microsoft.EntityFrameworkCore.InMemory package entries (versions) for net8.0 and net9.0 so the package is available for all target frameworks; update Directory.Packages.props to add those InMemory version entries before switching the project file's TargetFramework to TargetFrameworks.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@tests/NetEvolve.Pulse.EntityFramework.Tests.Unit/NetEvolve.Pulse.EntityFramework.Tests.Unit.csproj`:
- Around line 1-24: Add the two missing test projects to the solution so CI
builds them: open the solution file and add project entries for the
tests/NetEvolve.Pulse.EntityFramework.Tests.Unit (project name
NetEvolve.Pulse.EntityFramework.Tests.Unit) and
tests/NetEvolve.Pulse.SqlServer.Tests.Unit (project name
NetEvolve.Pulse.SqlServer.Tests.Unit); ensure the Project GUID entries and
relative paths point to the existing .csproj files in /tests/, and save the
updated Pulse.slnx so both projects appear under the /tests/ folder in the
solution hierarchy and are included in build/test runs.
In
`@tests/NetEvolve.Pulse.SqlServer.Tests.Unit/NetEvolve.Pulse.SqlServer.Tests.Unit.csproj`:
- Line 9: The tests project added a PackageReference for "Moq" while
ManagePackageVersionsCentrally is enabled; add a corresponding PackageVersion
entry for the Moq package into Directory.Packages.props so the central package
management can resolve the version. Locate the PackageReference Include="Moq" in
the NetEvolve.Pulse.SqlServer.Tests.Unit.csproj and add a <PackageVersion>
element for the same package ID (e.g., Moq) in Directory.Packages.props,
ensuring the version string is set consistently with other centrally managed
packages.
In
`@tests/NetEvolve.Pulse.SqlServer.Tests.Unit/SqlServerMediatorConfiguratorExtensionsTests.cs`:
- Around line 22-31: The test
AddSqlServerOutbox_WithNullConnectionString_ThrowsArgumentNullException
incorrectly expects ArgumentNullException; update it to expect ArgumentException
to match the production behavior in
SqlServerMediatorConfiguratorExtensions.AddSqlServerOutbox which calls
ArgumentException.ThrowIfNullOrWhiteSpace(connectionString). Change the
assertion in the test to assert Throws<ArgumentException>() so it aligns with
the existing empty-string and whitespace tests.
---
Nitpick comments:
In
`@tests/NetEvolve.Pulse.EntityFramework.Tests.Unit/NetEvolve.Pulse.EntityFramework.Tests.Unit.csproj`:
- Line 4: The project currently sets a single TargetFramework element
(TargetFramework) to net10.0; change it to multi-targeting by replacing
TargetFramework with TargetFrameworks and set its value to the shared property
$(_TestTargetFrameworks), and ensure Directory.Packages.props includes
Microsoft.EntityFrameworkCore.InMemory package entries (versions) for net8.0 and
net9.0 so the package is available for all target frameworks; update
Directory.Packages.props to add those InMemory version entries before switching
the project file's TargetFramework to TargetFrameworks.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 092fe4e1-cebe-46a4-8201-1cbec5ea2804
📒 Files selected for processing (13)
Directory.Packages.propstests/NetEvolve.Pulse.EntityFramework.Tests.Unit/EntityFrameworkEventOutboxTests.cstests/NetEvolve.Pulse.EntityFramework.Tests.Unit/EntityFrameworkMediatorConfiguratorExtensionsTests.cstests/NetEvolve.Pulse.EntityFramework.Tests.Unit/EntityFrameworkOutboxRepositoryTests.cstests/NetEvolve.Pulse.EntityFramework.Tests.Unit/EntityFrameworkOutboxTransactionScopeTests.cstests/NetEvolve.Pulse.EntityFramework.Tests.Unit/NetEvolve.Pulse.EntityFramework.Tests.Unit.csprojtests/NetEvolve.Pulse.EntityFramework.Tests.Unit/OutboxMessageConfigurationFactoryTests.cstests/NetEvolve.Pulse.EntityFramework.Tests.Unit/TestDbContext.cstests/NetEvolve.Pulse.SqlServer.Tests.Unit/NetEvolve.Pulse.SqlServer.Tests.Unit.csprojtests/NetEvolve.Pulse.SqlServer.Tests.Unit/SqlServerEventOutboxTests.cstests/NetEvolve.Pulse.SqlServer.Tests.Unit/SqlServerMediatorConfiguratorExtensionsTests.cstests/NetEvolve.Pulse.SqlServer.Tests.Unit/SqlServerOutboxRepositoryTests.cstests/NetEvolve.Pulse.SqlServer.Tests.Unit/SqlServerOutboxTransactionScopeTests.cs
...NetEvolve.Pulse.EntityFramework.Tests.Unit/NetEvolve.Pulse.EntityFramework.Tests.Unit.csproj
Show resolved
Hide resolved
tests/NetEvolve.Pulse.SqlServer.Tests.Unit/NetEvolve.Pulse.SqlServer.Tests.Unit.csproj
Outdated
Show resolved
Hide resolved
tests/NetEvolve.Pulse.SqlServer.Tests.Unit/SqlServerMediatorConfiguratorExtensionsTests.cs
Show resolved
Hide resolved
Introduce new test projects for NetEvolve.Pulse.EntityFramework and NetEvolve.Pulse.SqlServer, covering constructor validation, service registration, options configuration, and method argument checks. Update Directory.Packages.props to include EF Core InMemory for .NET 10.0. These tests ensure correct DI setup and robustness of outbox/event infrastructure.
samtrion
force-pushed
the
tests/extended-tests
branch
from
453a0c4 to
1bfc7a0
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #104 +/- ##
==========================================
+ Coverage 84.88% 86.04% +1.15%
==========================================
Files 42 42
Lines 1297 1297
Branches 117 117
==========================================
+ Hits 1101 1116 +15
+ Misses 157 144 -13
+ Partials 39 37 -2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
- Add Microsoft.EntityFrameworkCore.InMemory for in-memory EF tests across .NET 8, 9, and 10 - Add new unit test projects for EF and SQL Server to the solution - Enable multi-targeting in EF unit test project - Remove unused Moq reference from SQL Server unit test project - Update all SQL Server test connection strings to use Encrypt=true; - Refactor test methods to use extension method syntax and improve clarity - Minor code cleanups for maintainability
Extended the delay after starting the service in the test from 100ms to 300ms to ensure the first polling cycle, including batch failure handling and marking completion, has sufficient time to complete. This improves test reliability without altering core logic.
Updated the Polly event policy configuration in tests to use a 300ms timeout for TimeoutEvent instead of 100ms, ensuring more lenient timing for event handling scenarios.
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
tests/NetEvolve.Pulse.SqlServer.Tests.Unit/SqlServerMediatorConfiguratorExtensionsTests.cs (1)
22-30:⚠️ Potential issue | 🟡 MinorTest expects
ArgumentNullExceptionbut production throwsArgumentExceptionfor null connection string.The production method
AddSqlServerOutboxusesArgumentException.ThrowIfNullOrWhiteSpace(connectionString)(seeSqlServerMediatorConfiguratorExtensions.cs:48), which throwsArgumentExceptionfor null input—notArgumentNullException. This is inconsistent with the empty-string and whitespace tests (lines 33-46) that correctly expectArgumentException.🐛 Proposed fix
[Test] - public async Task AddSqlServerOutbox_WithNullConnectionString_ThrowsArgumentNullException() => + public async Task AddSqlServerOutbox_WithNullConnectionString_ThrowsArgumentException() => _ = await Assert .That(() => SqlServerMediatorConfiguratorExtensions.AddSqlServerOutbox( new MediatorConfiguratorStub(), (string)null! ) ) - .Throws<ArgumentNullException>(); + .Throws<ArgumentException>();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/NetEvolve.Pulse.SqlServer.Tests.Unit/SqlServerMediatorConfiguratorExtensionsTests.cs` around lines 22 - 30, The test AddSqlServerOutbox_WithNullConnectionString_ThrowsArgumentNullException is asserting ArgumentNullException but production SqlServerMediatorConfiguratorExtensions.AddSqlServerOutbox uses ArgumentException.ThrowIfNullOrWhiteSpace so it throws ArgumentException for null; update the test to expect ArgumentException instead (change the .Throws generic from ArgumentNullException to ArgumentException) while keeping the same call to SqlServerMediatorConfiguratorExtensions.AddSqlServerOutbox and MediatorConfiguratorStub.
🧹 Nitpick comments (2)
tests/NetEvolve.Pulse.EntityFramework.Tests.Unit/EntityFrameworkMediatorConfiguratorExtensionsTests.cs (1)
86-99: Consider addingAddOutbox()for consistency with other tests.This test calls
AddEntityFrameworkOutbox<TestDbContext>()directly without first callingAddOutbox(), unlike the other registration tests (lines 39, 57, 75, 109, 125). While the test passes becauseTimeProvideris registered viaTryAddSingleton, the inconsistency could mask issues ifAddOutbox()ever registers dependencies that affectTimeProviderbehavior.🔧 Suggested fix for consistency
[Test] public async Task AddEntityFrameworkOutbox_RegistersTimeProviderAsSingleton() { var services = new ServiceCollection(); - _ = services.AddPulse(config => config.AddEntityFrameworkOutbox<TestDbContext>()); + _ = services.AddDbContext<TestDbContext>(o => + o.UseInMemoryDatabase(nameof(AddEntityFrameworkOutbox_RegistersTimeProviderAsSingleton)) + ); + _ = services.AddPulse(config => config.AddOutbox().AddEntityFrameworkOutbox<TestDbContext>()); var descriptor = services.FirstOrDefault(d => d.ServiceType == typeof(TimeProvider));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/NetEvolve.Pulse.EntityFramework.Tests.Unit/EntityFrameworkMediatorConfiguratorExtensionsTests.cs` around lines 86 - 99, Update the test AddEntityFrameworkOutbox_RegistersTimeProviderAsSingleton to call AddOutbox() on the ServiceCollection before calling AddEntityFrameworkOutbox<TestDbContext>() so the registration path matches the other tests; specifically, modify the setup so services.AddPulse(config => { config.AddOutbox(); config.AddEntityFrameworkOutbox<TestDbContext>(); }) (or equivalent two-step call) to ensure TimeProvider is registered via the same initialization flow used in the other registration tests.tests/NetEvolve.Pulse.SqlServer.Tests.Unit/SqlServerEventOutboxTests.cs (1)
50-63: Test name implies transaction usage but passesnull.The test
Constructor_WithTransaction_CreatesInstanceexplicitly passestransaction: null, so it doesn't actually verify behavior with an activeSqlTransaction. Consider renaming toConstructor_WithNullTransaction_CreatesInstancefor clarity, or create an actual transaction to test the parameter path (though the latter would require an open connection and may be better suited for integration tests).🔧 Suggested rename for clarity
[Test] - public async Task Constructor_WithTransaction_CreatesInstance() + public async Task Constructor_WithNullTransaction_CreatesInstance() { using var connection = new SqlConnection("Server=.;Encrypt=true;"); var outbox = new SqlServerEventOutbox( connection, Options.Create(new OutboxOptions()), TimeProvider.System, transaction: null ); _ = await Assert.That(outbox).IsNotNull(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/NetEvolve.Pulse.SqlServer.Tests.Unit/SqlServerEventOutboxTests.cs` around lines 50 - 63, The test method Constructor_WithTransaction_CreatesInstance currently passes transaction: null so it doesn't exercise the transactional path; either rename the test to Constructor_WithNullTransaction_CreatesInstance to reflect the actual scenario, or update the test to open the SqlConnection and call BeginTransaction() then pass that SqlTransaction into the SqlServerEventOutbox constructor to truly validate the transaction path (refer to the test method Constructor_WithTransaction_CreatesInstance and the SqlServerEventOutbox constructor's transaction parameter when making the change).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@tests/NetEvolve.Pulse.SqlServer.Tests.Unit/SqlServerMediatorConfiguratorExtensionsTests.cs`:
- Around line 22-30: The test
AddSqlServerOutbox_WithNullConnectionString_ThrowsArgumentNullException is
asserting ArgumentNullException but production
SqlServerMediatorConfiguratorExtensions.AddSqlServerOutbox uses
ArgumentException.ThrowIfNullOrWhiteSpace so it throws ArgumentException for
null; update the test to expect ArgumentException instead (change the .Throws
generic from ArgumentNullException to ArgumentException) while keeping the same
call to SqlServerMediatorConfiguratorExtensions.AddSqlServerOutbox and
MediatorConfiguratorStub.
---
Nitpick comments:
In
`@tests/NetEvolve.Pulse.EntityFramework.Tests.Unit/EntityFrameworkMediatorConfiguratorExtensionsTests.cs`:
- Around line 86-99: Update the test
AddEntityFrameworkOutbox_RegistersTimeProviderAsSingleton to call AddOutbox() on
the ServiceCollection before calling AddEntityFrameworkOutbox<TestDbContext>()
so the registration path matches the other tests; specifically, modify the setup
so services.AddPulse(config => { config.AddOutbox();
config.AddEntityFrameworkOutbox<TestDbContext>(); }) (or equivalent two-step
call) to ensure TimeProvider is registered via the same initialization flow used
in the other registration tests.
In `@tests/NetEvolve.Pulse.SqlServer.Tests.Unit/SqlServerEventOutboxTests.cs`:
- Around line 50-63: The test method Constructor_WithTransaction_CreatesInstance
currently passes transaction: null so it doesn't exercise the transactional
path; either rename the test to Constructor_WithNullTransaction_CreatesInstance
to reflect the actual scenario, or update the test to open the SqlConnection and
call BeginTransaction() then pass that SqlTransaction into the
SqlServerEventOutbox constructor to truly validate the transaction path (refer
to the test method Constructor_WithTransaction_CreatesInstance and the
SqlServerEventOutbox constructor's transaction parameter when making the
change).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 705a6a78-2891-49d0-83fa-5dae96ff2508
📒 Files selected for processing (16)
Directory.Packages.propsPulse.slnxtests/NetEvolve.Pulse.EntityFramework.Tests.Unit/EntityFrameworkEventOutboxTests.cstests/NetEvolve.Pulse.EntityFramework.Tests.Unit/EntityFrameworkMediatorConfiguratorExtensionsTests.cstests/NetEvolve.Pulse.EntityFramework.Tests.Unit/EntityFrameworkOutboxRepositoryTests.cstests/NetEvolve.Pulse.EntityFramework.Tests.Unit/EntityFrameworkOutboxTransactionScopeTests.cstests/NetEvolve.Pulse.EntityFramework.Tests.Unit/NetEvolve.Pulse.EntityFramework.Tests.Unit.csprojtests/NetEvolve.Pulse.EntityFramework.Tests.Unit/OutboxMessageConfigurationFactoryTests.cstests/NetEvolve.Pulse.EntityFramework.Tests.Unit/TestDbContext.cstests/NetEvolve.Pulse.Polly.Tests.Integration/PollyEventInterceptorInMemoryTests.cstests/NetEvolve.Pulse.SqlServer.Tests.Unit/NetEvolve.Pulse.SqlServer.Tests.Unit.csprojtests/NetEvolve.Pulse.SqlServer.Tests.Unit/SqlServerEventOutboxTests.cstests/NetEvolve.Pulse.SqlServer.Tests.Unit/SqlServerMediatorConfiguratorExtensionsTests.cstests/NetEvolve.Pulse.SqlServer.Tests.Unit/SqlServerOutboxRepositoryTests.cstests/NetEvolve.Pulse.SqlServer.Tests.Unit/SqlServerOutboxTransactionScopeTests.cstests/NetEvolve.Pulse.Tests.Unit/Outbox/OutboxProcessorHostedServiceTests.cs
✅ Files skipped from review due to trivial changes (6)
- Pulse.slnx
- Directory.Packages.props
- tests/NetEvolve.Pulse.SqlServer.Tests.Unit/NetEvolve.Pulse.SqlServer.Tests.Unit.csproj
- tests/NetEvolve.Pulse.EntityFramework.Tests.Unit/EntityFrameworkOutboxTransactionScopeTests.cs
- tests/NetEvolve.Pulse.EntityFramework.Tests.Unit/NetEvolve.Pulse.EntityFramework.Tests.Unit.csproj
- tests/NetEvolve.Pulse.EntityFramework.Tests.Unit/OutboxMessageConfigurationFactoryTests.cs
🚧 Files skipped from review as they are similar to previous changes (5)
- tests/NetEvolve.Pulse.EntityFramework.Tests.Unit/TestDbContext.cs
- tests/NetEvolve.Pulse.EntityFramework.Tests.Unit/EntityFrameworkEventOutboxTests.cs
- tests/NetEvolve.Pulse.EntityFramework.Tests.Unit/EntityFrameworkOutboxRepositoryTests.cs
- tests/NetEvolve.Pulse.SqlServer.Tests.Unit/SqlServerOutboxTransactionScopeTests.cs
- tests/NetEvolve.Pulse.SqlServer.Tests.Unit/SqlServerOutboxRepositoryTests.cs
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
1 participant
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.
Introduce new test projects for NetEvolve.Pulse.EntityFramework and NetEvolve.Pulse.SqlServer, covering constructor validation, service registration, options configuration, and method argument checks. Update Directory.Packages.props to include EF Core InMemory for .NET 10.0. These tests ensure correct DI setup and robustness of outbox/event infrastructure.
Summary by CodeRabbit
Tests
Chores