-
Notifications
You must be signed in to change notification settings - Fork 1
V9.0.0/private assets experiment #12
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
555d650
5176fe0
a4e4bd4
31d40b3
41d6a98
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 |
|---|---|---|
|
|
@@ -35,6 +35,7 @@ | |
| </ItemGroup> | ||
|
|
||
| <ItemGroup> | ||
| <PackageReference Include="Cuemon.Core" Version="9.0.0-preview.9" PrivateAssets="compile" /> | ||
|
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. 💡 Codebase verification Cuemon.Core appears to be unused and may be removed. The Cuemon.Core package is referenced in multiple project files, but no usage was found in the codebase. If Cuemon.Core is not essential for your project, consider removing these references to clean up dependencies. 🔗 Analysis chainVerify the necessity and stability of the new Cuemon.Core package. The addition of the Cuemon.Core package (version 9.0.0-preview.9) with PrivateAssets="compile" is noted. While this aligns with the PR objectives for dependency updates, please consider the following:
To ensure this package is used correctly throughout the project, run the following script: This script will help identify where Cuemon.Core is being used and if it's referenced in other project files, which might be affected by the PrivateAssets attribute. 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Verify the usage of Cuemon.Core in the project
# Test: Search for Cuemon.Core usage
echo "Searching for Cuemon.Core usage:"
rg --type csharp "Cuemon\.Core" -g "!*.csproj"
# Test: Check if Cuemon.Core is referenced in other project files
echo "Checking for Cuemon.Core references in other .csproj files:"
rg --type xml "Cuemon\.Core" -g "*.csproj" -g "!src/Codebelt.Extensions.Xunit.Hosting/Codebelt.Extensions.Xunit.Hosting.csproj"
Length of output: 907 |
||
| <PackageReference Include="xunit.extensibility.core" Version="2.9.2" /> | ||
| </ItemGroup> | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| // This file is used by Code Analysis to maintain SuppressMessage | ||
| // attributes that are applied to this project. | ||
| // Project-level suppressions either have no target or are given | ||
| // a specific target and scoped to a namespace, type, member, etc. | ||
|
|
||
| using System.Diagnostics.CodeAnalysis; | ||
|
|
||
| [assembly: SuppressMessage("Major Code Smell", "S3881:\"IDisposable\" should be implemented correctly", Justification = "This is an implementation of the IDisposable interface tailored to avoid wrong implementations.", Scope = "type", Target = "~T:Codebelt.Extensions.Xunit.Hosting.HostFixture")] |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -11,9 +11,8 @@ namespace Codebelt.Extensions.Xunit.Hosting | |||||||||||||||||||||||||||||||||||||||||||
| /// <summary> | ||||||||||||||||||||||||||||||||||||||||||||
| /// Provides a default implementation of the <see cref="IHostFixture"/> interface. | ||||||||||||||||||||||||||||||||||||||||||||
| /// </summary> | ||||||||||||||||||||||||||||||||||||||||||||
| /// <seealso cref="Disposable" /> | ||||||||||||||||||||||||||||||||||||||||||||
| /// <seealso cref="IHostFixture" /> | ||||||||||||||||||||||||||||||||||||||||||||
| public class HostFixture : Disposable, IHostFixture | ||||||||||||||||||||||||||||||||||||||||||||
| public class HostFixture : IDisposable, IHostFixture | ||||||||||||||||||||||||||||||||||||||||||||
|
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. 💡 Codebase verification Incomplete Disposal Implementation for Unmanaged Resources in It appears that while
🔗 Analysis chainConfirm the implications of changing inheritance from By changing Run the following script to compare the members of Sure, let's gather more information to ensure all necessary disposal methods are correctly implemented. Sure, let's further investigate the inheritance hierarchy to ensure all disposal logic is correctly implemented. 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Check if all relevant disposal methods from `Disposable` are implemented in `HostFixture`.
# Test: Find methods in the `Disposable` class and compare with methods in `HostFixture`.
# Expected: All necessary disposal methods from `Disposable` are re-implemented in `HostFixture`.
# Locate the definition of the `Disposable` class
DISPOSABLE_FILE=$(fd 'Disposable.cs' --type f)
echo "Methods in Disposable class ($DISPOSABLE_FILE):"
rg --type cs 'protected\s+(virtual|abstract|override)?\s+void\s+\w+\s*\(' "$DISPOSABLE_FILE" -A 3
echo "Methods in HostFixture class (src/Codebelt.Extensions.Xunit.Hosting/HostFixture.cs):"
rg --type cs 'protected\s+(virtual|override)?\s+void\s+\w+\s*\(' src/Codebelt.Extensions.Xunit.Hosting/HostFixture.cs -A 3
Length of output: 1436 Script: #!/bin/bash
# Description: Verify the contents of `ManagedDisposable.cs` to ensure it includes all required disposal methods.
# Locate the definition of the `ManagedDisposable` class
MANAGED_DISPOSABLE_FILE=$(fd 'ManagedDisposable.cs' --type f)
echo "Contents of ManagedDisposable class ($MANAGED_DISPOSABLE_FILE):"
cat "$MANAGED_DISPOSABLE_FILE"
Length of output: 952 Script: #!/bin/bash
# Description: Identify the base classes of `HostFixture` to understand the inheritance chain.
# Locate and display the base classes of HostFixture
rg --type cs 'public\s+class\s+HostFixture\s*:\s*(.+)' src/Codebelt.Extensions.Xunit.Hosting/HostFixture.cs
Length of output: 166 |
||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||
| /// <summary> | ||||||||||||||||||||||||||||||||||||||||||||
| /// Initializes a new instance of the <see cref="HostFixture"/> class. | ||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -127,15 +126,52 @@ public virtual void ConfigureHost(Test hostTest) | |||||||||||||||||||||||||||||||||||||||||||
| #endif | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| /// <summary> | ||||||||||||||||||||||||||||||||||||||||||||
| /// Called when this object is being disposed by either <see cref="M:Cuemon.Disposable.Dispose" /> or <see cref="M:Cuemon.Disposable.Dispose(System.Boolean)" /> having <c>disposing</c> set to <c>true</c> and <see cref="P:Cuemon.Disposable.Disposed" /> is <c>false</c>. | ||||||||||||||||||||||||||||||||||||||||||||
| /// Gets a value indicating whether this <see cref="HostFixture"/> object is disposed. | ||||||||||||||||||||||||||||||||||||||||||||
| /// </summary> | ||||||||||||||||||||||||||||||||||||||||||||
| protected override void OnDisposeManagedResources() | ||||||||||||||||||||||||||||||||||||||||||||
| /// <value><c>true</c> if this <see cref="HostFixture"/> object is disposed; otherwise, <c>false</c>.</value> | ||||||||||||||||||||||||||||||||||||||||||||
| public bool Disposed { get; private 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. Ensure thread-safe access to the If Consider using a locking mechanism or marking |
||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| /// <summary> | ||||||||||||||||||||||||||||||||||||||||||||
| /// Called when this object is being disposed by either <see cref="Dispose()" /> or <see cref="Dispose(bool)" /> having <c>disposing</c> set to <c>true</c> and <see cref="Disposed" /> is <c>false</c>. | ||||||||||||||||||||||||||||||||||||||||||||
| /// </summary> | ||||||||||||||||||||||||||||||||||||||||||||
| protected virtual void OnDisposeManagedResources() | ||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||
| if (ServiceProvider is ServiceProvider sp) | ||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||
| sp.Dispose(); | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
| Host?.Dispose(); | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| /// <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> | ||||||||||||||||||||||||||||||||||||||||||||
| protected virtual void OnDisposeUnmanagedResources() | ||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| /// <summary> | ||||||||||||||||||||||||||||||||||||||||||||
| /// Releases all resources used by the <see cref="HostFixture"/> object. | ||||||||||||||||||||||||||||||||||||||||||||
| /// </summary> | ||||||||||||||||||||||||||||||||||||||||||||
| public void Dispose() | ||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||
| Dispose(true); | ||||||||||||||||||||||||||||||||||||||||||||
| GC.SuppressFinalize(this); | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| /// <summary> | ||||||||||||||||||||||||||||||||||||||||||||
| /// Releases the unmanaged resources used by the <see cref="HostFixture"/> object and optionally releases the managed resources. | ||||||||||||||||||||||||||||||||||||||||||||
| /// </summary> | ||||||||||||||||||||||||||||||||||||||||||||
| /// <param name="disposing"><c>true</c> to release both managed and unmanaged resources; <c>false</c> to release only unmanaged resources.</param> | ||||||||||||||||||||||||||||||||||||||||||||
| protected void Dispose(bool disposing) | ||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+166
to
+167
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. Make To follow the standard dispose pattern and allow derived classes to override the disposal logic, Apply this diff: -protected void Dispose(bool disposing)
+protected virtual void Dispose(bool disposing)📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||
| if (Disposed) { return; } | ||||||||||||||||||||||||||||||||||||||||||||
| if (disposing) | ||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||
| OnDisposeManagedResources(); | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
| OnDisposeUnmanagedResources(); | ||||||||||||||||||||||||||||||||||||||||||||
| Disposed = true; | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+168
to
+175
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 synchronization to the The Apply this diff to add locking: protected virtual void Dispose(bool disposing)
{
+ lock (this)
+ {
if (Disposed) { return; }
if (disposing)
{
OnDisposeManagedResources();
}
OnDisposeUnmanagedResources();
Disposed = true;
+ }
}📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,46 @@ | ||
| using System.ComponentModel; | ||
| using Codebelt.Extensions.Xunit.Hosting.AspNetCore.Http; | ||
| using Microsoft.AspNetCore.Http; | ||
| using Microsoft.Extensions.DependencyInjection; | ||
| using Xunit; | ||
| using Xunit.Abstractions; | ||
|
|
||
| namespace Codebelt.Extensions.Xunit.Hosting.AspNetCore | ||
| { | ||
| public class ServiceCollectionExtensionsTest : Test | ||
| { | ||
| public ServiceCollectionExtensionsTest(ITestOutputHelper output) : base(output) | ||
| { | ||
| } | ||
|
|
||
| [Theory] | ||
| [InlineData(ServiceLifetime.Transient)] | ||
| [InlineData(ServiceLifetime.Scoped)] | ||
| [InlineData(ServiceLifetime.Singleton)] | ||
| public void AddFakeHttpContextAccessor_ShouldAddService(ServiceLifetime lifetime) | ||
| { | ||
| // Arrange | ||
| var services = new ServiceCollection(); | ||
|
|
||
| // Act | ||
| services.AddFakeHttpContextAccessor(lifetime); | ||
| var serviceProvider = services.BuildServiceProvider(); | ||
| var httpContextAccessor = serviceProvider.GetService<IHttpContextAccessor>(); | ||
|
|
||
| // Assert | ||
| Assert.NotNull(httpContextAccessor); | ||
| Assert.IsType<FakeHttpContextAccessor>(httpContextAccessor); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void AddFakeHttpContextAccessor_ShouldThrowInvalidEnumArgumentException_ForInvalidLifetime() | ||
| { | ||
| // Arrange | ||
| var services = new ServiceCollection(); | ||
| var invalidLifetime = (ServiceLifetime)999; | ||
|
|
||
| // Act & Assert | ||
| Assert.Throws<InvalidEnumArgumentException>(() => services.AddFakeHttpContextAccessor(invalidLifetime)); | ||
| } | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,6 +15,7 @@ | |
| </ItemGroup> | ||
|
|
||
| <ItemGroup> | ||
| <PackageReference Include="Cuemon.Core" Version="9.0.0-preview.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. 💡 Codebase verification Add The <PackageReference Include="Cuemon.Core" Version="9.0.0-preview.9" PrivateAssets="compile" />This change will prevent the package from flowing to projects that consume this test project, maintaining encapsulation of dependencies. 🔗 Analysis chainVerify compatibility and consider adding PrivateAssets="compile" The addition of the Cuemon.Core package (version 9.0.0-preview.9) aligns with the PR objectives for updating package references. However, there are a few points to consider:
To verify the compatibility and usage of this package, you can run the following script: 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Check for Cuemon.Core usage and verify target framework compatibility
# Check for Cuemon.Core usage in the test project
echo "Checking for Cuemon.Core usage:"
rg --type csharp "using Cuemon" test/Codebelt.Extensions.Xunit.Hosting.Tests
# Verify target framework in the project file
echo "Verifying target framework:"
rg "<TargetFramework>" test/Codebelt.Extensions.Xunit.Hosting.Tests/Codebelt.Extensions.Xunit.Hosting.Tests.csproj
# Check other package versions for consistency
echo "Checking other package versions:"
rg "<PackageReference Include=" test/Codebelt.Extensions.Xunit.Hosting.Tests/Codebelt.Extensions.Xunit.Hosting.Tests.csproj
Length of output: 1060 |
||
| <PackageReference Include="Xunit.Priority" Version="1.1.6" /> | ||
| </ItemGroup> | ||
|
|
||
|
|
||
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.
🛠️ Refactor suggestion
Simplify Object Initialization for Better Readability
The nested object initializer on line 42 can be refactored to improve readability. Separating the initialization steps makes the code clearer and easier to maintain.
Apply this diff to simplify the initialization:
This change enhances clarity by explicitly setting the
RequestServicesproperty after creating theFakeHttpContextAccessorinstance.📝 Committable suggestion