Add `HasValue.setValue(V, boolean)` to allow pretending that the event came from the browser

[mvysny]

Describe your motivation

This TestBench UI test issue summarizes the problem nicely: https://github.com/vaadin/testbench/issues/1814 . I think this would be rather essential thing as application logic is very likely to have cases where it has if-statements checking if the event is from client and logic discards programmatic value changes. This is often necessary to prevent eternal loops etc.

Describe the solution you'd like

HasValue.setValue(V, boolean) would fire all events with isFromClient() returning true.

[Legioth]

I wouldn't want to bloat such a widely used API in that way. Do we have any other options?

[mvysny]

Yeah, that interface is widely used, any change could easily become an incompatible one. Even though we could implement the function as default one, simply throwing UnsupportedOperationException... Anyways, I think these alternatives would work equally well:

1. Add setValue(V, boolean) as a public method to AbstractField
2. Similar to 1, but create a HasValueExt which extends HasValue and adds setValue(V, boolean) function; then AbstractField would implement HasValueExt

[TatuLund]

One of the troubles from UIUnitTest perspective is that various fields override these methods.

ComboBox, DatePicker, Select, ...

https://github.com/vaadin/flow-components/blob/main/vaadin-combo-box-flow-parent/vaadin-combo-box-flow/src/main/java/com/vaadin/flow/component/combobox/ComboBoxBase.java#L524

So in order to make UIUnitTest work properly, these components should be changed to override setValue(V, boolean) instead. And user that default implementation of setValue is just:

setValue(value) {
  setValue(value, false);
}

So in order to make it work nicely, a quite evasive code change is needed in many classes.

[jcgueriaud1]

This is often necessary to prevent eternal loops etc.

Usually I'm using AbstractField#setModelValue(T newModelValue, boolean fromClient) for this purpose.

It's protected so it can't be used for TestBench

[Legioth]

How about creating a static helper that bypasses the protected status of that instance method without polluting the instance method namespace, e.g. AbstractField.setModelValue(fieldInstance, newValue, true)?

[mvysny]

@Legioth that would work for my case. Yet Tatu had a point that we should not bypass the validation of ComboBoxBase.setValue() for example.

[Legioth]

Is that validation necessary for UI unit tests? The main thing it would protect against is bugs in the unit test that makes it do things that a regular user couldn't do.

[mvysny]

Good point; for ComboBoxBase in particular the check might be bypassed. However, for example CheckboxGroup refreshes nested checkboxes; AbstractNumberField calls validate() and so does DatePicker; bypassing those validate() calls or refreshes might render the component state to be inconsistent. Also notice that RadioButtonGroup.setValue() updates nested radio buttons. This analysis is not exhaustive - I didn't went through all the components yet.

[TatuLund]

Is that validation necessary for UI unit tests?

@Legioth client side validation is irrelevant for UI Unit Tests ... But if the field has overriden the setValue to perform some operation e.g. with DataView, DataProvider, that is necessary for integrity.

[jcgueriaud1]

Another usecase for the application: Create a Field with an external Clear button. If a user is clicking on the clear button I would like to set the valu and keep the information that's coming from the client.

[Legioth]

Create a Field with an external Clear button.

Assuming the clear button is internal to the field itself, this could probably done using the protected setModelValue(T newModelValue, boolean fromClient) method?

[jcgueriaud1]

That means you have to extend the field like DateTimePicker, but yes that would work. Also if you have a list of fields with the Binder and a clear all button.