InputField dataTest prop doesn't target the input itself

[martinkrulltott]

When using an InputField with the dataTest prop like this:
<InputField value={null} onChange={() => {}} dataTest={'mycustomname'} />
the data-test prop doesn't target the input itself, as seen in the outputted HTML:

       <div data-test="mycustomname-content" class="jsx-3375842594 ">
            <div data-test="dhis2-uicore-box" class="jsx-1220078726 ">
                 <div data-test="dhis2-uicore-input" class="jsx-3353877153 jsx-1090541828 input">
                      <input type="text" class="jsx-3353877153 jsx-1090541828 " value="">
                      <div class="jsx-3353877153 jsx-1090541828 status-icon">
                   </div>
               </div>
          </div>
     </div>
</div>

The mycustomname only makes it down 2 levels and is then replaced by dhis2-uicore-box and dhis2-uicore-input, while the input itself doesn't get a data-test prop at all.

If the purpose is to be able to target an element when testing, I suggest that the data-test propagates down all the way to the input, which is really what's being tested here.

[Mohammer5]

The currently applied strategy is to only add a data-test prop to the most outer dom element of each component.
The InputField uses two components, so there will be three dom elements with a data-test attribute (the InputFields, Boxes and Input's outer containers). We only pass down the dataTest value internally if a component consists of multiple components (like the Transfer component for example).

I'd suggest to just use [data-test="mycustomname-content"] input. That will make the code a bit more resilient to change as the internals and/or data-test strings could change, but the element you're trying to target is probably still an input

[HendrikThePendric]

I think guaranteeing the presence of a data-test attribute on the outer element(s) is as far as we should take this. On some components we may be passing down the base data-test string plus a suffix, but I definitely think we should NOT guarantee this is always the case. Cypress offers a lot of commands that can help a dev find the correct DOM element within the container with the data-test attribute.

[martinkrulltott]

Cypress offers a lot of commands that can help a dev find the correct DOM element within the container with the data-test attribute.

My concern with this approach is that any changes to the internal DOM tree structure could become a breaking change for testing purposes for apps. Putting a data-test prop (with a suffix if needed) at appropriate places decouples the app tests from the internal tree.

[ghost]

Putting a data-test prop (with a suffix if needed) at appropriate places decouples the app tests from the internal tree.

Yes, that is also the purpose as I see it. About the coupling of those data-test attributes to our versioning (which is how I read Hendrik's comment), I'm not sure. But I think Martin's definition should be leading our implementation of the data-test attributes, as that's their main (if not only) purpose.

[amcgee]

This doesn't seem to be a unique situation

• Transfer: 'pickedoptions' data test prop targets the wrong element? #219
• https://jira.dhis2.org/browse/LIBS-118

[varl]

It's not, and it's a very good discussion to have now that UI components are being used in various apps. It's not necessarily so that we got the data-test strategy right on the first (or second) try, and the input (no pun intended) here is very valuable to consider.

We don't want to tie tests to internal structure for the application tests, however that's fine for tests in UI. We have a consensus and commitment that breaking changes means that the public facing API has changed, but we are free to change the internals of UI components however we want. We cannot use the default recommendation then that App developers target UI component internals for testing purposes.

Obviously this bites of a large piece that we need to chew on for a bit, so for the purposes of what the answer to this question is today is to use: [data-test="mycustomname-content"] input for now while we work on revisiting how dataTest is implemented.

[amcgee]

I created a proposal to formalize our dataTest concept and approach, feedback welcome:

#224