✅ add macOS platform support to disposable test assets#65
Conversation
Updates conditional compilation directives to align with NET9.0+ where macOS support is available. Adds macOS native library loading via libSystem.B.dylib in UnmanagedDisposable for both NET9+ and earlier TFM branches. Ensures platform-specific resource disposal tests work on macOS.
Greptile SummaryThis PR adds macOS platform support to the
Confidence Score: 4/5The macOS changes are safe and correctly gated behind NET9_0_OR_GREATER; the only concern is two unreachable macOS blocks in the legacy net48 code path that add noise but pose no runtime risk. The core macOS addition in UnmanagedDisposable is correct and follows the existing Linux pattern. The #else branch copies produce dead code that is never compiled into a macOS-capable binary, which is not a runtime hazard but is a maintainability concern. The HostFixture.cs guard rename is a no-op across all current TFMs. test/Codebelt.Extensions.Xunit.Tests/Assets/UnmanagedDisposable.cs — the macOS blocks in the #else (net48) section are unreachable and should be removed or documented. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[UnmanagedDisposable constructor] --> B{NET9_0_OR_GREATER?}
B -- Yes --> C{OS Platform}
C -- Windows --> D[NativeLibrary.TryLoad kernel32.dll]
C -- Linux --> E[NativeLibrary.TryLoad libc.so.6]
C -- OSX NEW --> F[NativeLibrary.TryLoad libSystem.B.dylib]
D --> G[_handle = CreateFileW result]
E --> H[_handle = _libHandle]
F --> H
B -- No net48/Windows only --> I{OS Platform}
I -- Windows --> J[NativeLibraryLoader kernel32.dll]
I -- Linux --> K[NativeLibraryLoader libc.so.6]
I -- OSX DEAD CODE --> L[NativeLibraryLoader libSystem.B.dylib]
L -.->|net48 never runs on macOS| M((unreachable))
subgraph Dispose
N[OnDisposeUnmanagedResources] --> O{NET9_0_OR_GREATER?}
O -- Yes --> P{OS Platform}
P -- Windows --> Q[CloseHandle + NativeLibrary.Free]
P -- Linux --> R[NativeLibrary.Free]
P -- OSX NEW --> R
O -- No --> S{OS Platform}
S -- Windows --> T[_nativeLibrary.Dispose]
S -- Linux --> U[_nativeLibrary.Dispose]
S -- OSX DEAD CODE --> V[_nativeLibrary.Dispose]
V -.->|net48 never runs on macOS| M
end
Prompt To Fix All With AIFix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
test/Codebelt.Extensions.Xunit.Tests/Assets/UnmanagedDisposable.cs:84-89
**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.
### Issue 2 of 2
test/Codebelt.Extensions.Xunit.Tests/Assets/UnmanagedDisposable.cs:141-144
**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.
Reviews (1): Last reviewed commit: "✅ add macOS platform support to disposab..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| 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 | ||
| } |
There was a problem hiding this comment.
#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!
| else if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX)) | ||
| { | ||
| _nativeLibrary.Dispose(); | ||
| } |
There was a problem hiding this comment.
#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!
2 participants
Updates conditional compilation directives to align with NET9.0+ where macOS support is available. Adds macOS native library loading via libSystem.B.dylib in UnmanagedDisposable for both NET9+ and earlier TFM branches. Ensures platform-specific resource disposal tests work on macOS.