ComboBox testers: no clear(), no custom value, and multi-select replaces instead of toggling

[totally-not-ai]

Problem. Three things a user does with a ComboBox have no tester
representation.

No clear(). ComboBox implements HasClearButton, and emptying a field
is always available to the user (#168). ComboBoxTester.selectItem(null) is the
only route, which reads as "select nothing" rather than "the user cleared it",
and MultiSelectComboBoxTester has no equivalent at all.

No custom value entry. setAllowCustomValue(true) +
addCustomValueSetListener is a core ComboBox feature — the user types
something that isn't in the list and the app decides what to do with it. There
is no way to fire CustomValueSetEvent, so that whole branch of application
code is untestable.

comboBox.setAllowCustomValue(true);
comboBox.addCustomValueSetListener(e -> items.add(e.getDetail()));
// no tester method reaches this

MultiSelect replaces instead of toggles.
MultiSelectComboBoxTester.selectItem(String...) overwrites the whole
selection, so selecting a second item silently drops the first — but in the
browser each click toggles one chip. There is also no deselectItem, and no way
to remove a single chip via its ✕. MultiSelectListBoxTester gets this right:
selectItems adds to the existing selection and deselectItems removes.

Ask.

• clear() on both combo box testers, consistent with feat: give every value tester a uniform clear() #168.
• setCustomValue(String) (or similar) firing CustomValueSetEvent with
  isFromClient() == true, throwing when isAllowCustomValue() is false.
• Make MultiSelectComboBoxTester.selectItem additive and add deselectItem /
  deselectAll, matching MultiSelectListBoxTester.

The last one is a behavioural change and wants a release note.

Found while auditing the project against #153 / #168.

[totally-not-ai]

Looked into this against main (df099d0). All three asks are valid, but they are not equally clear-cut, so here is what the code actually does today plus a couple of related defects the audit didn't mention, and then the options for the one part that needs a decision.

What's there today

ComboBoxTester (ComboBoxTester.java#L107) — selectItem(null) short-circuits to setValueAsUser(null). So the behaviour of clearing exists; what's missing is the name. Confirmed: no clear(), no custom-value entry.

MultiSelectComboBoxTester (MultiSelectComboBoxTester.java#L83) — confirmed replace-not-toggle: it ends in getComponent().setValue(filtered), where filtered is built purely from the labels passed in, so the previous selection is dropped. No clear(), no deselectItem.

Related defects in the same two methods

Worth fixing in the same pass, since any new method here will want the same guards:

1. Neither selectItem calls ensureComponentIsUsable(). setFilter does, and CheckboxGroupTester/MultiSelectListBoxTester do it on every select/deselect. So today you can select an item in a read-only or disabled ComboBox — something the user cannot do. MultiSelectComboBoxTesterTest only covers the setFilter guard.
2. MultiSelectComboBoxTester selects server-side, not as the user. setValue(filtered) / deselectAll() produce a value change with isFromClient() == false, unlike every other selection tester (which routes through setValueAsUser). App code that branches on isFromClient() sees the wrong thing.
3. Unknown labels are silently ignored. The guard is filtered.size() < 1, so selectItem("test-foo", "typo") selects test-foo and says nothing. MultiSelectListBoxTester compares against items.size() and throws. Also, the message concatenates a String[], so it prints No item found for '[Ljava.lang.String;@1b6d…'.

On the individual asks

1. clear() — agreed, but it has an ordering dependency

clear() should be the same one-line delegate the other value testers are getting in #168, which introduces protected ComponentTester#clearAsUser() and explains why clear() must be declared on each tester (LocatorProcessor only walks the tester superclass chain, so an inherited method never reaches the generated locators). #168 is still open and does not touch either combo tester. Adding clear() here first would either duplicate that body or conflict with it, so this should land on top of #168, not beside it.

For MultiSelectComboBox the empty value is the empty Set, so the same helper works; it is the natural home for the "user clicked the ✕ on the field" gesture, distinct from removing a single chip.

2. setCustomValue(String) — agreed, mechanically easy, one semantic question

ComboBoxBase.CustomValueSetEvent(TComponent, boolean fromClient, String detail) is public, so this is a ComponentUtil.fireEvent one-liner in the style of MessageInputTester/ConfirmDialogTester. It applies to both testers — setAllowCustomValue lives on ComboBoxBase. Guards should be ensureComponentIsUsable() + IllegalStateException when isAllowCustomValue() is false.

The one decision: in the browser the user types the text (setting the filter) and then commits it with Enter. Should the tester (a) only fire the event, or (b) set the filter first and then fire, so a listener that inspects the filtered suggestions sees the same state as in the browser? (b) is the more faithful simulation; (a) is simpler and composes with an explicit setFilter() call. I'd go with (b), documented, since the point of the method is fidelity.

3. Additive multi-select — agreed on the problem, three ways to spell the fix

The issue cites MultiSelectListBoxTester, but the closer precedent is CheckboxGroupTester, the other multi-select tester: selectItem / selectItems / selectAll / deselectItem / deselectItems / deselectAll, all additive, all as-user, all usability-checked. That's the house style, and MultiSelectComboBoxTester is the outlier.

• Option A — match CheckboxGroupTester exactly. selectItem(String...) becomes additive; add deselectItem/deselectItems/deselectAll. Most consistent, smallest API surface, and the resulting tester reads identically to the other multi-selects. Breaking: any existing test that calls selectItem twice expecting a replace now accumulates — silent, no compile error. Needs the release note.
• Option B — additive under new names, leave selectItem alone. Add selectItems/deselectItems (additive) and deprecate selectItem. Nothing breaks. But selectItems would then be additive while selectItem replaces, which is a worse trap than the current state, and it contradicts CheckboxGroupTester where both are additive.
• Option C — make the replace explicit. Deprecate selectItem(String...), add additive selectItem/selectItems/deselectItem/deselectItems/deselectAll and setSelection(String...) for the "replace everything" gesture. No silent break (the deprecated method keeps its old meaning until removed), migration is mechanical. Costs one extra method and a deprecation cycle.

Recommendation: A, with the release note the issue already anticipates. The replace semantics aren't something a test can legitimately depend on — there is no browser gesture that produces them — and B leaves the API in a state that is harder to explain than the bug. C is the safe fallback if silent breakage in downstream test suites is judged unacceptable; if so, setSelection is the piece worth keeping.

Separately: selectItem(null) currently means "deselect all". Under A that becomes deselectAll()/clear(), and passing null should probably start throwing NullPointerException like MultiSelectListBoxTester does, rather than staying a second spelling of clear.

Suggested sequencing

1. Land feat: give every value tester a uniform clear() #168 (clearAsUser()).
2. On top of it: clear() on both combo testers + setCustomValue(String) on both + the three guard/consistency fixes above. Purely additive apart from fix 1 and 3, which only turn currently-silent wrong behaviour into an exception.
3. The multi-select toggling change as its own PR, once an option above is picked, so the release note has a single commit to point at.

Not opening a PR for now: step 2 is blocked on #168, and step 3 needs the API decision. Happy to implement either as soon as an option is chosen.

[mshabarov]

To be investigated: how item selections work on the client side - do they produce separate server side event for every user selection or do they aggregate the selection and send it in a single server call.

Then let's consider option C as a way to go.

[mshabarov]

Also would be better to split into few separate PRs.