Demote granular internal logs from Debug to Trace level

[thomhurst]

Summary

• Moves per-item scheduling, hook building, test grouping, and assembly loading logs from Debug to Trace level, reducing noise at Debug verbosity
• Keeps high-level flow logs (test count, max parallelism config, hook init start/complete, infrastructure bookends) at Debug
• Adds IsTraceEnabled property to TUnitFrameworkLogger for guard checks

Files changed

• ConstraintKeyScheduler — all 5 per-test constraint key logs → Trace
• TestScheduler — 7 per-group/per-batch start logs → Trace; kept 2 flow + 5 config logs as Debug
• HookDelegateBuilder — 2 per-hook detail logs → Trace; kept 2 top-level summary logs as Debug
• TestGroupingService — 7 summary detail lines → Trace
• InfrastructureGenerator — 4 per-assembly generated log patterns → Trace; kept init start/complete as Debug
• TUnitFrameworkLogger — added IsTraceEnabled property
• 4 snapshot .verified.txt files updated to match

Test plan

• TUnit.Engine builds with 0 warnings
• TUnit.Core.SourceGenerator builds with 0 warnings
• Snapshot verified files updated to match generator output
• CI passes

[claude]

Code Review

Overall this is a clean, well-scoped PR that improves the signal-to-noise ratio at Debug level. The separation between Debug (flow-level) and Trace (per-item detail) is a meaningful improvement for users who run with --log-level debug.

---

Concern: Assembly load failures demoted to Trace

The four Failed to load ... messages in InfrastructureGenerator.cs (and the generated snapshot files) have been moved to Trace:

// Before
LogDebug("[ModuleInitializer:...] Failed to load ...: " + ex.Message);
// After  
LogTrace("[ModuleInitializer:...] Failed to load ...: " + ex.Message);

I'd argue these should stay at Debug. The success path (Assembly initialized) is absolutely right to move to Trace — it's noise. But a failure deserves more visibility. A user who runs --log-level debug to investigate a test discovery problem would no longer see these. They'd need to know to enable Trace first, which is a significant discoverability hurdle.

The PR description itself notes this pattern: "Keeps high-level flow logs... at Debug". An assembly load failure is effectively a flow event (it affects what tests are discovered), not a per-item detail.

Suggested approach: Keep Failed to load at Debug, move only Loading assembly containing and Assembly initialized to Trace.

---

Inconsistency: Missing IsTraceEnabled guards in HookDelegateBuilder

ConstraintKeyScheduler and TestScheduler correctly guard their trace calls:

if (_logger.IsTraceEnabled)
    await _logger.LogTraceAsync($"Starting test {test.TestId} with constraint keys: {string.Join(...)}");

But HookDelegateBuilder.cs does not:

// No guard — string interpolation always runs
await _logger.LogTraceAsync($"Creating delegate for {hookTypeName} hook: {hook.Name}");
await _logger.LogTraceAsync($"Built {hooks.Count} {hookTypeName} hook delegate(s)");

The existing LogAsync base method short-circuits on IsEnabled, so correctness is fine — but the string interpolation always allocates even when Trace is off. The guards in the other files exist specifically to avoid this. The PR adds IsTraceEnabled precisely to enable this pattern; it should be applied consistently.

---

Minor: TestGroupingService summary block has no guard

The 7-call summary block:

await _logger.LogTraceAsync("═══ Test Grouping Summary ═══").ConfigureAwait(false);
await _logger.LogTraceAsync($"  Parallel (no constraints): {parallelTests.Count} tests").ConfigureAwait(false);
// ... 5 more calls

Could be wrapped in a single guard to avoid 7 async overhead calls when Trace is disabled:

if (_logger.IsTraceEnabled)
{
    await _logger.LogTraceAsync("═══ Test Grouping Summary ═══").ConfigureAwait(false);
    // ...
}

The individual string interpolations here are cheap (just integer property accesses), so this is low priority — but it would be consistent with the guard pattern used elsewhere and GroupTestsByConstraintsAsync is on the hot path.

---

What's good

• The two-tier Debug/Trace separation is clearly reasoned and the PR description maps it well.
• Adding to is the right way to enable efficient guard checks — it mirrors the existing pattern symmetrically.
• Snapshot files correctly updated across all 4 target frameworks.
• The per-test constraint key and scheduling logs are absolutely right to move to Trace — they'd produce O(N) lines per test run at Debug level.

[claude]

Code Review

Overall this is a clean, well-scoped PR that improves the signal-to-noise ratio at Debug level. The separation between Debug (flow-level) and Trace (per-item detail) is a meaningful improvement for users who run with --log-level debug.

---

Concern: Assembly load failures demoted to Trace

The four Failed to load ... messages in InfrastructureGenerator.cs (and the generated snapshot files) have been moved to Trace:

// Before
LogDebug("[ModuleInitializer:...] Failed to load ...: " + ex.Message);
// After  
LogTrace("[ModuleInitializer:...] Failed to load ...: " + ex.Message);

I'd argue these should stay at Debug. The success path (Assembly initialized) is absolutely right to move to Trace — it's noise. But a failure deserves more visibility. A user who runs --log-level debug to investigate a test discovery problem would no longer see these. They'd need to know to enable Trace first, which is a significant discoverability hurdle.

The PR description itself notes this pattern: "Keeps high-level flow logs... at Debug". An assembly load failure is effectively a flow event (it affects what tests are discovered), not a per-item detail.

Suggested approach: Keep Failed to load at Debug, move only Loading assembly containing and Assembly initialized to Trace.

---

Inconsistency: Missing IsTraceEnabled guards in HookDelegateBuilder

ConstraintKeyScheduler and TestScheduler correctly guard their trace calls:

if (_logger.IsTraceEnabled)
    await _logger.LogTraceAsync($"Starting test {test.TestId} with constraint keys: {string.Join(...)}");

But HookDelegateBuilder.cs does not:

// No guard — string interpolation always runs
await _logger.LogTraceAsync($"Creating delegate for {hookTypeName} hook: {hook.Name}");
await _logger.LogTraceAsync($"Built {hooks.Count} {hookTypeName} hook delegate(s)");

The existing LogAsync base method short-circuits on IsEnabled, so correctness is fine — but the string interpolation always allocates even when Trace is off. The guards in the other files exist specifically to avoid this. The PR adds IsTraceEnabled precisely to enable this pattern; it should be applied consistently.

---

Minor: TestGroupingService summary block has no guard

The 7-call summary block:

await _logger.LogTraceAsync("=== Test Grouping Summary ===").ConfigureAwait(false);
await _logger.LogTraceAsync($"  Parallel (no constraints): {parallelTests.Count} tests").ConfigureAwait(false);
// ... 5 more calls

Could be wrapped in a single guard to avoid 7 async overhead calls when Trace is disabled:

if (_logger.IsTraceEnabled)
{
    await _logger.LogTraceAsync("=== Test Grouping Summary ===").ConfigureAwait(false);
    // ...
}

The individual string interpolations here are cheap (just integer property accesses), so this is low priority — but it would be consistent with the guard pattern used elsewhere, and GroupTestsByConstraintsAsync is on the hot path.

---

What's good

• The two-tier Debug/Trace separation is clearly reasoned and the PR description maps it well.
• Adding IsTraceEnabled to TUnitFrameworkLogger is the right way to enable efficient guard checks — it mirrors the existing IsDebugEnabled pattern symmetrically.
• Snapshot files correctly updated across all 4 target frameworks.
• The per-test constraint key and scheduling logs are absolutely right to move to Trace — they would produce O(N) lines per test run at Debug level, which was the real noise problem.