test: fill the two empty util test stubs

[bobbyg603]

Closes #167

Tests/Runtime/Util/ShouldPostExceptionImplTests.cs and Tests/Runtime/Util/ExceptionPostOptionsExtensionsTests.cs were empty // TODO stubs. This fills both with 24 tests. No production code changes.

These document current behaviour, not desired behaviour

Both units have defects that are deliberately not fixed here. Every test that encodes one carries a comment saying so, so the redesign in #170 knows which assertions it is free to break:

Pinned behaviour	Why it is arguably wrong
An unrelated exception inside the 3s window is blocked	Time-based only, no per-signature dedup — and the report is dropped, not deferred, so a genuinely distinct fault that lands within 3s of another is never sent
Separate delegates share one window	Every IClientSettingsRepository defaults to the same static method, so two BugSplat clients in one process rate-limit each other
A timestamp in the future blocks everything until real time passes it	The window is measured against DateTime.Now, a local wall clock — DST or an NTP correction wedges the limiter
Concurrent callers can admit more than one report	The read-then-write of the shared timestamp is unsynchronized, and exceptions are reported from background threads
Whitespace-only option values suppress the client default	The guard is IsNullOrEmpty, not IsNullOrWhiteSpace, so the report ships a field that renders blank
Calling SetNullOrEmptyValues twice duplicates attachments	Nothing dedupes; callers only get away with it because the manager builds fresh options per report
clientSettings.Attributes == null throws NullReferenceException	The foreach dereferences it unguarded, so the failure surfaces as an unhandled exception inside the report path

How the shared static state is handled

ShouldPostExceptionImpl's entire state is one private static DateTime, so a naive fixture would be order-dependent and would need multi-second sleeps. Both problems are solved the same way: the fixture reflects on that field to reset it to its declared initial value in [SetUp] and [TearDown], and tests that need an aged or skewed window write the field instead of sleeping.

Consequences, all verified:

• Order-independent by construction — every test starts from a known state regardless of what ran before.
• No residue — [TearDown] restores the initial value, so this fixture cannot perturb the rest of the suite.
• Nothing else in the suite uses the default delegate (every other test injects its own ShouldPostException), so there is no interference in either direction.
• No sleeps; the whole fixture runs in milliseconds.
• A [OneTimeSetUp] asserts the field still exists, so if B3: Rate limiter — per-signature dedup, suppressed count, honest docs #170 renames or removes it these tests fail loudly with an explanatory message instead of NREing.

The concurrency test is the one place where an exact assertion would be a coin flip, so it asserts only what must always hold — at least one caller admitted, no more than all of them, and the window closed afterwards — with a comment explaining that the exact count becomes assertable once #170 adds synchronization. A flaky test here would be worse than no test.

Overlap with #161

#161 is fixing the inverted Attachments?.Count != 0 guard concurrently and adding a tightly-scoped test to ExceptionPostOptionsExtensionsTests.cs. The null-Attachments case is intentionally left uncovered here so this PR does not assert the behaviour that PR is changing. Everything else about the attachment merge is covered (empty list, non-empty list, append-not-replace, double-apply). Expect a small textual conflict in that file since both PRs replace the same stub body; the two sets of tests are complementary and should merge by keeping both.

Note the fixture class name (ExceptionPostOptionsExtensionsTests) does not match the unit (ReportPostOptionsExtensions) — kept as-is to preserve the filename #167 and #161 both refer to.

Verification

Both units are plain C# apart from one UnityEngine.Debug.Log call, so the tests were actually executed, not just compiled. A throwaway dotnet test project (net10.0, NUnit 3.14 + NUnit3TestAdapter 4.5 + Microsoft.NET.Test.Sdk 17.11) compiled the two units plus their dependencies against Unity 6000.5.6f1's real UnityEngine.CoreModule.dll and BugSplatDotNetStandard.dll:

Passed! - Failed: 0, Passed: 24, Skipped: 0, Total: 24

• 24/24 pass (10 rate limiter, 14 options merge).
• Full suite run 5x — 24/24 every time.
• The concurrency test run 10x in isolation — passed every time.
• Individual tests run alone under a filter — pass, confirming order-independence.
• The whole Runtime/** + Tests/** tree compiles against Unity's assemblies and Unity's NUnit build with 0 errors and only the two pre-existing CS0649 warnings in BugSplat.cs.

One harness detail worth recording: UnityEngine.Debug.Log bottoms out in an ECall that only exists inside the Unity player, so under plain dotnet test it throws SecurityException: ECall methods must be packaged into a system module — and on a background thread that aborts the run. The harness installed a no-op ILogHandler via Debug.unityLogger.logHandler to get past it. That stub lives only in the throwaway project, not in this branch; under Unity's own test runner Debug.Log works normally and LogAssert ignores LogType.Log.

🤖 Generated with Claude Code

[Copilot]

Pull request overview

Adds comprehensive NUnit coverage for two previously-empty util test stubs, documenting current (including known-defective) behaviors for the rate limiter and post-options merge logic, to support future redesign work.

Changes:

• Implemented a full ShouldPostExceptionImpl rate-limiter test suite, including deterministic static-state reset via reflection and a concurrency-focused test.
• Implemented a full ReportPostOptionsExtensions.SetNullOrEmptyValues merge-behavior test suite, including pinned “known defects” behaviors and attachment/attribute merge semantics.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
Tests/Runtime/Util/ShouldPostExceptionImplTests.cs	Adds deterministic, state-resetting tests for the ShouldPostExceptionImpl static time-window rate limiter (including concurrency).
Tests/Runtime/Util/ExceptionPostOptionsExtensionsTests.cs	Adds merge-behavior tests for SetNullOrEmptyValues, covering string defaults, attachments, and attributes (including pinned defect cases).

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.