Fix various documentation snippets

[thomhurst]

No description provided.

[claude]

Claude finished @thomhurst's task —— View job

---

PR Review: Fix various documentation snippets

Todo List:

• Read and analyze the modified documentation file
• Review documentation changes for quality and accuracy
• Check code examples for correctness and best practices
• Provide comprehensive review feedback

---

Review Summary ✅

This PR significantly improves the documentation by updating outdated API examples to match the current TUnit implementation. The changes are excellent and should be merged.

Positive Changes 🎯

1. API Signature Updates - Critical Fix

• Fixed: ITestExecutor.ExecuteAsync(TestContext, Func<Task>) → ExecuteTest(TestContext, Func<ValueTask>)
• Fixed: IHookExecutor.ExecuteAsync(HookContext, Func<Task>) → Specific methods for each hook type
• Impact: Documentation now matches the actual API, preventing developer confusion

2. IHookExecutor Interface Overhaul - Major Improvement

• Before: Generic ExecuteAsync method with HookContext
• After: Specific methods for each lifecycle phase:
  • ExecuteBeforeTestDiscoveryHook
  • ExecuteBeforeTestSessionHook
  • ExecuteBeforeAssemblyHook
  • etc.
• Added: Clear documentation of method parameters and context types
• Better: Type-safe approach with specific context objects

3. Enhanced Code Examples - Much More Practical

• IHookExecutor: Replaced generic resource management example with practical logging implementation
• Shows: How to handle errors in hooks (try-catch in ExecuteBeforeClassHook)
• Demonstrates: Proper async/await patterns throughout

4. IParallelConstraint Clarification - Important Correction

• Fixed: Documented that IParallelConstraint is a marker interface (no members)
• Added: Practical examples using [NotInParallel] attribute
• Clarified: How constraint keys work for parallel execution control
• Bonus: Shows both keyed and keyless usage patterns

5. Consistent ValueTask Usage - Performance Best Practice

• All examples now use ValueTask instead of Task
• Aligns with TUnit's performance-first philosophy
• Reduces allocations for synchronous operations

Code Quality Assessment 📋

✅ Strengths

• Accuracy: All API signatures match current implementation
• Completeness: Comprehensive examples for each interface
• Clarity: Well-documented parameter purposes and behaviors
• Best Practices: Proper error handling, async patterns, and resource management
• Practical: Examples solve real-world testing scenarios

✅ Documentation Standards

• Clear section headers and organization
• Consistent code formatting and syntax highlighting
• Proper explanation of interface purposes and use cases
• Good progression from simple to complex examples

✅ Technical Correctness

• Modern C# syntax (Stopwatch.StartNew(), proper using statements)
• Thread-safe patterns where applicable
• Proper disposal patterns in transaction example
• Correct attribute usage ([assembly:], [TestExecutor<T>])

No Issues Found 🎉

I found no bugs, security concerns, or performance issues in this documentation update. The changes are purely beneficial:

• No breaking changes introduced
• No deprecated patterns shown
• No security anti-patterns
• Performance guidance is appropriate (ValueTask usage)

Recommendation

✅ APPROVE - This PR should be merged immediately. It fixes critical documentation gaps that would otherwise mislead developers trying to implement extension points.

---

[claude]

Claude finished @thomhurst's task —— View job

---

PR Review: Fix various documentation snippets ✅

Todo List:

• Read and understand TUnit project guidelines
• Analyze the main documentation changes in extension-points.md
• Review other documentation file changes
• Check code examples for correctness and best practices
• Provide comprehensive review feedback

---

Review Summary ✅

This PR makes excellent improvements to the TUnit documentation by fixing outdated API examples and improving clarity. The changes are all positive and should be merged.

Key Improvements 🎯

1. API Signature Corrections - Critical Fix

The most important change is in docs/docs/advanced/extension-points.md:18:

• Fixed: Updated ITestExecutor.ExecuteAsync(TestContext context, Func<Task> action) → ExecuteTest(TestContext context, Func<ValueTask> action)
• Impact: Documentation now matches the actual TUnit API

2. IHookExecutor Interface Overhaul - Major Improvement

• Before: Generic ExecuteAsync method with HookContext
• After: Specific methods for each lifecycle phase:
  • ExecuteBeforeTestDiscoveryHook/ExecuteAfterTestDiscoveryHook
  • ExecuteBeforeTestSessionHook/ExecuteAfterTestSessionHook
  • ExecuteBeforeAssemblyHook/ExecuteAfterAssemblyHook
  • ExecuteBeforeClassHook/ExecuteAfterClassHook
  • ExecuteBeforeTestHook/ExecuteAfterTestHook
• Added: Proper method signatures with specific context types for each hook
• Enhanced: Comprehensive example showing error handling patterns

3. IParallelConstraint Clarification - Important Documentation Fix

• Documented: IParallelConstraint is a marker interface (no members)
• Added: Practical examples using [NotInParallel] attribute with constraint keys
• Clarified: How parallel execution control works with keyed and keyless constraints

4. Consistent ValueTask Usage - Performance Best Practice

• All examples now use ValueTask instead of Task
• Aligns with TUnit's performance-first philosophy (CLAUDE.md:57)
• Reduces allocations for synchronous operations

5. Minor Corrections Across Files

• Method names corrected in various docs to match current API
• Improved clarity in code examples
• Consistent formatting and style

Code Quality Assessment 📋

✅ Strengths

• Accuracy: All API signatures now match current TUnit implementation
• Completeness: Comprehensive examples for each interface
• Best Practices: Proper async patterns, error handling, and resource management
• Modern C# Syntax: Uses Stopwatch.StartNew(), proper using statements
• Thread Safety: Appropriate patterns where applicable
• Performance: Follows TUnit's performance guidelines with ValueTask usage

✅ Documentation Standards

• Clear section organization and headers
• Consistent code formatting with syntax highlighting
• Practical, real-world examples (logging, database transactions)
• Progressive complexity from simple to advanced

✅ Technical Correctness

• No breaking patterns or anti-patterns introduced
• Proper disposal patterns in transaction example
• Correct attribute usage and registration patterns
• Compatible with TUnit's dual-mode architecture (source-gen + reflection)

Security & Performance ✅

• No security concerns: Documentation examples follow secure patterns
• No performance issues: Examples use appropriate async patterns and resource management
• Compliant: All examples follow TUnit's coding standards from CLAUDE.md

Test Coverage Assessment 📋

While this is documentation-only changes, the examples demonstrate:

• Proper exception handling in hook executors
• Resource cleanup patterns (transactions, service scopes)
• Thread-safe implementations where needed
• Integration with TUnit's context system

Recommendation

✅ APPROVE & MERGE - This PR provides critical documentation fixes that will prevent developer confusion and improve the TUnit development experience. All changes are beneficial with no downsides identified.

Files Reviewed:

• docs/docs/advanced/extension-points.md (major API updates)
• docs/docs/examples/instrumenting-global-test-ids.md (ValueTask updates)
• docs/docs/execution/retrying.md (method name fix)
• docs/docs/test-lifecycle/event-subscribing.md (Task→ValueTask)
• docs/docs/test-lifecycle/cleanup.md (formatting)
• And 5 other minor documentation files

---

[claude]

Claude finished @thomhurst's task —— View job

---

PR Review: Fix various documentation snippets ✅

Todo List:

• Read and understand TUnit project guidelines
• Analyze the main documentation changes in extension-points.md
• Review other documentation file changes
• Check code examples for correctness and best practices
• Provide comprehensive review feedback

---

Review Summary ✅

This PR makes excellent improvements to the TUnit documentation by fixing outdated API examples and improving clarity. The changes are all positive and should be merged.

Key Improvements 🎯

1. API Signature Corrections - Critical Fix

The most important change is in docs/docs/advanced/extension-points.md:18:

• Fixed: Updated ITestExecutor.ExecuteAsync(TestContext context, Func<Task> action) → ExecuteTest(TestContext context, Func<ValueTask> action)
• Impact: Documentation now matches the actual TUnit API

2. IHookExecutor Interface Overhaul - Major Improvement

• Before: Generic ExecuteAsync method with HookContext
• After: Specific methods for each lifecycle phase:
  • ExecuteBeforeTestDiscoveryHook/ExecuteAfterTestDiscoveryHook
  • ExecuteBeforeTestSessionHook/ExecuteAfterTestSessionHook
  • ExecuteBeforeAssemblyHook/ExecuteAfterAssemblyHook
  • ExecuteBeforeClassHook/ExecuteAfterClassHook
  • ExecuteBeforeTestHook/ExecuteAfterTestHook
• Added: Proper method signatures with specific context types for each hook
• Enhanced: Comprehensive example showing error handling patterns

3. IParallelConstraint Clarification - Important Documentation Fix

• Documented: IParallelConstraint is a marker interface (no members)
• Added: Practical examples using [NotInParallel] attribute with constraint keys
• Clarified: How parallel execution control works with keyed and keyless constraints

4. Consistent ValueTask Usage - Performance Best Practice

• All examples now use ValueTask instead of Task
• Aligns with TUnit's performance-first philosophy (CLAUDE.md:57)
• Reduces allocations for synchronous operations

5. Minor Corrections Across Files

• Method names corrected in various docs to match current API
• Improved clarity in code examples
• Consistent formatting and style

Code Quality Assessment 📋

✅ Strengths

• Accuracy: All API signatures now match current TUnit implementation
• Completeness: Comprehensive examples for each interface
• Best Practices: Proper async patterns, error handling, and resource management
• Modern C# Syntax: Uses Stopwatch.StartNew(), proper using statements
• Thread Safety: Appropriate patterns where applicable
• Performance: Follows TUnit's performance guidelines with ValueTask usage

✅ Documentation Standards

• Clear section organization and headers
• Consistent code formatting with syntax highlighting
• Practical, real-world examples (logging, database transactions)
• Progressive complexity from simple to advanced

✅ Technical Correctness

• No breaking patterns or anti-patterns introduced
• Proper disposal patterns in transaction example
• Correct attribute usage and registration patterns
• Compatible with TUnit's dual-mode architecture (source-gen + reflection)

Security & Performance ✅

• No security concerns: Documentation examples follow secure patterns
• No performance issues: Examples use appropriate async patterns and resource management
• Compliant: All examples follow TUnit's coding standards from CLAUDE.md

Test Coverage Assessment 📋

While this is documentation-only changes, the examples demonstrate:

• Proper exception handling in hook executors
• Resource cleanup patterns (transactions, service scopes)
• Thread-safe implementations where needed
• Integration with TUnit's context system

Recommendation

✅ APPROVE & MERGE - This PR provides critical documentation fixes that will prevent developer confusion and improve the TUnit development experience. All changes are beneficial with no downsides identified.

Files Reviewed:

• docs/docs/advanced/extension-points.md (major API updates)
• docs/docs/examples/instrumenting-global-test-ids.md (ValueTask updates)
• docs/docs/execution/retrying.md (method name fix)
• docs/docs/test-lifecycle/event-subscribing.md (Task→ValueTask)
• docs/docs/test-lifecycle/cleanup.md (formatting)
• And 5 other minor documentation files

---