Skip to content

Fix TreeNodeFilter OR-pattern diagnostics#7415

Open
Evangelink wants to merge 4 commits into
mainfrom
dev/amauryleve/tree-node-filter
Open

Fix TreeNodeFilter OR-pattern diagnostics#7415
Evangelink wants to merge 4 commits into
mainfrom
dev/amauryleve/tree-node-filter

Conversation

@Evangelink
Copy link
Copy Markdown
Member

Description

  • Fixes https://github.com/--treenode-filter pattern matching is at least non obvious, potentially errored #7300 by clarifying behavior and improving failure diagnostics for unsupported parenthesized full-path OR patterns.
  • Root cause: Tree-node OR is valid within a single path segment, but patterns like (/A/B/C/X)|(/A/B/C/Y) hit separator processing inside parentheses and throw a generic invalid operation.
  • Changes:
    • Add regression tests for supported single-segment OR and exact-match behavior in TreeNodeFilterTests.cs.
    • Improve parser exception guidance and grammar remarks in TreeNodeFilter.cs.
    • Add changelog entry in Changelog-Platform.md.

Validation

Notes

This change does not expand grammar to support full-path OR in parentheses; it adds explicit guidance and test coverage for current supported syntax.

Copy link
Copy Markdown
Member

@Youssef1313 Youssef1313 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have doubts that the problem here is clarity of error message. The behavior seems buggy IMO. Let's discuss offline.

Copilot AI review requested due to automatic review settings May 16, 2026 12:20
@Evangelink
Copy link
Copy Markdown
Member Author

Merged latest main into the PR branch (was ~941 commits behind). The merge was clean with no conflicts. Verified the fix is still relevant — TreeNodeFilter.cs on main has not received an equivalent fix.

Verification:

  • Built Microsoft.Testing.Platform.csproj (Debug): 0 warnings, 0 errors.
  • Ran Microsoft.Testing.Platform.UnitTests filtered to TreeNodeFilter: 165 passed / 0 failed across net8.0, net9.0, net48.

Ready for CI re-run and review.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR clarifies TreeNodeFilter behavior for OR patterns and improves diagnostics when unsupported full-path OR expressions are parenthesized.

Changes:

  • Adds regression coverage for single-segment OR patterns and exact-match behavior.
  • Updates TreeNodeFilter grammar remarks and propagates filter text into parser error construction.
  • Improves the unexpected slash diagnostic for parenthesized full-path OR patterns.
Show a summary per file
File Description
test/UnitTests/Microsoft.Testing.Platform.UnitTests/Requests/TreeNodeFilterTests.cs Adds tests for supported OR syntax and unsupported parenthesized full-path OR diagnostics.
src/Platform/Microsoft.Testing.Platform/Requests/TreeNodeFilter/TreeNodeFilter.cs Documents OR syntax limitations and augments the unexpected slash exception message.

Copilot's findings

  • Files reviewed: 2/2 changed files
  • Comments generated: 1

throw new InvalidOperationException(PlatformResources.TreeNodeFilterUnexpectedSlashOperatorErrorMessage);
throw new InvalidOperationException(
$"{PlatformResources.TreeNodeFilterUnexpectedSlashOperatorErrorMessage} "
+ $"Filter: '{filter}'. To combine alternatives for one path segment, use syntax like '/A/B/C/(X|Y)'.");
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved the string to PlatformResources.resx as TreeNodeFilterUnexpectedSlashOperatorInPathSegmentErrorMessage so it's localized with the rest of the TreeNodeFilter diagnostics. XLFs regenerated via UpdateXlf.

Copilot AI added 2 commits May 16, 2026 16:04
The merge of origin/main into this branch brought in many new resx entries
(IsGreaterThan, IsLessThan, IsPositive, IsNegative, IsInRange, Contains*,
DoesNotContain*, ContainsSingle, etc.). Regenerated all 13 XLF files via
'dotnet msbuild /t:UpdateXlf' to keep them in sync.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Move the inline hint diagnostic appended to TreeNodeFilterUnexpectedSlashOperatorErrorMessage into a new PlatformResources entry (TreeNodeFilterUnexpectedSlashOperatorInPathSegmentErrorMessage) so the full message is localized like the surrounding parser diagnostics. XLFs regenerated via UpdateXlf.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings May 16, 2026 15:07
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

  • Files reviewed: 29/29 changed files
  • Comments generated: 1

Comment on lines +62 to +66
/// OR expressions are supported for a single path segment, for example:
/// <c>/A/B/C/(Test1|Test2)</c>.
///
/// OR expressions over full paths are not supported, for example:
/// <c>(/A/B/C/Test1)|(/A/B/C/Test2)</c>.
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.

4 participants