Skip to content

fix(engine): apply --treenode-filter to --list-tests#6334

Merged
thomhurst merged 1 commit into
mainfrom
feat/list-tests-respect-treenode-filter
Jun 30, 2026
Merged

fix(engine): apply --treenode-filter to --list-tests#6334
thomhurst merged 1 commit into
mainfrom
feat/list-tests-respect-treenode-filter

Conversation

@thomhurst

Copy link
Copy Markdown
Owner

Problem

--list-tests ignored --treenode-filter and listed every test in the assembly. Users expect app --list-tests --treenode-filter X to list only the tests filter X selects — the same set a real run would execute.

Root cause

MTP routes --list-tests to a DiscoverTestExecutionRequest, which flows through TestDiscoveryService.DiscoverTests with isForExecution: false. The filter was applied only when isForExecution was true:

var filteredTests = isForExecution ? _testFilterService.FilterTests(filter, allTests) : allTests;

isForExecution legitimately gates side-effecting work — argument registration / fixture creation (#6151) and dependency-closure expansion. But filtering itself is pure (MatchesTest only compares the test's path/properties against the filter), so running it during discovery was always safe; it just was never wired up. Filtering got conflated with the side-effect gate.

Fix

Apply the filter during discovery too, but only when a real filter is present:

var hasRealFilter = filter is not (null or NopFilter);
var filteredTests = (isForExecution || hasRealFilter)
    ? _testFilterService.FilterTests(filter, allTests)
    : allTests;
  • No filter (bare --list-tests, IDE Test Explorer)null/NopFilter → returns all tests unchanged, including [Explicit] ones (which FilterTests would otherwise strip). No behavior change.
  • Filter present → reuse FilterTests, so the listing mirrors exactly what a run with that filter would execute.
  • No fixture leaksRegisterTestsAsync is still called with isForExecution: false, so RegisterTestArgumentsAsync stays skipped.
  • No dependency pull-in — the dependency-closure block stays guarded by isForExecution.

Engine-runtime only; works identically in reflection and source-generated modes since both flow through TestDiscoveryService. No snapshot/source-gen output changes.

Tests

Adds ListTestsFilterTests (reflection + source-generated modes):

  • filter lists only matching tests,
  • bare --list-tests still lists everything (guards the no-filter path).

Manually verified against TUnit.TestProject:

Check Result
source-gen + [Category=Pass] filter only Pass*, 0 Fail*
reflection + filter 86 Pass / 0 Fail
no filter full 18,138 listed, Pass1+Fail1 present
new engine test (4 cases) all pass

Discovery requests (--list-tests, IDE Test Explorer) flow through
TestDiscoveryService.DiscoverTests with isForExecution=false, which skipped
the filter entirely and listed every test. The isForExecution gate exists for
side-effecting work (argument registration / fixture creation #6151, dependency
closure expansion), but filtering itself is pure (MatchesTest only compares the
test path/properties), so it was always safe to run during discovery.

Now apply the filter during discovery when a real filter is present. Bare
--list-tests and IDE discovery send null/NopFilter and keep listing everything
(including [Explicit] tests, which FilterTests would otherwise strip). When a
filter is present, reuse FilterTests so the listing mirrors what a run would
execute. No fixture leaks: RegisterTestsAsync is still called with
isForExecution=false.

Adds ListTestsFilterTests covering reflection + source-generated modes.
@codacy-production

Copy link
Copy Markdown

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 7 complexity

Metric Results
Complexity 7

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

claude Bot commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

Summary: The fix correctly widens filtering in TestDiscoveryService.DiscoverTests to also apply during discovery-only requests (isForExecution: false) when a real filter (filter is not (null or NopFilter)) is present, while leaving bare --list-tests/IDE Test Explorer discovery (null/NopFilter) and side-effecting work (RegisterTestsAsync, dependency-closure expansion) unchanged. Verified:

  • Filtering via TestFilterService.FilterTests is side-effect-free, so applying it outside the isForExecution gate is safe.
  • The new ListTestsFilterTests.cs exercises both reflection and source-generated modes (Dual-Mode compliance) and follows the established InvokableTestBase/ExternalCancellationTests conventions for driving the built test executable, with a documented reason for not reusing InvokableTestBase directly.
  • No source-generator output or public API changes, so no snapshot updates were needed.
  • The intentional tradeoff (dependency-closure expansion stays gated on isForExecution, so listing with a filter won't show pulled-in dependency tests that a real run would execute) is clearly documented in the PR description.

🤖 Generated with Claude Code

@thomhurst thomhurst enabled auto-merge (squash) June 30, 2026 21:48
@thomhurst thomhurst merged commit 4b75dc1 into main Jun 30, 2026
13 checks passed
@thomhurst thomhurst deleted the feat/list-tests-respect-treenode-filter branch June 30, 2026 22:00
@claude claude Bot mentioned this pull request Jun 30, 2026
1 task
This was referenced Jun 30, 2026
This was referenced Jul 2, 2026
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.

1 participant