Skip to content

feat: hierarchical test organization with [Tag] attribute#4950

Closed
thomhurst wants to merge 2 commits into
mainfrom
feat/hierarchical-organization
Closed

feat: hierarchical test organization with [Tag] attribute#4950
thomhurst wants to merge 2 commits into
mainfrom
feat/hierarchical-organization

Conversation

@thomhurst

Copy link
Copy Markdown
Owner

Closes #4892. Adds [Tag] with dot-notation matching and --tag filter.

@claude

claude Bot commented Feb 19, 2026

Copy link
Copy Markdown
Contributor

Code Review

Overall the implementation is clean and follows established patterns (mirroring Category). Three issues worth addressing before merge:


1. Public API Snapshot Tests Not Updated

The PR adds multiple public members to TUnit.Core:

  • ITestDetailsMetadata.Tags (interface)
  • TestDetails.Tags (public property)
  • TestDescriptor.Tags + HasTag() (public members)
  • TagAttribute (new public class)

The CLAUDE.md rule is:

"Changes to source generator output or public APIs require running snapshot tests. Commit .verified.txt files."

The four TUnit.PublicAPI/Tests.Core_Library_Has_No_API_Changes.*.verified.txt files (for .NET 8, 9, 10, and 4.7) track the full TUnit.Core public surface. None were updated in this PR. The public API tests will fail when run — they already track ITestDetailsMetadata at that path but are missing the new Tags member, TagAttribute, and the additions to TestDetails and TestDescriptor.

Fix: Run the snapshot update (accept .received.txt as .verified.txt) and commit the four updated files.


2. TestDescriptor.Tags and HasTag() are Dead Code

TestDescriptor.Tags is initialized to [] and the XML doc comment says "Pre-computed at compile time to avoid runtime attribute instantiation" — but this is not true. Neither the source generator nor the reflection collector populates this field. The actual tag filtering path in TestFilterService reads test.Context.Metadata.TestDetails.Tags (the runtime list populated by TagAttribute.OnTestDiscovered), not descriptor.Tags. And HasTag() is never called from anywhere in the engine.

public string[] Properties { get; init; }
/// <summary>
/// Gets the pre-extracted tag values from TagAttribute.
/// Tags use dot notation for hierarchy (e.g., "integration.database.postgres").
/// Pre-computed at compile time to avoid runtime attribute instantiation.
/// </summary>
public string[] Tags { get; init; }

There are two ways to resolve this:

  • Option A (Simpler): Remove TestDescriptor.Tags and HasTag() since they're unused, and update the doc comment on MatchesHierarchicalTag to reflect it's only used via TestDetails.
  • Option B (More complete): Follow the same pattern as Categories — implement ExtractTags() in TestMetadataGenerator.cs and emit Tags = tagsArray in generated descriptors. This would enable future pre-filtering at the descriptor level before full materialization, which is how categories work. This would require updating source generator snapshots too.

The "Pre-computed at compile time" comment is actively misleading and should be corrected regardless of which option is chosen.


3. MatchesAnyTag Name Implies OR Semantics, But Implements AND

private static bool MatchesAnyTag(AbstractExecutableTest test, string[] tagFilters)
{
var tags = test.Context.Metadata.TestDetails.Tags;
foreach (var filterTag in tagFilters)
{
var matched = false;
foreach (var testTag in tags)
{
if (TestDescriptor.MatchesHierarchicalTag(testTag, filterTag))
{
matched = true;
break;
}
}
if (!matched)
{
return false;
}
}
return true;
}

The method returns false as soon as any single filter tag is unmatched — meaning --tag integration --tag slow requires a test to have both integration AND slow. This is AND/all-of semantics.

The name MatchesAnyTag strongly implies OR/any-of semantics (the test passes if it matches at least one tag). A name like SatisfiesAllTagFilters or MatchesAllTagFilters would accurately describe the behavior and prevent future confusion for contributors. This AND behavior is also worth documenting in the TagCommandProvider description string so users understand what multi---tag means.


The overall design is solid — the hierarchical dot-notation matching logic is correct, the ITestDiscoveryEventReceiver approach is consistent with CategoryAttribute, and the TestFilterService integration handles all filter combinations correctly. The feature works end-to-end. The above are the things that should be tidied up before merge.

Add support for hierarchical test categorization via a new [Tag]
attribute that uses dot-notation (e.g., [Tag("integration.database.postgres")]).
Filtering for a parent tag matches all children: --tag integration
matches integration, integration.database, integration.database.postgres.

- TagAttribute in TUnit.Core with ITestDiscoveryEventReceiver
- Tags collection on TestDetails/ITestDetailsMetadata
- HasTag with hierarchical matching on TestDescriptor
- --tag CLI option via TagCommandProvider
- Tag-based filtering in TestFilterService (works alongside tree filters)
- Tag properties in test node PropertyBag for platform reporting
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat: hierarchical test organization and advanced tag filtering

1 participant