Replace Report.ExpandClassTimeline with [ClassTimeline] attribute

[thomhurst]

Closes #5871.

Summary

• Replaces the global TUnitSettings.Default.Report.ExpandClassTimeline (added in Show multi-step test spans in class timeline, align report ordering with execution, and correlate linked OTel activities #5847) with a per-class opt-in attribute [ClassTimeline(TimelineMode.FullExecution)].
• Class-level usage wins over assembly-level via IScopedAttribute (same pattern as [NotInParallel]).
• Net code change in the reporter: zero. The attribute writes a tunit.report.timeline custom property at discovery; existing TestExtensions.cs plumbing already flows that into ReportTestResult.CustomProperties. The JS reads it from group.tests[0].customProperties to gate the timeline branch.

Why per-class beats a global flag

Most projects mix BDD [DependsOn] chains (where the multi-step view is gold) with independent-test classes (where it just adds noise). The single global toggle from #5847 forced an all-or-nothing trade. Per-class lets BDD classes opt in without polluting the rest of the report.

API surface change

Removed:

• TUnit.Core.Settings.ReportSettings
• TUnit.Core.Settings.TUnitSettings.Report

Added:

• TUnit.Core.ClassTimelineAttribute(TimelineMode) — [AttributeUsage(Class | Assembly)]
• TUnit.Core.Enums.TimelineMode { Collapsed = 0, FullExecution = 1 }

ReportSettings shipped in #5847 with no downstream consumers, so this is a clean swap rather than a breaking change in practice. PublicAPI snapshots updated for net4.7/net8/net9/net10.

Test plan

• TUnit.Engine.Tests/HtmlReporterTests (13/13 pass) — added two cases: attribute exposes Mode/ScopeType; tunit.report.timeline=FullExecution round-trips into the embedded report JSON via ReportTestResult.CustomProperties.
• TUnit.UnitTests/TUnitSettingsTests (6/6 pass) — reverted the _savedExpandClassTimeline snapshot/restore additions from Show multi-step test spans in class timeline, align report ordering with execution, and correlate linked OTel activities #5847.
• TUnit.PublicAPI Core net10 verified (snapshots updated for all four TFMs).
• Full TUnit.slnx build clean (0 errors, pre-existing warnings only).
• End-to-end smoke: drop [ClassTimeline(TimelineMode.FullExecution)] on a 3-test BDD chain in a sample project, run with the HTML reporter, confirm the class timeline shows test-case spans + child activities (and a sibling class without the attribute still shows only suite/init/dispose). Recommend the reviewer eyeballs this on real output before merge.

[codacy-production]

Not up to standards ⛔

🔴 Issues 1 critical

Alerts:
⚠ 1 issue (≤ 0 issues of at least minor severity)

Results:
1 new issue

Category	Results
Security	1 critical

View in Codacy

🟢 Metrics 0 complexity

Metric	Results
Complexity	0

View in Codacy

_{NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.}

[claude]

Code Review — Replace Report.ExpandClassTimeline with [ClassTimeline] attribute

Summary

Clean, well-motivated migration. Per-class opt-in is the right design for this use case — a global toggle is genuinely wrong for mixed codebases (BDD chains + independent test classes side-by-side). The IScopedAttribute pattern is the established TUnit convention for this kind of attribute-priority question. Overall this is good work; a few things worth calling out below.

---

What's good

• Design alignment — Using IScopedAttribute with ScopeType => typeof(ClassTimelineAttribute) mirrors [NotInParallel] exactly. Class-wins-over-assembly scoping comes for free from the runtime with no bespoke logic.
• Zero data-pipeline change — The attribute pushes data via DiscoveredTestContext.AddProperty at discovery time; the existing TestExtensions.cs → ReportTestResult.CustomProperties flow carries it through untouched. No new coupling between the core attribute and the reporter.
• Contract test is good — GenerateHtml_RoundTrips_ClassTimeline_CustomProperty_OnTest pins the C#/JS string contract ("tunit.report.timeline" / "FullExecution") at a level that will catch regressions if either side changes.
• JS defensive coding — isClassTimelineFullExecution correctly handles group.tests being falsy/empty before calling .some(...), so no runtime exception on an empty group.

---

Issues / Suggestions

1. [ClassTimeline(TimelineMode.Collapsed)] writes a custom property that has no visible effect

// ClassTimelineAttribute.cs
public ValueTask OnTestDiscovered(DiscoveredTestContext context)
{
    context.AddProperty(ClassTimelinePropertyKey, Mode.ToString()); // always writes
    return default;
}

When Mode == Collapsed, the JS check (value === 'FullExecution') returns false, so the property is written but ignored. This is not a bug, but it is wasteful and potentially confusing: a user who explicitly writes [ClassTimeline(TimelineMode.Collapsed)] gets no change in behaviour over having no attribute at all.

Suggested fix: guard the write, or document why the no-op write is intentional:

public ValueTask OnTestDiscovered(DiscoveredTestContext context)
{
    if (Mode != TimelineMode.Collapsed)
        context.AddProperty(ClassTimelinePropertyKey, Mode.ToString());
    return default;
}

Alternatively, remove Collapsed from the public API entirely — or give it a concrete opt-out semantic so the class-level value can override an assembly-level FullExecution. That second interpretation is arguably more useful: [ClassTimeline(TimelineMode.Collapsed)] on a class inside an assembly tagged [assembly: ClassTimeline(TimelineMode.FullExecution)] would explicitly collapse just that class. The current JS only checks for FullExecution, so Collapsed already works as an override in practice, but the docs for Collapsed say "Same as the implicit default" rather than "Explicit opt-out when assembly is FullExecution" — the intent should be clearer.

2. No test covering assembly-level usage or class-wins-over-assembly scoping

The PR adds [AttributeUsage(Class | Assembly)] and mentions IScopedAttribute handles precedence, but neither ClassTimelineAttribute_Exposes_Mode_And_ScopeType nor any other new test exercises the assembly-level path or proves that a class-level attribute overrides an assembly-level one.

The runtime machinery for IScopedAttribute is covered elsewhere, but a simple test asserting the expected scoping behaviour would make the contract explicit and protect against a future regression where someone changes ScopeType:

[Test]
public void ClassTimeline_ClassLevelWins_OverAssemblyLevel()
{
    // IScopedAttribute contract: same ScopeType → class-level instance wins
    var asm   = new ClassTimelineAttribute(TimelineMode.FullExecution);
    var klass = new ClassTimelineAttribute(TimelineMode.Collapsed);
    asm.ScopeType.ShouldBe(klass.ScopeType); // same type → runtime de-dupes by scope
}

This is minor, but the PR description explicitly calls out this scoping as a key design decision.

3. ClassTimelinePropertyKey is internal but the roundtrip test accesses it directly

// HtmlReporterTests.cs
new ReportKeyValue { Key = ClassTimelineAttribute.ClassTimelinePropertyKey, ... }

This works today only because InternalsVisibleTo (or a shared test assembly) grants access. If the visibility is narrowed, the test breaks silently. The string literal "tunit.report.timeline" is also hardcoded in embedded.ShouldContain(...) two lines later — so the constant isn't being used consistently within the same test. Either use the constant throughout, or accept the literal everywhere and drop the internal const. Mixed usage is the worst of both.

---

Dual-mode check (CLAUDE.md requirement)

ITestDiscoveryEventReceiver.OnTestDiscovered is part of the core interfaces invoked by both the source-gen and reflection paths. No source-generator changes are needed here since the attribute carries its own discovery logic. This appears to satisfy the dual-mode requirement.

---

Verdict

Approve with the above noted. Issue #1 (the Collapsed no-op semantic) is the most worth addressing before merge because it affects public API clarity. Issues #2 and #3 are minor and can be addressed in a follow-up if preferred.

[claude]

Code Review — Replace Report.ExpandClassTimeline with [ClassTimeline] attribute

Summary

Clean, well-motivated migration. Per-class opt-in is the right design for this use case — a global toggle is genuinely wrong for mixed codebases (BDD chains + independent test classes side-by-side). The IScopedAttribute pattern is the established TUnit convention for this kind of attribute-priority question. Overall this is good work; a few things worth calling out below.

---

What's good

• Design alignment — Using IScopedAttribute with ScopeType => typeof(ClassTimelineAttribute) mirrors [NotInParallel] exactly. Class-wins-over-assembly scoping comes for free from the runtime with no bespoke logic.
• Zero data-pipeline change — The attribute pushes data via DiscoveredTestContext.AddProperty at discovery time; the existing TestExtensions.cs → ReportTestResult.CustomProperties flow carries it through untouched. No new coupling between the core attribute and the reporter.
• Contract test is good — GenerateHtml_RoundTrips_ClassTimeline_CustomProperty_OnTest pins the C#/JS string contract ("tunit.report.timeline" / "FullExecution") at a level that will catch regressions if either side changes.
• JS defensive coding — isClassTimelineFullExecution correctly handles group.tests being falsy/empty before calling .some(...), so no runtime exception on an empty group.

---

Issues / Suggestions

1. [ClassTimeline(TimelineMode.Collapsed)] writes a custom property that has no visible effect

// ClassTimelineAttribute.cs
public ValueTask OnTestDiscovered(DiscoveredTestContext context)
{
    context.AddProperty(ClassTimelinePropertyKey, Mode.ToString()); // always writes
    return default;
}

When Mode == Collapsed, the JS check (value === 'FullExecution') returns false, so the property is written but ignored. This is not a bug, but it is wasteful and potentially confusing: a user who explicitly writes [ClassTimeline(TimelineMode.Collapsed)] gets no change in behaviour over having no attribute at all.

Suggested fix — guard the write, or document why the no-op write is intentional:

public ValueTask OnTestDiscovered(DiscoveredTestContext context)
{
    if (Mode != TimelineMode.Collapsed)
        context.AddProperty(ClassTimelinePropertyKey, Mode.ToString());
    return default;
}

Alternatively, give Collapsed a concrete opt-out semantic: [ClassTimeline(TimelineMode.Collapsed)] on a class inside an assembly tagged [assembly: ClassTimeline(TimelineMode.FullExecution)] would explicitly collapse just that class. The current JS only checks for FullExecution, so Collapsed already works as an override in practice — but the docs say "Same as the implicit default" rather than "Explicit opt-out when assembly is FullExecution". The intent should be clearer either way.

2. No test covering assembly-level usage or class-wins-over-assembly scoping

The PR adds [AttributeUsage(Class | Assembly)] and mentions IScopedAttribute handles precedence, but neither new test exercises the assembly-level path or proves a class-level attribute overrides an assembly-level one. A simple assertion would make the contract explicit:

[Test]
public void ClassTimeline_ScopeType_Is_Same_For_All_Instances()
{
    var asm   = new ClassTimelineAttribute(TimelineMode.FullExecution);
    var klass = new ClassTimelineAttribute(TimelineMode.Collapsed);
    // same ScopeType → runtime de-dupes by scope (class wins)
    asm.ScopeType.ShouldBe(klass.ScopeType);
}

Minor, but the PR description calls this out as a key design decision.

3. ClassTimelinePropertyKey used inconsistently in the same test

// HtmlReporterTests.cs — line that uses the constant:
new ReportKeyValue { Key = ClassTimelineAttribute.ClassTimelinePropertyKey, Value = nameof(TimelineMode.FullExecution) }

// Lines that hardcode the same string two lines later:
embedded.ShouldContain("\"key\":\"tunit.report.timeline\"");
embedded.ShouldContain("\"value\":\"FullExecution\"");

Either use the constant throughout (including the ShouldContain assertions), or accept the literal everywhere and drop the internal const. Mixed usage means a rename of the key constant won't fully protect the test.

---

Dual-mode check (CLAUDE.md requirement)

ITestDiscoveryEventReceiver.OnTestDiscovered is part of the core interfaces invoked by both the source-gen and reflection paths. No source-generator changes are needed here since the attribute carries its own discovery logic. This appears to satisfy the dual-mode requirement.

---

Verdict

Good change overall — approve with the above noted. Issue #1 (the Collapsed no-op semantic) is the most worth addressing before merge since it affects public API clarity. Issues #2 and #3 are minor and can be handled in a follow-up if preferred.