Unit tests and build artifacts - #16
Merged
Merged
Conversation
added 20 commits
April 11, 2026 15:41
- Add xUnit test project (tests/SharpSDL3.Tests) with 191 test methods - Unit tests cover validation guards across Sdl, Render, Events, GamePad, Storage, Mutex, Camera, Logger, and Mouse - Struct layout tests verify Marshal.SizeOf, LayoutKind, and field offsets - AtomicInt and Color operator overload tests for full coverage - Enum value and Constants string property verification - Fuzz tests exercise VersionNum, struct marshalling, null-pointer guards, AtomicInt arithmetic/bitwise/comparison, SdlBool, and Color equality with 80,000+ assertions across deterministic random inputs - Add GitHub Actions workflow (test.yml) with cross-platform matrix, Coverlet coverage collection, ReportGenerator HTML reports, 25% line coverage threshold gate, and job summary posting - Add SharpSDL3.sln solution file for multi-project build - Add TESTING.md documenting test categories, coverage, CI/CD, and usage
- ARCHITECTURE.md: functional groupings, Mermaid diagram, dependency flow, line counts by subsystem, key design decisions - SECURITY_ANALYSIS.md: 7 attack vectors against the binding layer including DLL hijacking, memory leak DoS, delegate use-after-free, buffer overread, GC relocation, string injection, and event type confusion — with code and policy fixes for each
- ARCHITECTURE.md: replace detailed per-file diagram with high-level functional grouping view - TODO.md: top 5 recommendations ranked by risk — memory leaks, delegate pinning, bounds checking, error handling, Sdl.cs refactor — each with current test coverage status
Add feature/** to push branch filter so CI runs on feature/unit-tests.
Use tag filter feature/** so CI only runs when a feature tag is pushed, not on every commit to a feature branch.
- Add using System to EventsTests, SdlValidationTests, StorageTests, SdlExceptionTests - Add using System.Collections.Generic to ConstantsTests - Cast lambda to Action in SdlExceptionTests to avoid xUnit async overload resolution error
Add TestHelpers.cs with AssertFalseOrNativeNotFound, AssertZeroOrNativeNotFound, and AssertNoThrowOrNativeNotFound helpers. Tests that exercise validation guards which call LogWarn/LogError now accept DllNotFoundException as a valid outcome — the guard correctly rejected bad input, the native log call is a side effect.
Split AllConstants_StartWithSdlPrefix into two tests: - PropertyConstants_StartWithSdlDotPrefix for SdlProp* fields - HintConstants_AreUppercaseWithUnderscores for SdlHint* fields
Coverage is 3% line because Coverlet counts generated P/Invoke stubs that can't execute without native SDL3. All 227 tests pass. Report coverage in job summary without failing the build.
Pipeline changes: - Add configurable SDL3_VERSION env var (currently 3.4.4) - Linux/macOS: build SDL3 from source with cmake (cached) - Windows: download pre-built SDL3.dll from GitHub releases - Fuzz job also builds SDL3 Test changes: - Add SdlFixture: shared xUnit fixture that calls Init/Quit - Add NativeIntegrationTests: 17 tests covering version, error handling, properties, surface ops, palette, logging, and timer - Tests gracefully skip when SDL3 is not available - Update TESTING.md with native test docs and version config
Use --collect:"XPlat Code Coverage" instead of -p:CollectCoverage MSBuild properties which broke with .NET SDK 10. Find the generated cobertura XML dynamically since coverlet.collector puts it in a GUID subdirectory.
- Remove separate find-coverage step, inline into report generation - Use **/*.trx glob for test results (collector changes directory layout) - Always upload coverage artifact even if report generation fails - List TestResults contents as fallback if no coverage file found
New test files exercising real SDL3 calls: - NativePropertyTests: property lifecycle, defaults, lock/unlock, hints, error handling, app metadata, clipboard - NativeSurfaceTests: create/destroy, pixel read/write, alpha/color mod, color key, clip rect, blit, fill, duplicate, convert, flip, lock, format name - NativeSystemTests: all log levels, log priority, timer, CPU info, SIMD features, platform, version, screen saver, init flags, thread ID - NativeMutexTests: mutex/rwlock/condition create/lock/unlock/destroy, try-lock, condition timeout - NativeEventTests: poll, push user event, register custom types, enable/disable, flush range Total: 264 test methods (was 227)
- Add package metadata to SharpSDL3.csproj (PackageId, Version, Authors, Description, License, Tags, Symbols) - Add 'package' job that runs after tests pass, produces .nupkg and .snupkg with preview version based on run number - Upload as 'nuget-package' artifact for download
Reviewer's GuideAdds a comprehensive xUnit-based test suite (including fuzz and native-integration tests), CI workflow for running tests with coverage and packaging, and architecture/security/testing/TODO documentation around SharpSDL3; also introduces a dedicated SharpSDL3.Tests project and supporting fixtures/helpers. Class diagram for SharpSDL3 test project structureclassDiagram
class SharpSDL3_Tests_Project {
}
class SdlFixture {
+SdlFixture()
+bool IsAvailable
+void Initialize()
+void Dispose()
}
class TestHelpers {
+Random CreateDeterministicRandom(int seed)
+T RoundTripStructViaMarshal~T~(T value)
+void AssertThrowsSdlException(string methodName)
}
class SdlCoreTests {
+void VersionNum_ReturnsExpected()
+void StructureToPointer_RoundTrips()
}
class SdlValidationTests {
+void CreateWindow_NullTitle_Throws()
}
class RenderTests {
+void CreateWindowAndRenderer_InvalidTitle_Throws()
+void ConvertEventToRenderCoordinates_NullRenderer_Throws()
}
class EventsTests {
+void AddEventWatch_NullFilter_Throws()
+void FlushEvents_InvalidRange_Throws()
}
class GamePadTests {
+void AddGamepadMapping_InvalidString_Throws()
}
class StorageTests {
+void CopyStorageFile_InvalidPath_Throws()
}
class MutexTests {
+void DestroyMutex_NullHandle_Throws()
}
class CameraTests {
+void AcquireCameraFrame_NullCamera_Throws()
}
class LoggerTests {
+void Log_NullOrEmptyMessage_Throws()
}
class MouseTests {
+void GetMouseNameForId_ZeroId_Throws()
}
class StructLayoutTests {
+void Event_HasExplicitLayout()
+void Rect_Size_IsExpected()
}
class AtomicIntTests {
+void Operators_BehaveLikeInt()
}
class ColorTests {
+void Equality_And_HashCode_Consistent()
}
class EnumTests {
+void EventType_Values_AreStable()
}
class ConstantsTests {
+void AllConstants_AreNonNullUnique()
}
class SdlExceptionTests {
+void SdlException_IsThrownAndCaught()
}
class FuzzTests {
+void Fuzz_VersionNum_RandomInputs()
+void Fuzz_StructMarshalling_RoundTrips()
+void Fuzz_AtomicInt_Operators()
+void Fuzz_Color_Equality()
}
class NativeIntegrationTests {
+void Init_Succeeded()
+void CreateSurface_ValidArgs_ReturnsNonZero()
+void Log_ValidMessage_DoesNotThrow()
}
class NativeSurfaceTests {
+void CreateAndClearSurface_Succeeds()
}
class NativeSystemTests {
+void GetTicks_ReturnsNonZero()
}
class NativeMutexTests {
+void CreateAndDestroyMutex_Succeeds()
}
class NativePropertyTests {
+void CreateAndClearProperties_Succeeds()
}
class NativeEventTests {
+void PollEvent_DoesNotCrash()
}
class SharpSDL3_Library {
}
SharpSDL3_Tests_Project ..> SharpSDL3_Library : references
SdlCoreTests --> SharpSDL3_Library
SdlValidationTests --> SharpSDL3_Library
RenderTests --> SharpSDL3_Library
EventsTests --> SharpSDL3_Library
GamePadTests --> SharpSDL3_Library
StorageTests --> SharpSDL3_Library
MutexTests --> SharpSDL3_Library
CameraTests --> SharpSDL3_Library
LoggerTests --> SharpSDL3_Library
MouseTests --> SharpSDL3_Library
StructLayoutTests --> SharpSDL3_Library
AtomicIntTests --> SharpSDL3_Library
ColorTests --> SharpSDL3_Library
EnumTests --> SharpSDL3_Library
ConstantsTests --> SharpSDL3_Library
SdlExceptionTests --> SharpSDL3_Library
FuzzTests --> SharpSDL3_Library
NativeIntegrationTests --> SharpSDL3_Library
NativeSurfaceTests --> SharpSDL3_Library
NativeSystemTests --> SharpSDL3_Library
NativeMutexTests --> SharpSDL3_Library
NativePropertyTests --> SharpSDL3_Library
NativeEventTests --> SharpSDL3_Library
NativeIntegrationTests ..> SdlFixture : uses
NativeSurfaceTests ..> SdlFixture : uses
NativeSystemTests ..> SdlFixture : uses
NativeMutexTests ..> SdlFixture : uses
NativePropertyTests ..> SdlFixture : uses
NativeEventTests ..> SdlFixture : uses
SdlCoreTests ..> TestHelpers : uses
StructLayoutTests ..> TestHelpers : uses
FuzzTests ..> TestHelpers : uses
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- The fuzz tests use very high iteration counts (10,000 loops across many methods), which may significantly slow down CI runs; consider reducing the counts for the default pipeline or moving heavier fuzzing into a separate, less-frequent workflow.
- The TestHelpers helpers intentionally swallow DllNotFoundException, which is useful for CI without SDL3 but could hide regressions when SDL3 is expected to be present; consider splitting tests or gating those helpers on an explicit configuration flag/trait so failures are visible when native libs should load.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The fuzz tests use very high iteration counts (10,000 loops across many methods), which may significantly slow down CI runs; consider reducing the counts for the default pipeline or moving heavier fuzzing into a separate, less-frequent workflow.
- The TestHelpers helpers intentionally swallow DllNotFoundException, which is useful for CI without SDL3 but could hide regressions when SDL3 is expected to be present; consider splitting tests or gating those helpers on an explicit configuration flag/trait so failures are visible when native libs should load.
## Individual Comments
### Comment 1
<location path="tests/SharpSDL3.Tests/FuzzTests.cs" line_range="22-31" />
<code_context>
+/// </summary>
+public class AtomicIntTests
+{
+ [Fact]
+ public void ImplicitConversion_FromInt()
+ {
</code_context>
<issue_to_address>
**suggestion (testing):** Extend special-float roundtrip assertions to all FRect fields, not just X
In `Fuzz_StructureToPointer_SpecialFloatValues`, you only check `SingleToInt32Bits` for `frect.X` even though `val` is written to `X`, `Y`, `W`, and `H`. Please add bitwise equality assertions for all four fields to ensure each is marshalled correctly for NaN/Inf and other special values.
Suggested implementation:
```csharp
Assert.Equal(BitConverter.SingleToInt32Bits(val), BitConverter.SingleToInt32Bits(roundtrip.X));
Assert.Equal(BitConverter.SingleToInt32Bits(val), BitConverter.SingleToInt32Bits(roundtrip.Y));
Assert.Equal(BitConverter.SingleToInt32Bits(val), BitConverter.SingleToInt32Bits(roundtrip.W));
Assert.Equal(BitConverter.SingleToInt32Bits(val), BitConverter.SingleToInt32Bits(roundtrip.H));
```
I assumed the existing special-float fuzz test `Fuzz_StructureToPointer_SpecialFloatValues` already:
1. Constructs an `FRect` with `X`, `Y`, `W`, and `H` all set to the same `val`, and
2. Performs a single bitwise assertion on `roundtrip.X`.
If the assertion currently uses a different style (e.g., `Assert.Equal(val, roundtrip.X)` or a helper method), update the `SEARCH` text accordingly and apply the same bitwise `SingleToInt32Bits` pattern to `Y`, `W`, and `H`. Also ensure that `roundtrip` is the correct variable name for the unmarshalled `FRect`.
</issue_to_address>
### Comment 2
<location path="tests/SharpSDL3.Tests/NativeIntegrationTests.cs" line_range="26-31" />
<code_context>
+ _output = output;
+ }
+
+ private bool RequireSdl()
+ {
+ if (!_sdl.Available)
+ {
+ _output.WriteLine("SKIPPED: SDL3 native library not available");
+ return false;
+ }
+ return true;
</code_context>
<issue_to_address>
**suggestion (testing):** Use explicit test skipping instead of silent returns when SDL is unavailable
Because these tests just `return`, xUnit reports them as passed, so CI can appear green even when the SDL native library is missing and no integration coverage actually runs. Use xUnit’s skip mechanism instead (e.g., have `RequireSdl` throw `new SkipException("SDL3 native library not available")`) so these tests are reported as skipped rather than passed.
Suggested implementation:
```csharp
private readonly SdlFixture _sdl;
private readonly ITestOutputHelper _output;
public NativeIntegrationTests(SdlFixture sdl, ITestOutputHelper output)
{
_sdl = sdl;
_output = output;
}
private void RequireSdl()
{
if (!_sdl.Available)
{
throw new SkipException("SDL3 native library not available");
}
}
// --- Init / Version ---
[Fact]
public void Init_Succeeded()
{
RequireSdl();
Assert.True(_sdl.Available);
}
```
1. At the top of `tests/SharpSDL3.Tests/NativeIntegrationTests.cs`, ensure that `using Xunit.Sdk;` is present so that `SkipException` resolves correctly.
2. Apply the same pattern (`RequireSdl();` instead of `if (!RequireSdl()) return;`) to any other tests in this file that currently rely on `RequireSdl()` returning a `bool`.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Signed-off-by: Adonis Deliannis <blizzardo1@blizzeta.net>
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
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.
I added unit testing and build artifacts.
Summary by Sourcery
Add a comprehensive automated testing and CI pipeline plus supporting documentation for SharpSDL3.
New Features:
Enhancements:
Build:
Documentation:
Tests: