Add testcase filter using parameters

[Diksha-Garg]

Bug / Requirement Description

Clearly and concisely describe the problem.

Solution description

Describe your code changes in detail for reviewers.

Checklist:

• Test
• Example (both test_plan.py and .rst)
• Documentation (API)
• News fragment present for release notes
• MS info leakage check
• For new driver: driver index page
• For new assertion: ui/pdf/std renderers, documentation
• For new cmdline arg: documentation

[yuxuan-ms]

1. testplan/web_ui/testing/src/Toolbar/ExtendedSearchDropdown.js:168 — blocker

null and "" are legitimate parametrization values, but they collide with the "Any" sentinel, so selecting them silently disables filtering instead of narrowing.

handleParamChange (line 403) uses "" as the token for "Any" but stores the value itself in selectedParams. applyParamFilters then decides "nothing selected" from the value: v === "" || v == null. A parameter whose value is None (which _serialize_parametrization_value maps to JSON null) or "" therefore always takes the "Any" branch.

Transcribing the two functions and running them:

perms = [
  { name: "case 0", params: { note: null } },
  { name: "case 1", params: { note: "hi" } },
  { name: "case 2", params: { note: ""   } },
]

select note=null (token object:null) -> 3 results: case 0, case 1, case 2
select note=""   (token string:"")   -> 3 results: case 0, case 1, case 2
select note="hi" (token string:"hi") -> 1 result:  case 1

The <select> still shows the value as selected and the "Permutations (N)" count updates to the unfiltered total, so there's no signal to the user that the filter was ignored.

Suggested fix: decide "is this filter active" from key presence, not from the value — _.every(Object.entries(selected), ([k, v]) => _.isEqual(p.params[k], v)) combined with handleParamChange deleting the key for "Any" (which it already does). Alternatively store tokens rather than values and compare paramValueToken(p.params[k]) === token, which also removes the _.find lookup.

Worth adding null and "" cases to the applyParamFilters describe block. Note that it("treats empty string as 'Any'") currently pins the buggy behaviour as intended, so that test needs to change too.

2. testplan/web_ui/testing/src/Toolbar/ExtendedSearchDropdown.js:428

This backslash escaping can't work — the search grammar has no escape support, so the text written back into the filter box becomes unparseable.

SearchFieldParser.pegjs:29 is:

EXACT_SEARCH = DOUBLE_QUOTE letters:[^\"]* DOUBLE_QUOTE

[^"]* terminates at the first " regardless of what precedes it. For a testcase named a"b, the emitted text c:"a\"b" parses as case: ["a\"], then b, then a dangling " that neither WORD nor whitespace can consume — so the next keystroke in the filter box raises a SyntaxError, the error highlight turns on, and onFilterChange pushes filters: [], clearing the filter.

The filters array you hand to handleNavFilter uses the raw unescaped name, so the filter applied at click time is correct; only the round-trip through the visible text is broken. That also means the replace on this line and on line 434 is dead code today.

Options: teach the grammar an escape rule, or when a name contains " skip the text and pass only filters (or fall back to a re: term).

3. testplan/web_ui/testing/src/Toolbar/ExtendedSearchDropdown.js:443

The generated filter is broader than the permutation the user actually clicked, so the nav tree keeps siblings around. Two causes stack:

1. name_filter (reportFilter.js:12) matches by substring, not equality. With name_func=None the generated names are test_order 0 … test_order 11, so c:"test_order 1" also keeps test_order 10 and test_order 11.
2. filters only pushes suite and case. Even when the user narrowed by Test in the dropdown, no { type: "test", search: [...] } term is emitted, so identically named suite/case pairs in other multitests survive the filter.

Navigation itself is accurate (it goes through uids); it's the surrounding tree that's wrong. Adding the test term when permutation.testName is set fixes (2). For (1), an anchored re: term would be exact, at the cost of escaping regex metacharacters in the name.

4. testplan/report/testing/base.py:86

Running the sanitiser inside TestCaseReport.__init__ means a problem in user-supplied parametrization data now fails report construction, and the output has no size bound.

Putting the conversion in __init__ rather than in the schema is the right call — the dill.dumps(report) assertion in the new unit test shows the goal is for the report object to survive pickling across process boundaries, which a schema-level fix can't achieve. The concern is the failure modes of the fallback str(value) on line 103:

• A self-referential list or dict raises RecursionError.
• A __str__ that raises propagates straight out. MultiTest._new_testcase_report and dry_run have no guard on this path, so the whole report fails to build.

Both are new exposure: with name_func=None the parameter values were previously never stringified at all.

There's also no truncation. str() of a large object (DataFrame, long list, big dict) gets embedded in every testcase node, and for v3+ reports that lands in the structure file the UI loads eagerly — so a heavily parametrized plan pays for it in report size and UI load time.

Suggestions: wrap the str(value) fallback in a try/except returning something like f"<{type(value).__name__}>", cap recursion depth and string length, and consider whether a large str() should be stored at all.

Minor, same function: a set falls through to str(value) and yields e.g. "{1, 2, 3}", whose ordering depends on PYTHONHASHSEED, so report content isn't reproducible across runs for set-valued parameters.

Also worth noting: schemas.py already has the NaN/Infinity conversion in EntriesField._serialize (via boltons remap). Two copies of the same scalar-sanitising rules will drift; extracting one shared helper would be better even though the two call sites can't be merged.

5. testplan/web_ui/testing/src/Toolbar/ExtendedSearchDropdown.js:531

Once a testcase is selected this combobox can't be edited — the user has to clear the whole field to pick a different one.

value={selectedTestcase || searchText} means the input displays selectedTestcase after a selection. Typing one more character produces test_orderx, which handleTestcaseSearch doesn't find in uniqueTestcaseNames, so it clears selectedTestcase and sets searchText to test_orderx — and the option list renders "No matching testcases found".

Seeding searchText from selectedTestcase on mount (and in handleTestcaseSelect), or clearing the displayed value on focus, would make it behave like a normal combobox.

6. testplan/web_ui/testing/src/Toolbar/ExtendedSearchDropdown.js:57

result.push(...collectTestcases(...)) throws on large reports, and the memo re-walks the whole tree on every interactive poll.

The spread is bounded by the call stack. Measured on node 24:

50000  ok
100000 ok
125000 ok
150000 FAIL Maximum call stack size exceeded

So a multitest subtree with roughly 10^5+ testcases takes down the dropdown with RangeError: Maximum call stack size exceeded. A plain for loop pushing one at a time, or threading an accumulator array through the recursion, removes the limit.

Separately, allTestcases memoises on [report], and in interactive mode POLL_MS = 1000 hands down a fresh report object every second, so the full tree walk repeats at 1 Hz while the dropdown is open. Depending on report.hash instead would skip the no-op rebuilds.

7. testplan/web_ui/testing/src/Toolbar/ExtendedSearchDropdown.css:1 — nit

Styling convention: the rest of the web UI uses aphrodite (StyleSheet.create + css()), and index.css is currently the only plain CSS import in src/. This adds a second one, with unscoped global class names. Not a problem today given the extended-search- prefix, but it's a new pattern — worth a deliberate decision rather than drifting into it.

(The hardcoded light-mode colours are fine — there's no theme/dark-mode support in this UI.)

[yuxuan-ms]

ExtendedSearchDropdown: three behaviour issues, plus some size

1. The generated filter still keeps sibling multitests whose names share a prefix

handleResultClick now emits {type: "test", search: [test]}, but name_filter (Report/reportFilter.js:11) matches by substring, not equality. Two multitests named MyTest and MyTest2 with the same suite and testcase names: clicking the permutation under MyTest leaves both in the nav tree. I transcribed the emitted filters through PropagateIndices + filterReport and got

report-1/mt1/ts1/test_order/test_order__0
report-1/mt2/ts1/test_order/test_order__0

where only the first was clicked. The testcase name is exact now (re:"^…$"), so this is only about the test/suite terms.

Emitting an anchored regexp for the test name as well should fix it — mt2's entries carry MyTest2|multitest in name_type_index, so ^MyTest$ won't match them, while mt1's subtree matches both anchored terms:

filters.push({ type: "regexp", search: `^${_.escapeRegExp(test)}$` });

Worth a test with two multitests sharing a name prefix.

2. Typing the full testcase name clears the input box

handleTestcaseSearch sets setSearchText("") on an exact match (line 286) while the input renders value={searchText}. So when the user types the last character of test_order, the field goes blank and shows the placeholder, even though the selection took effect and the permutations render below. The next keystroke then starts from an empty string rather than extending what was typed.

Clicking an option goes through handleTestcaseSelect, which sets searchText to the name (line 311), so the two paths disagree. Setting searchText to value in the exact-match branch makes them consistent.

3. text: "" and interactive mode

Skipping the text terms when a name contains " is the right call, but it creates a state that didn't exist before: filters are applied while filter.text is empty. InteractiveReport.js:538 uses the truthiness of filteredReport.filter.text as its "is a filter active" signal, and when it's falsy shallowReportEntry doesn't attach the pruned subtree — so "run all" would run the unfiltered set while the tree on screen is filtered. This is from reading the code, I haven't exercised that path. Narrow trigger (a testcase name containing a double quote), but it's worth either widening that check to filter.filters?.length or keeping the text non-empty in some other form.

---

Size and structure

The component is 756 lines: 108 helpers, 60 useMemo, 147 handlers, 218 render, 113 styles. A few things account for most of it.

The selection state is stored twice. FilterBox holds extendedSearchState with exactly the four fields the dropdown also keeps in useState, so every handler writes them twice — once through the setter, once through persistState with a hand-written patch object (6 call sites: 260, 275, 289, 298, 314, 331). That duplication has already produced two artefacts:

• searchText isn't in the persisted set, so it drifts out of sync with selectedTestcase (issue 2 above is part of this)
• there are two reset mechanisms for the same event: key={this.props.report?.uid} on the dropdown already remounts it with fresh state, and componentDidUpdate → resetExtendedSearchState() clears FilterBox's shadow copy

Making the dropdown controlled — value and onChange both from FilterBox — removes the four useState, the persistState wrapper, the six patch objects and one of the two reset paths.

8 useMemo, where the rest of src/ has 0. Only allTestcases walks the whole report tree; uniqueTestNames, uniqueTestsuiteNames, uniqueTestcaseNames, filteredTestcaseNames, filterOptions and filteredPermutations are uniq/filter/sortBy over a few dozen entries. Dropping those six also removes the eslint-disable-next-line react-hooks/exhaustive-deps.

Single-use helpers exported for tests. enrichWithParams (3 lines), getPermutations and compareParamValues are each used once. Inlining them and testing the behaviour through the component ("selecting quantity=1000 leaves one permutation") would cut a few hundred lines across the component and its test file. collectTestcases, getUniqueField, buildFilterOptions and applyParamFilters have real logic and are worth keeping exported.

Test hooks in the markup. data-testid appears 8 times here and 0 times anywhere else in src/; role= 6 times vs 0; aria-* 6 times vs 3 (in two files). The existing component tests select with .find("select"), .find(Component) and snapshots — worth following that rather than introducing a second convention.

Dead branch. In handleResultClick, !onNavigate can't be true: it's PropTypes.func.isRequired and FilterBox:185 only renders the dropdown when onExtendedSearchNavigate is set. !reportUid is a real guard (report can be null), keep that half.

compareParamValues isn't a total order. Once a parameter mixes types, 1000 and "1000" compare equal, "1000" < 2.5, but 1000 > 2.5, so the result depends on input order. [1000, "1000", true, null, "", 2.5, 500] currently renders as "", 2.5, 1000, 1000, 500, Buy, false, null, true. If parameter values end up as scalars only, one line covers it and sorts numbers correctly:

const compareParamValues = (left, right) =>
  String(left).localeCompare(String(right), undefined, { numeric: true });