Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/Codebelt.Extensions.Xunit.Hosting/HostFixture.cs
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ protected virtual void OnDisposeManagedResources()
/// <summary>
/// Called when this object is being disposed by <see cref="DisposeAsync()"/>.
/// </summary>
#if NET8_0_OR_GREATER
#if NET9_0_OR_GREATER
protected virtual async ValueTask OnDisposeManagedResourcesAsync()
{
if (Host is IAsyncDisposable asyncDisposable)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ namespace Codebelt.Extensions.Xunit.Assets;
public class AsyncDisposable : Test
{
IDisposable _disposableResource = new MemoryStream();
#if NET8_0_OR_GREATER
#if NET9_0_OR_GREATER
IAsyncDisposable _asyncDisposableResource = new MemoryStream();
#else
IAsyncDisposable _asyncDisposableResource = new WemoryStream();
Expand Down
25 changes: 23 additions & 2 deletions test/Codebelt.Extensions.Xunit.Tests/Assets/UnmanagedDisposable.cs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ public delegate IntPtr CreateFileDelegate(string lpFileName,

public UnmanagedDisposable()
{
#if NET8_0_OR_GREATER
#if NET9_0_OR_GREATER
if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
{
if (NativeLibrary.TryLoad("kernel32.dll", GetType().Assembly, DllImportSearchPath.System32, out _libHandle))
Expand All @@ -53,6 +53,13 @@ public UnmanagedDisposable()
_handle = _libHandle; // i don't know of any native methods on unix
}
}
else if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX))
{
if (NativeLibrary.TryLoad("libSystem.B.dylib", GetType().Assembly, DllImportSearchPath.SafeDirectories, out _libHandle))
{
_handle = _libHandle; // i don't know of any native methods on unix
}
}
#else
if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
{
Expand All @@ -74,6 +81,12 @@ public UnmanagedDisposable()
_libHandle = _nativeLibrary.Handle;
_handle = _libHandle; // i don't know of any native methods on unix
}
else if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX))
{
_nativeLibrary = new NativeLibrary("libSystem.B.dylib");
_libHandle = _nativeLibrary.Handle;
_handle = _libHandle; // i don't know of any native methods on unix
}
Comment on lines +84 to +89

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Dead code: macOS branch inside the #else (net48) block

The #else block only compiles for target frameworks where NET9_0_OR_GREATER is false. From Directory.Build.props, the only such test TFM is net48, which the project explicitly restricts to Windows-only runners. RuntimeInformation.IsOSPlatform(OSPlatform.OSX) can never be true inside this block, so the new _nativeLibrary = new NativeLibrary("libSystem.B.dylib") assignment and the corresponding _nativeLibrary.Dispose() call in OnDisposeUnmanagedResources are unreachable. macOS tests always target net9.0 or net10.0, meaning they always hit the #if NET9_0_OR_GREATER branch added above.

Prompt To Fix With AI
This is a comment left during a code review.
Path: test/Codebelt.Extensions.Xunit.Tests/Assets/UnmanagedDisposable.cs
Line: 84-89

Comment:
**Dead code: macOS branch inside the `#else` (net48) block**

The `#else` block only compiles for target frameworks where `NET9_0_OR_GREATER` is false. From `Directory.Build.props`, the only such test TFM is `net48`, which the project explicitly restricts to Windows-only runners. `RuntimeInformation.IsOSPlatform(OSPlatform.OSX)` can never be `true` inside this block, so the new `_nativeLibrary = new NativeLibrary("libSystem.B.dylib")` assignment and the corresponding `_nativeLibrary.Dispose()` call in `OnDisposeUnmanagedResources` are unreachable. macOS tests always target `net9.0` or `net10.0`, meaning they always hit the `#if NET9_0_OR_GREATER` branch added above.

How can I resolve this? If you propose a fix, please make it concise.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

#endif
}

Expand All @@ -89,7 +102,7 @@ protected override void OnDisposeManagedResources()

protected override void OnDisposeUnmanagedResources()
{
#if NET8_0_OR_GREATER
#if NET9_0_OR_GREATER
if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
{
if (_handle != IntPtr.Zero)
Expand All @@ -106,6 +119,10 @@ protected override void OnDisposeUnmanagedResources()
{
NativeLibrary.Free(_libHandle);
}
else if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX))
{
NativeLibrary.Free(_libHandle);
}
#else
if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
{
Expand All @@ -121,6 +138,10 @@ protected override void OnDisposeUnmanagedResources()
{
_nativeLibrary.Dispose();
}
else if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX))
{
_nativeLibrary.Dispose();
}
Comment on lines +141 to +144

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Dead code: macOS disposal inside the #else block

Same issue as the constructor: the #else guard makes this reachable only under net48, which never runs on macOS. The _nativeLibrary.Dispose() call here will never be executed in any supported CI configuration.

Prompt To Fix With AI
This is a comment left during a code review.
Path: test/Codebelt.Extensions.Xunit.Tests/Assets/UnmanagedDisposable.cs
Line: 141-144

Comment:
**Dead code: macOS disposal inside the `#else` block**

Same issue as the constructor: the `#else` guard makes this reachable only under `net48`, which never runs on macOS. The `_nativeLibrary.Dispose()` call here will never be executed in any supported CI configuration.

How can I resolve this? If you propose a fix, please make it concise.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

#endif
}
}
2 changes: 1 addition & 1 deletion test/Codebelt.Extensions.Xunit.Tests/DisposableTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ public DisposableTest(ITestOutputHelper output) : base(output)
{
}

#if NET8_0_OR_GREATER
#if NET9_0_OR_GREATER
[Fact]
public async Task AsyncDisposable_VerifyThatAssetIsBeingDisposed()
{
Expand Down
Loading