perf: cache reporting properties on test contexts

[thomhurst]

Review fixes and current performance (751f6d3)

Addressed both memory-retention findings. Every terminal ToTestNode update releases the context's cached reporting properties, including deferred-enumeration placeholders and failures before coordinator execution. ClearCaches also clears reporting fields on registered contexts, covering discovery-only requests. A scope check after publication removes stale properties if construction races with cache reset. Published nodes keep their independent property snapshots.

Regression verification: the new cleanup assertions fail in all seven cases against the previous PR's saved Core/Engine binaries (c994193). The fixed binaries pass all 11 TestNodeLocationTests and all 17 GitHubReporterTests. The generated 10,000-test executable passes in both source-generated and reflection modes. Release builds pass for net10.0 and netstandard2.0; the test-project build reports existing analyzer/code-fixer warnings. The previously documented HtmlReporterTests baseline failure remains outside this fix; that suite was not rerun for this revision.

Reran the reporting benchmark with the original dictionary baseline, previous PR, and fixed implementation in the same BenchmarkDotNet run. Before = 656b66e; BeforeReview = c994193; After = 751f6d3. Each operation resets caches and packages discovered, in-progress, and passed updates for 1,000 contexts. The measurement includes the new registry sweep and terminal cleanup. Context construction remains outside measurement, and all variants share the fixed Core assembly.

Against the original dictionary, the fixed implementation takes 38.1% less time without categories and 31.4% less with three categories, with 17.6% and 12.4% fewer allocated bytes, respectively. Compared with the previous PR, the measured mean is 3.0% higher without categories and 9.5% lower with three categories; allocation totals are unchanged at the displayed precision. These are isolated reporting results, not whole-executable speedups. The whole-executable timing evidence below belongs to the original PR revision and was not rerun for this fix.

No builds, tests, or other benchmarks launched by this task overlapped this measurement. Dry validation passed first; the measured run used 20 iterations and six warmups.

BenchmarkDotNet v0.15.8, Windows 11 (10.0.26200.9168/25H2/2025Update/HudsonValley2)
12th Gen Intel Core i7-12700K 3.60GHz, 1 CPU, 20 logical and 12 physical cores
.NET SDK 11.0.100-preview.7.26381.103
  [Host] : .NET 10.0.12 (10.0.12, 10.0.1226.42308), X64 RyuJIT x86-64-v3

Toolchain=InProcessEmitToolchain  IterationCount=20  WarmupCount=6  

Method	CategoryCount	Mean	Error	StdDev	Ratio	RatioSD	Gen0	Gen1	Allocated	Alloc Ratio
Before	0	466.4 μs	6.40 μs	7.12 μs	1.00	0.02	90.3320	22.9492	1159.1 KB	1.00
After	0	288.7 μs	6.57 μs	7.57 μs	0.62	0.02	74.7070	-	954.81 KB	0.82
BeforeReview	0	280.3 μs	2.73 μs	2.92 μs	0.60	0.01	74.7070	24.9023	954.81 KB	0.82
Before	3	612.4 μs	5.27 μs	5.41 μs	1.00	0.01	128.9063	47.8516	1651.29 KB	1.00
After	3	420.0 μs	7.63 μs	8.48 μs	0.69	0.01	113.2813	0.4883	1447 KB	0.88
BeforeReview	3	464.3 μs	12.81 μs	14.75 μs	0.76	0.02	113.2813	56.6406	1447 KB	0.88

Reproduce the review comparison

Use the original reproduction instructions below to build 656b66e into baseline-runner and c994193 into idea4-runner. Build 751f6d3 into pr6791-fixed. Keep the three folders beside the benchmark project. Set PERF_ROOT to their parent, update the signing-key path, and use these project/source files. Program.cs remains the BenchmarkSwitcher entry point shown below.

dotnet run -c Release -- --filter '*ReportingBench*' --job Dry --inProcess --artifacts ../review-dry
dotnet run -c Release --no-build -- --filter '*ReportingBench*' --inProcess --iterationCount 20 --warmupCount 6 --artifacts ../review-results --exporters fulljson

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>net10.0</TargetFramework>
    <ImplicitUsings>enable</ImplicitUsings>
    <Nullable>enable</Nullable>
    <AssemblyName>TUnit.UnitTests</AssemblyName>
    <SignAssembly>true</SignAssembly>
    <AssemblyOriginatorKeyFile>C:/git/TUnit/eng/strongname.snk</AssemblyOriginatorKeyFile>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="BenchmarkDotNet" Version="0.15.8" />
    <Reference Include="TUnit.Core"><HintPath>../pr6791-fixed/TUnit.Core.dll</HintPath></Reference>
    <Reference Include="Microsoft.Testing.Platform"><HintPath>../baseline-runner/Microsoft.Testing.Platform.dll</HintPath></Reference>
    <Reference Include="Microsoft.Testing.Extensions.TrxReport.Abstractions"><HintPath>../baseline-runner/Microsoft.Testing.Extensions.TrxReport.Abstractions.dll</HintPath></Reference>
  </ItemGroup>

</Project>

using System.Reflection;
using System.Runtime.Loader;
using BenchmarkDotNet.Attributes;
using Microsoft.Testing.Platform.Extensions.Messages;
using TUnit.Core;

[MemoryDiagnoser]
public class ReportingBench
{
    [Params(0, 3)]
    public int CategoryCount { get; set; }
    private TestContext[] _contexts = null!;
    private Func<TestContext, TestNodeStateProperty, TestNode> _before = null!, _after = null!;
    private Action _clearBefore = null!, _clearAfter = null!;
    private Func<TestContext, TestNodeStateProperty, TestNode> _beforeReview = null!;
    private Action _clearBeforeReview = null!;

    [GlobalSetup]
    public void Setup()
    {
        var root = Environment.GetEnvironmentVariable("PERF_ROOT") ?? "C:/git/TUnit-perf-evidence-20260912";
        (_before, _clearBefore) = Load(root + "/baseline-runner/TUnit.Engine.dll");
        (_after, _clearAfter) = Load(root + "/pr6791-fixed/TUnit.Engine.dll");
        (_beforeReview, _clearBeforeReview) = Load(root + "/idea4-runner/TUnit.Engine.dll");
        var classMetadata = new ClassMetadata
        {
            TypeInfo = new ConcreteType(typeof(ReportingBench)), Type = typeof(ReportingBench), Name = nameof(ReportingBench),
            Namespace = "Benchmarks", Assembly = new AssemblyMetadata { Name = "Benchmarks" },
            Parameters = [], Properties = [], Parent = null
        };
        var methodMetadata = MethodMetadataFactory.Create("Test", typeof(ReportingBench), typeof(void), classMetadata);
        _contexts = Enumerable.Range(0, 1000).Select(i =>
        {
            var context = new TestContext("Test", new EmptyServices(), null!, new TestBuilderContext { TestMetadata = methodMetadata }, CancellationToken.None);
            context.Metadata.TestDetails = new TestDetails([])
            {
                TestId = "Benchmarks.ReportingBench.Test" + i, TestName = "Test" + i, ClassType = typeof(ReportingBench), MethodName = "Test",
                ClassInstance = this, TestMethodArguments = [], TestClassArguments = [], MethodMetadata = methodMetadata,
                ReturnType = typeof(void), AttributesByType = new Dictionary<Type, IReadOnlyList<Attribute>>(), TestFilePath = "Tests.cs", TestLineNumber = i + 1
            };
            for (var category = 0; category < CategoryCount; category++) context.TestDetails.Categories.Add("Category" + category);
            context.TestStart = new DateTimeOffset(2026, 9, 12, 12, 0, 0, TimeSpan.Zero);
            context.Execution.TestEnd = context.TestStart.Value.AddMilliseconds(1);
            return context;
        }).ToArray();
        var a = Before();
        var b = After();
        if (!a.Uid.Equals(b.Uid) || a.DisplayName != b.DisplayName || !Properties(a).SequenceEqual(Properties(b)))
            throw new Exception("Reporting output differs");
    }

    private static List<IProperty> Properties(TestNode node)
    {
        var result = new List<IProperty>();
        foreach (var property in node.Properties) result.Add(property);
        return result;
    }

    [GlobalCleanup]
    public void Cleanup()
    {
        foreach (var context in _contexts) { context.RemoveFromRegistry(); context.Dispose(); }
        _clearBefore(); _clearAfter(); _clearBeforeReview();
    }

    [Benchmark(Baseline = true)]
    public TestNode Before() => Report(_before, _clearBefore);
    [Benchmark]
    public TestNode After() => Report(_after, _clearAfter);
    [Benchmark]
    public TestNode BeforeReview() => Report(_beforeReview, _clearBeforeReview);

    private TestNode Report(Func<TestContext, TestNodeStateProperty, TestNode> report, Action clear)
    {
        clear();
        TestNode last = null!;
        // One operation packages three updates for every test in a fresh 1,000-test session.
        foreach (var context in _contexts)
        {
            report(context, DiscoveredTestNodeStateProperty.CachedInstance);
            report(context, InProgressTestNodeStateProperty.CachedInstance);
            last = report(context, PassedTestNodeStateProperty.CachedInstance);
        }
        return last;
    }

    private static (Func<TestContext, TestNodeStateProperty, TestNode>, Action) Load(string path)
    {
        var loadContext = new AssemblyLoadContext(path, isCollectible: true);
        var type = loadContext.LoadFromAssemblyPath(Path.GetFullPath(path)).GetType("TUnit.Engine.Extensions.TestExtensions", true)!;
        return (type.GetMethod("ToTestNode", BindingFlags.Static | BindingFlags.NonPublic)!
            .CreateDelegate<Func<TestContext, TestNodeStateProperty, TestNode>>(),
            type.GetMethod("ClearCaches", BindingFlags.Static | BindingFlags.NonPublic)!.CreateDelegate<Action>());
    }

    private sealed class EmptyServices : IServiceProvider
    {
        public object? GetService(Type serviceType) => null;
    }
}

Original revision evidence (c994193)

Every test-node update looks up cached reporting metadata in a global ConcurrentDictionary keyed by TestId. Store that immutable metadata on its TestContext instead. A scope token preserves ClearCaches invalidation, volatile publication supports concurrent updates, and completion releases the cached properties so retained contexts do not retain reporting state. Every update still creates its own TestNode and PropertyBag.

Packaging discovered, in-progress and passed updates for 1,000 plain tests takes 286.2 us instead of 456.6 us (37.3% less) and allocates 954.80 KB instead of 1159.09 KB (17.6% less). With three categories per test, time falls 23.7% and allocations fall 12.4%. These are reporting-operation measurements, not complete test executions.

The benchmark excludes TestContext construction. The implementation adds one object reference to each TestContext (8 bytes per field on this x64 machine); this cost is outside the allocation column. Both engine versions run against the candidate Core assembly to share compatible TestContext instances. Completion-time clearing was added after the isolated measurement; it is covered by the final executable comparison and regression test.

Validation:

• Five TestNodeLocationTests passed, including cache reset/completion cleanup and concurrent updates preserving independent states.
• All 17 GitHubReporterTests passed; 42 of 43 HtmlReporterTests passed. The remaining test, BuildReportData_Reconstructs_Attempts_From_RetryAttemptsProperty, also fails with the saved baseline Core and Engine DLLs: InvalidOperationException, "Sequence contains no matching element", HtmlReporterTests.cs:566. This is an existing failure, not hidden or disabled by this PR.
• Release builds passed for net10.0 and netstandard2.0.
• A generated executable passed exactly 10,000 tests in both source-generated and reflection modes. Benchmark setup compares the baseline/candidate node identity, display name and property values.

Whole-executable check, 20 alternating AB/BA pairs after three warmups per variant: before mean 1211.16 ms, median 1031.23 ms; after mean 1245.34 ms, median 993.07 ms. Paired mean reduction -34.18 ms, approximate 95% t interval [-140.76, 72.40] ms. This noisy result establishes neither an end-to-end gain nor a regression. A previous sample set overlapped a validation run and was discarded; only the clean rerun appears below. No whole-suite speedup is claimed.

Baseline 656b66e723; candidate c994193334. Windows 11 / Intel i7-12700K, SDK 11.0.100-preview.7.26381.103, .NET 10.0.12, BenchmarkDotNet 0.15.8. Isolated AssemblyLoadContexts with shared Core/MTP dependencies, InProcessEmitToolchain, 20 iterations, six warmups. No other benchmarks/builds/tests launched by this task during the accepted measurements. Each operation resets reporting caches before packaging all three updates for every test.

BenchmarkDotNet v0.15.8, Windows 11 (10.0.26200.9168/25H2/2025Update/HudsonValley2)
12th Gen Intel Core i7-12700K 3.60GHz, 1 CPU, 20 logical and 12 physical cores
.NET SDK 11.0.100-preview.7.26381.103
  [Host] : .NET 10.0.12 (10.0.12, 10.0.1226.42308), X64 RyuJIT x86-64-v3

Toolchain=InProcessEmitToolchain  IterationCount=20  WarmupCount=6  

Method	CategoryCount	Mean	Error	StdDev	Ratio	RatioSD	Gen0	Gen1	Allocated	Alloc Ratio
Before	0	456.6 μs	6.02 μs	6.69 μs	1.00	0.02	90.3320	22.9492	1159.09 KB	1.00
After	0	286.2 μs	5.92 μs	6.58 μs	0.63	0.02	74.7070	24.9023	954.8 KB	0.82
Before	3	610.3 μs	12.09 μs	13.93 μs	1.00	0.03	128.9063	47.8516	1651.27 KB	1.00
After	3	465.8 μs	12.00 μs	12.84 μs	0.76	0.03	113.2813	56.6406	1446.99 KB	0.88

Reproduce the microbenchmark

Save this project and source in an external RuntimeBench directory. Replace the signing-key checkout path in the project. Build the baseline and PR into sibling baseline-runner and idea4-runner directories:

$env:PERF_ROOT = 'C:/path/to/evidence'
dotnet build C:/path/to/baseline/src/TUnit.Engine -c Release -f net10.0 -p:CopyLocalLockFileAssemblies=true -o "$env:PERF_ROOT/baseline-runner"
dotnet build C:/path/to/candidate/src/TUnit.Engine -c Release -f net10.0 -p:CopyLocalLockFileAssemblies=true -o "$env:PERF_ROOT/idea4-runner"
Set-Location "$env:PERF_ROOT/RuntimeBench"
dotnet run -c Release -- --filter '*ReportingBench*' --job Dry --inProcess --artifacts ../dry
dotnet run -c Release --no-build -- --filter '*ReportingBench*' --inProcess --iterationCount 20 --warmupCount 6 --artifacts ../results --exporters fulljson

Program.cs:

BenchmarkDotNet.Running.BenchmarkSwitcher.FromAssembly(typeof(ReportingBench).Assembly).Run(args);

RuntimeBench.csproj:

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>net10.0</TargetFramework>
    <ImplicitUsings>enable</ImplicitUsings>
    <Nullable>enable</Nullable>
    <AssemblyName>TUnit.UnitTests</AssemblyName>
    <SignAssembly>true</SignAssembly>
    <AssemblyOriginatorKeyFile>C:/git/TUnit-perf-blog/eng/strongname.snk</AssemblyOriginatorKeyFile>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="BenchmarkDotNet" Version="0.15.8" />
    <Reference Include="TUnit.Core"><HintPath>../idea4-runner/TUnit.Core.dll</HintPath></Reference>
    <Reference Include="Microsoft.Testing.Platform"><HintPath>../baseline-runner/Microsoft.Testing.Platform.dll</HintPath></Reference>
    <Reference Include="Microsoft.Testing.Extensions.TrxReport.Abstractions"><HintPath>../baseline-runner/Microsoft.Testing.Extensions.TrxReport.Abstractions.dll</HintPath></Reference>
  </ItemGroup>

</Project>

ReportingBench.cs:

using System.Reflection;
using System.Runtime.Loader;
using BenchmarkDotNet.Attributes;
using Microsoft.Testing.Platform.Extensions.Messages;
using TUnit.Core;

[MemoryDiagnoser]
public class ReportingBench
{
    [Params(0, 3)]
    public int CategoryCount { get; set; }
    private TestContext[] _contexts = null!;
    private Func<TestContext, TestNodeStateProperty, TestNode> _before = null!, _after = null!;
    private Action _clearBefore = null!, _clearAfter = null!;

    [GlobalSetup]
    public void Setup()
    {
        var root = Environment.GetEnvironmentVariable("PERF_ROOT") ?? "C:/git/TUnit-perf-evidence-20260912";
        (_before, _clearBefore) = Load(root + "/baseline-runner/TUnit.Engine.dll");
        (_after, _clearAfter) = Load(root + "/idea4-runner/TUnit.Engine.dll");
        var classMetadata = new ClassMetadata
        {
            TypeInfo = new ConcreteType(typeof(ReportingBench)), Type = typeof(ReportingBench), Name = nameof(ReportingBench),
            Namespace = "Benchmarks", Assembly = new AssemblyMetadata { Name = "Benchmarks" },
            Parameters = [], Properties = [], Parent = null
        };
        var methodMetadata = MethodMetadataFactory.Create("Test", typeof(ReportingBench), typeof(void), classMetadata);
        _contexts = Enumerable.Range(0, 1000).Select(i =>
        {
            var context = new TestContext("Test", new EmptyServices(), null!, new TestBuilderContext { TestMetadata = methodMetadata }, CancellationToken.None);
            context.Metadata.TestDetails = new TestDetails([])
            {
                TestId = "Benchmarks.ReportingBench.Test" + i, TestName = "Test" + i, ClassType = typeof(ReportingBench), MethodName = "Test",
                ClassInstance = this, TestMethodArguments = [], TestClassArguments = [], MethodMetadata = methodMetadata,
                ReturnType = typeof(void), AttributesByType = new Dictionary<Type, IReadOnlyList<Attribute>>(), TestFilePath = "Tests.cs", TestLineNumber = i + 1
            };
            for (var category = 0; category < CategoryCount; category++) context.TestDetails.Categories.Add("Category" + category);
            context.TestStart = new DateTimeOffset(2026, 9, 12, 12, 0, 0, TimeSpan.Zero);
            context.Execution.TestEnd = context.TestStart.Value.AddMilliseconds(1);
            return context;
        }).ToArray();
        var a = Before();
        var b = After();
        if (!a.Uid.Equals(b.Uid) || a.DisplayName != b.DisplayName || !Properties(a).SequenceEqual(Properties(b)))
            throw new Exception("Reporting output differs");
    }

    private static List<IProperty> Properties(TestNode node)
    {
        var result = new List<IProperty>();
        foreach (var property in node.Properties) result.Add(property);
        return result;
    }

    [GlobalCleanup]
    public void Cleanup()
    {
        foreach (var context in _contexts) { context.RemoveFromRegistry(); context.Dispose(); }
        _clearBefore(); _clearAfter();
    }

    [Benchmark(Baseline = true)]
    public TestNode Before() => Report(_before, _clearBefore);
    [Benchmark]
    public TestNode After() => Report(_after, _clearAfter);

    private TestNode Report(Func<TestContext, TestNodeStateProperty, TestNode> report, Action clear)
    {
        clear();
        TestNode last = null!;
        // One operation packages three updates for every test in a fresh 1,000-test session.
        foreach (var context in _contexts)
        {
            report(context, DiscoveredTestNodeStateProperty.CachedInstance);
            report(context, InProgressTestNodeStateProperty.CachedInstance);
            last = report(context, PassedTestNodeStateProperty.CachedInstance);
        }
        return last;
    }

    private static (Func<TestContext, TestNodeStateProperty, TestNode>, Action) Load(string path)
    {
        var loadContext = new AssemblyLoadContext(path, isCollectible: true);
        var type = loadContext.LoadFromAssemblyPath(Path.GetFullPath(path)).GetType("TUnit.Engine.Extensions.TestExtensions", true)!;
        return (type.GetMethod("ToTestNode", BindingFlags.Static | BindingFlags.NonPublic)!
            .CreateDelegate<Func<TestContext, TestNodeStateProperty, TestNode>>(),
            type.GetMethod("ClearCaches", BindingFlags.Static | BindingFlags.NonPublic)!.CreateDelegate<Action>());
    }

    private sealed class EmptyServices : IServiceProvider
    {
        public object? GetService(Type serviceType) => null;
    }
}

Raw whole-executable samples (milliseconds)

```csv "Pair","Variant","Milliseconds" "1","Before","1844.8189" "1","After","2023.8598" "2","After","2266.0609" "2","Before","2310.8018" "3","Before","1471.2523" "3","After","2150.7197" "4","After","1503.5988" "4","Before","1085.087" "5","Before","1104.559" "5","After","1196.6912" "6","After","1825.4811" "6","Before","1730.2069" "7","Before","1637.9832" "7","After","1123.7757" "8","After","1103.8656" "8","Before","1057.9552" "9","Before","1115.3834" "9","After","963.0704" "10","After","976.8791" "10","Before","958.4455" "11","Before","989.8522" "11","After","987.1668" "12","After","954.4256" "12","Before","968.6624" "13","Before","981.9879" "13","After","992.2406" "14","After","967.3678" "14","Before","1004.5135" "15","Before","980.9562" "15","After","978.1436" "16","After","946.2435" "16","Before","1090.6588" "17","Before","993.9318" "17","After","950.8155" "18","After","993.9059" "18","Before","970.9519" "19","Before","968.484" "19","After","1040.5333" "20","After","962.0398" "20","Before","956.7691"

</details>


<!-- This is an auto-generated comment: release notes by coderabbit.ai -->
## Summary by CodeRabbit

- **Bug Fixes**
  - Clearing cached test metadata now refreshes information for existing test contexts.
  - Final test updates now release cached reporting information across all terminal states.
  - Improved isolation of test state during concurrent test execution, preserving accurate messages and file-location details.
  - Reporting information is refreshed when a test context is removed and recreated, reducing the risk of stale results.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-09-12T09:45:32.206036Z	751f6d3	New commits

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 496bad0d-8a14-43d0-80be-c4280a0a2935

📥 Commits

Reviewing files that changed from the base of the PR and between c994193 and 751f6d3.

📒 Files selected for processing (3)

• src/TUnit.Core/TestContext.cs
• src/TUnit.Engine/Extensions/TestExtensions.cs
• tests/TUnit.Engine.Tests/TestNodeLocationTests.cs

Included review availability: Your plan provides up to 8 included reviews per hour; 3 remain after this review.

---

📝 Walkthrough

Walkthrough

The reporting property cache moved from a global test-ID dictionary to per-TestContext storage. Cache scopes now invalidate entries during ClearCaches(). Final node creation releases cached properties. Tests cover refresh behavior, terminal states, and concurrent node creation.

Changes

Reporting property cache

Layer / File(s)	Summary
Per-context cache ownership src/TUnit.Core/TestContext.cs, src/TUnit.Engine/Extensions/TestExtensions.cs	TestContext stores engine-owned cached reporting properties. Cached entries record their scope.
Cache invalidation and refresh src/TUnit.Core/TestContext.cs, src/TUnit.Engine/Extensions/TestExtensions.cs, tests/TUnit.Engine.Tests/TestNodeLocationTests.cs	ClearCaches() replaces the cache scope and clears registered contexts. Property retrieval rebuilds entries for a new scope. Tests verify metadata refresh.
Terminal cleanup and concurrency validation src/TUnit.Engine/Extensions/TestExtensions.cs, tests/TUnit.Engine.Tests/TestNodeLocationTests.cs	Final node creation clears cached properties for six terminal states. Tests verify retained node metadata and independent concurrent state.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Refactor

Sequence Diagram(s)

sequenceDiagram
  participant TestNodeLocationTests
  participant TestExtensions
  participant TestContext
  TestNodeLocationTests->>TestExtensions: Request cached properties
  TestExtensions->>TestContext: Read per-context cache
  TestExtensions->>TestContext: Write properties for the current scope
  TestNodeLocationTests->>TestExtensions: ClearCaches()
  TestExtensions->>TestContext: Clear registered context caches
  TestNodeLocationTests->>TestExtensions: Create final node
  TestExtensions->>TestContext: Release cached properties

Loading

Merge Risk: ⚪ Minimal · up to 751f6

The reporting-cache change is covered for refresh, cleanup, and concurrent node creation, with no actionable merge risk identified.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 10.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 3 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: caching reporting properties on test contexts for performance.

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch perf/blog-reporting-cache

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

<sub>A rabbit checks the cache at dawn
Fresh scopes replace the stale ones gone
Each context keeps its own small store
Final nodes leave no crumbs at the door
Sixty-four hops keep states apart
Clean reporting, a tidy start</sub>

<sub>Comment @coderabbitai help to get the list of available commands.</sub>

[greptile-apps]

Greptile Summary

This PR moves immutable reporting-property caching from a global TestId-keyed dictionary onto each TestContext, preserving cache invalidation while reducing reporting overhead.

• Rotates a cache-scope token and clears per-context properties when reporting caches reset.
• Releases cached reporting properties after terminal updates and registry removal.
• Adds coverage for reset behavior, terminal-state cleanup, and concurrent node updates.
• The previous reporting-metadata retention concern is addressed by terminal-state cleanup and reset-time sweeping.

Confidence Score: 5/5

The PR appears safe to merge; no actionable correctness or security issues remain.

The current implementation releases reporting metadata on terminal updates, registry removal, and cache resets while scope checks prevent invalidated properties from being reused. The previous retention finding is fully addressed.

Important Files Changed

Filename	Overview
src/TUnit.Core/TestContext.cs	Adds per-context reporting-cache storage and cleanup during registry removal and global cache resets.
src/TUnit.Engine/Extensions/TestExtensions.cs	Replaces the global per-test cache with scope-aware TestContext caching and terminal-state cleanup.
tests/TUnit.Engine.Tests/TestNodeLocationTests.cs	Adds regression coverage for invalidation, final-state release, and concurrent update isolation.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[ToTestNode called] --> B{Cached properties match current scope?}
    B -->|Yes| C[Reuse immutable reporting properties]
    B -->|No| D[Create and publish reporting properties]
    D --> E{Scope changed during creation?}
    E -->|Yes| F[Remove newly published stale entry]
    E -->|No| C
    C --> G[Create independent PropertyBag and TestNode]
    G --> H{Final state?}
    H -->|Yes| I[Clear context reporting cache]
    H -->|No| J[Keep cache for next update]
    K[ClearCaches] --> L[Rotate global scope]
    L --> M[Sweep registered contexts and clear properties]

Loading

<sub>Reviews (2): Last reviewed commit: "fix: release reporting caches after disc..." | Re-trigger Greptile</sub>

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c994193334

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[github-actions]

Review: perf: cache reporting properties on test contexts (#6791)

Summary of the change: Moves the reporting-property cache from a global ConcurrentDictionary<string, CachedTestNodeProperties> keyed by test ID to a field stored directly on each TestContext (CachedReportingProperties), invalidated via a rotating scope token. The follow-up commit (751f6d3) adds cleanup on terminal ToTestNode updates, on RemoveFromRegistry, and a new TestContext.ClearReportingCaches() sweep invoked from ClearCaches() to release entries for contexts that never reach a terminal state through the normal path (discovery-only sessions, pre-execution failures).

Correctness of the concurrency-critical path: I traced the "publish, then verify scope, then self-revert" pattern in GetOrCreateCachedProperties (TestExtensions.cs:48-137) against interleavings with ClearCaches()/ClearReportingCaches(). It holds up: the final Interlocked.CompareExchange(ref testContext.CachedReportingProperties, null, properties) only clears the field if it still references the exact object this thread published, so a newer, correctly-scoped write from another thread is never clobbered. I couldn't construct a scenario where a stale (pre-reset) CachedTestNodeProperties is returned to a caller computing against the new scope. Nice use of reference identity as a lock-free generation stamp instead of a lock.

Two suggestions, not blockers:

1. Test coverage gap for the exact race this PR fixes. Concurrent_Updates_Keep_Separate_Message_State only races ToTestNode calls against each other under a fixed scope — it never calls TestExtensions.ClearCaches() concurrently with ToTestNode/GetOrCreateCachedProperties. That's precisely the race the "scope check after publication" logic (and the PR description's second paragraph) claims to guard against, so right now it's verified only by inspection/manual benchmarking, not by an automated regression test. Worth adding a test that hammers ClearCaches() from one task while another repeatedly calls ToTestNode on live contexts, asserting no exceptions and that every returned node has non-null TestFileLocationProperty/TestMethodIdentifierProperty.

2. Consider keeping the cache off TestContext entirely. CachedReportingProperties is an untyped internal object? added to TestContext in TUnit.Core purely so TUnit.Engine has somewhere to stash engine-specific state (the comment even calls this out: "Owned by the engine; object keeps Core independent of MTP reporting types"). A ConditionalWeakTable<TestContext, object> held statically inside TestExtensions (Engine-side) would give the same per-context caching and the same explicit-clear semantics (Remove instead of Volatile.Write(..., null), and CWT supports enumeration for the ClearReportingCaches() sweep) without leaking an Engine concern into the public Core type at all. That keeps the Core/Engine boundary the codebase otherwise maintains (per CLAUDE.md) a bit cleaner, at the cost of a slightly slower lookup than a direct field — likely negligible next to the allocations this change already removes.

Everything else checks out:

• The benchmark methodology and numbers in the PR description are transparent about scope (isolated reporting-path microbenchmark, not whole-suite), which is good practice — the whole-executable comparison is explicitly called out as noisy/inconclusive rather than oversold.
• [NotInParallel] was correctly added to the new test class; per TUnit's semantics that isolates it from every concurrently running test, not just other [NotInParallel] tests, which matters here since ClearCaches()/ClearReportingCaches() touch process-wide state.
• Placeholder/deferred-enumeration tests (DeferredEnumerationExecutableTest, DeferredTestExpander) get their own independent TestContext per expanded child, so there's no stale-cache risk from metadata changing after an initial Discovered report — I went looking for that failure mode specifically and didn't find it.
• The three external bot reviews (Codex, CodeRabbit, Greptile) on this PR also found no actionable issues on the current revision, and the second commit's description shows the author already reasoned through and closed out the memory-retention findings from the first review pass.

No blocking issues found. The two suggestions above are about long-term maintainability/testability rather than correctness bugs.