-
Notifications
You must be signed in to change notification settings - Fork 1
V9.0.0/support for async lifetime #19
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
8cca17b
a860320
9a2dcf0
6b17a01
45c3ba2
c4205b5
314adb9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,26 +5,37 @@ | |
|
|
||
| namespace Codebelt.Extensions.Xunit.Hosting | ||
| { | ||
| /// <summary> | ||
| /// Provides a way to use Microsoft Dependency Injection in unit tests. | ||
| /// </summary> | ||
| /// <seealso cref="IDisposable" /> | ||
| public interface IHostFixture : IServiceTest, IHostTest, IConfigurationTest, IHostingEnvironmentTest | ||
| { | ||
| #if NETSTANDARD2_0_OR_GREATER | ||
| public partial interface IHostFixture | ||
| { | ||
| /// <summary> | ||
| /// Gets or sets the delegate that adds configuration and environment information to a <see cref="HostTest{T}"/>. | ||
| /// </summary> | ||
| /// <value>The delegate that adds configuration and environment information to a <see cref="HostTest{T}"/>.</value> | ||
| Action<IConfiguration, IHostingEnvironment> ConfigureCallback { get; set; } | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Inconsistent use of 'IHostingEnvironment' and 'IHostEnvironment' in 'ConfigureCallback' The
This inconsistency may lead to compilation errors or unexpected behavior when switching target frameworks. Consider using #if NETSTANDARD2_0_OR_GREATER
public partial interface IHostFixture
{
//...
- Action<IConfiguration, IHostingEnvironment> ConfigureCallback { get; set; }
+ Action<IConfiguration, IHostEnvironment> ConfigureCallback { get; set; }
//...
}
#else
public partial interface IHostFixture : IAsyncDisposable
{
//...
Action<IConfiguration, IHostEnvironment> ConfigureCallback { get; set; }
//...
}
#endifAlso applies to: 34-34 |
||
| } | ||
| #else | ||
| public partial interface IHostFixture | ||
| { | ||
| /// <summary> | ||
| /// Gets or sets the delegate that adds configuration and environment information to a <see cref="HostTest{T}"/>. | ||
| /// </summary> | ||
| /// <value>The delegate that adds configuration and environment information to a <see cref="HostTest{T}"/>.</value> | ||
| Action<IConfiguration, IHostEnvironment> ConfigureCallback { get; set; } | ||
| } | ||
| #endif | ||
|
|
||
| /// <summary> | ||
| /// Provides a way to use Microsoft Dependency Injection in unit tests. | ||
| /// </summary> | ||
| /// <seealso cref="IServiceTest" /> | ||
| /// <seealso cref="IHostTest" /> | ||
| /// <seealso cref="IConfigurationTest" /> | ||
| /// <seealso cref="IHostingEnvironmentTest" /> | ||
| /// <seealso cref="IDisposable" /> | ||
| /// <seealso cref="IAsyncDisposable" /> | ||
| public partial interface IHostFixture : IServiceTest, IHostTest, IConfigurationTest, IHostingEnvironmentTest, IDisposable, IAsyncDisposable | ||
| { | ||
| /// <summary> | ||
| /// Gets or sets the delegate that adds services to the container. | ||
| /// </summary> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,8 @@ | ||
| using System; | ||
| using System.Linq; | ||
| using System.Text.RegularExpressions; | ||
| using System.Threading.Tasks; | ||
| using Xunit; | ||
| using Xunit.Abstractions; | ||
|
|
||
| namespace Codebelt.Extensions.Xunit | ||
|
|
@@ -9,8 +11,10 @@ namespace Codebelt.Extensions.Xunit | |
| /// Represents the base class from which all implementations of unit testing should derive. | ||
| /// </summary> | ||
| /// <seealso cref="ITestOutputHelper"/> | ||
| public abstract class Test : ITest | ||
| public abstract class Test : ITest, IAsyncLifetime | ||
| { | ||
| private readonly object _lock = new(); | ||
|
|
||
| /// <summary> | ||
| /// Provides a way, with wildcard support, to determine if <paramref name="actual" /> matches <paramref name="expected" />. | ||
| /// </summary> | ||
|
|
@@ -82,6 +86,14 @@ protected virtual void OnDisposeManagedResources() | |
| { | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Called when this object is being disposed by <see cref="DisposeAsync()"/>. | ||
| /// </summary> | ||
| protected virtual ValueTask OnDisposeManagedResourcesAsync() | ||
| { | ||
| return default; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Called when this object is being disposed by either <see cref="Dispose()"/> or <see cref="Dispose(bool)"/> and <see cref="Disposed"/> is <c>false</c>. | ||
| /// </summary> | ||
|
|
@@ -105,12 +117,42 @@ public void Dispose() | |
| protected void Dispose(bool disposing) | ||
| { | ||
| if (Disposed) { return; } | ||
| if (disposing) | ||
| lock (_lock) | ||
| { | ||
| OnDisposeManagedResources(); | ||
| if (Disposed) { return; } | ||
| if (disposing) | ||
| { | ||
| OnDisposeManagedResources(); | ||
| } | ||
| OnDisposeUnmanagedResources(); | ||
| Disposed = true; | ||
| } | ||
| OnDisposeUnmanagedResources(); | ||
| Disposed = true; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Asynchronously releases the resources used by the <see cref="Test"/>. | ||
| /// </summary> | ||
| /// <returns>A <see cref="ValueTask"/> that represents the asynchronous dispose operation.</returns> | ||
| /// <remarks>https://learn.microsoft.com/en-us/dotnet/standard/garbage-collection/implementing-disposeasync#the-disposeasync-method</remarks> | ||
| public async ValueTask DisposeAsync() | ||
| { | ||
| await OnDisposeManagedResourcesAsync().ConfigureAwait(false); | ||
| Dispose(false); | ||
| GC.SuppressFinalize(this); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Called immediately after the class has been created, before it is used. | ||
| /// </summary> | ||
| /// <returns>A <see cref="Task"/> that represents the asynchronous operation.</returns> | ||
| public virtual Task InitializeAsync() | ||
| { | ||
| return Task.CompletedTask; | ||
| } | ||
|
|
||
| Task IAsyncLifetime.DisposeAsync() | ||
| { | ||
| return DisposeAsync().AsTask(); | ||
|
Comment on lines
+132
to
+155
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Potential race condition in The To ensure thread safety, apply the same locking mechanism in public async ValueTask DisposeAsync()
{
+ if (Disposed) { return; }
+ lock (_lock)
+ {
+ if (Disposed) { return; }
await OnDisposeManagedResourcesAsync().ConfigureAwait(false);
Dispose(false);
GC.SuppressFinalize(this);
+ Disposed = true;
+ }
}This change synchronizes access to the disposal logic and updates the
|
||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,65 @@ | ||
| using System; | ||
| using Codebelt.Extensions.Xunit.Hosting.AspNetCore.Assets; | ||
| using Microsoft.AspNetCore.Builder; | ||
| using Xunit; | ||
| using Xunit.Abstractions; | ||
|
|
||
| namespace Codebelt.Extensions.Xunit.Hosting.AspNetCore | ||
| { | ||
| public class AspNetCoreHostFixtureTest : Test | ||
| { | ||
| public AspNetCoreHostFixtureTest(ITestOutputHelper output) : base(output) | ||
| { | ||
| } | ||
|
Comment on lines
+9
to
+13
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add tests for async lifecycle methods. Given that this PR introduces async lifecycle support (InitializeAsync, DisposeAsync), the test class should include tests that verify these new async capabilities of AspNetCoreHostFixture. Consider adding the following test cases:
Would you like me to help generate these async test implementations? |
||
|
|
||
| [Fact] | ||
| public void ConfigureHost_NullHostTest_ThrowsArgumentNullException() | ||
| { | ||
| // Arrange | ||
| var fixture = new AspNetCoreHostFixture(); | ||
|
|
||
| // Act & Assert | ||
| Assert.Throws<ArgumentNullException>(() => fixture.ConfigureHost(null)); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void ConfigureHost_InvalidHostTestType_ThrowsArgumentOutOfRangeException() | ||
| { | ||
| // Arrange | ||
| var fixture = new AspNetCoreHostFixture(); | ||
| var invalidHostTest = new InvalidHostTest<AspNetCoreHostFixture>(fixture); | ||
|
|
||
| // Act & Assert | ||
| Assert.Throws<ArgumentOutOfRangeException>(() => fixture.ConfigureHost(invalidHostTest)); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void ConfigureApplicationCallback_SetAndGet_ReturnsCorrectValue() | ||
| { | ||
| // Arrange | ||
| var fixture = new AspNetCoreHostFixture(); | ||
| Action<IApplicationBuilder> callback = app => { }; | ||
|
|
||
| // Act | ||
| fixture.ConfigureApplicationCallback = callback; | ||
|
|
||
| // Assert | ||
| Assert.Equal(callback, fixture.ConfigureApplicationCallback); | ||
| } | ||
|
Comment on lines
+36
to
+48
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Enhance test to verify callback execution. The current test only verifies the property storage. Consider enhancing it to verify that the stored callback is actually executed when the application is configured. [Fact]
public void ConfigureApplicationCallback_SetAndGet_ReturnsCorrectValue()
{
// Arrange
var fixture = new AspNetCoreHostFixture();
- Action<IApplicationBuilder> callback = app => { };
+ var callbackExecuted = false;
+ Action<IApplicationBuilder> callback = app => { callbackExecuted = true; };
// Act
fixture.ConfigureApplicationCallback = callback;
+ fixture.ConfigureHost(new ValidHostTest(fixture));
// Assert
Assert.Equal(callback, fixture.ConfigureApplicationCallback);
+ Assert.True(callbackExecuted, "Callback should have been executed during host configuration");
}
|
||
|
|
||
| [Fact] | ||
| public void ConfigureHost_ValidHostTest_ConfiguresHostCorrectly() | ||
| { | ||
| // Arrange | ||
| var fixture = new AspNetCoreHostFixture(); | ||
| var hostTest = new ValidHostTest(fixture); | ||
|
|
||
| // Act | ||
| fixture.ConfigureHost(hostTest); | ||
|
|
||
| // Assert | ||
| Assert.NotNull(fixture.Host); | ||
| Assert.NotNull(fixture.Application); | ||
| } | ||
|
Comment on lines
+50
to
+63
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Enhance host configuration validation. The current test only verifies that Host and Application are non-null. Consider adding more specific assertions about the expected configuration state. [Fact]
public void ConfigureHost_ValidHostTest_ConfiguresHostCorrectly()
{
// Arrange
var fixture = new AspNetCoreHostFixture();
var hostTest = new ValidHostTest(fixture);
+ var expectedServiceType = typeof(ITestService);
// Act
fixture.ConfigureHost(hostTest);
// Assert
Assert.NotNull(fixture.Host);
Assert.NotNull(fixture.Application);
+ // Verify expected services are registered
+ var serviceProvider = fixture.Host.Services;
+ var service = serviceProvider.GetService(expectedServiceType);
+ Assert.NotNull(service);
+ // Verify expected middleware is configured
+ // Add more specific assertions based on ValidHostTest configuration
}
|
||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,11 @@ | ||||||||||||||||||||
| using Xunit; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| namespace Codebelt.Extensions.Xunit.Hosting.AspNetCore.Assets | ||||||||||||||||||||
| { | ||||||||||||||||||||
| public class InvalidHostTest<T> : Test, IClassFixture<T> where T : class, IHostFixture | ||||||||||||||||||||
| { | ||||||||||||||||||||
| public InvalidHostTest(T hostFixture) | ||||||||||||||||||||
| { | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
Comment on lines
+7
to
+9
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Store the hostFixture parameter for test usage. The constructor accepts a hostFixture parameter but doesn't store it, which might be needed for test scenarios. public InvalidHostTest(T hostFixture)
{
+ HostFixture = hostFixture;
}
+protected T HostFixture { get; }📝 Committable suggestion
Suggested change
|
||||||||||||||||||||
| } | ||||||||||||||||||||
| } | ||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| using Microsoft.AspNetCore.Builder; | ||
| using Microsoft.Extensions.DependencyInjection; | ||
|
|
||
| namespace Codebelt.Extensions.Xunit.Hosting.AspNetCore.Assets | ||
| { | ||
| public class ValidHostTest : AspNetCoreHostTest<AspNetCoreHostFixture> | ||
| { | ||
| public ValidHostTest(AspNetCoreHostFixture hostFixture) : base(hostFixture) | ||
| { | ||
| } | ||
|
|
||
| public override void ConfigureServices(IServiceCollection services) | ||
| { | ||
|
|
||
| } | ||
|
|
||
| public override void ConfigureApplication(IApplicationBuilder app) | ||
| { | ||
|
|
||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Document additional changes mentioned in PR objectives.
The release notes should also include:
IHostFixtureinterface (newDisposeAsyncmethod)HostFixtureclass (new async lifecycle methods)xunit.extensibility.corepackage dependencyApply this diff to add the missing changes:
📝 Committable suggestion