data-test-id attributes on all elements in the react (and other?) apps

[Mohammer5]

Original message by @Philip-Larsen-Donnelly on Slack:

Hi guys. How possible do you think it would be to get data-test-id attributes onto all elements in the react (and other?) apps?

• I would envisage this being done in an automated way, for convenience.
• I personally think the id's don't have to be "nice" names. The important thing is they they are unique (per app) and constant (once defined they stay the same for that element)
• we need to handle the component libraries, maintaining uniqueness within the app.
• could we enforce it as part of style? (So that we can ensure commits with new elements are compliant?)
• can we deal with removed elements, so that we avoid reusing previous id's? (Maybe a file in the app repo with all used id's; even if since removed).
• from my point of view the attributes should be in production code, but we could also have a strategy for removal).

Is this something we could get in in the short to medium term?

[Mohammer5]

How possible do you think it would be to get data-test-id attributes onto all elements in the react (and other?) apps?

All elements like in all dom-elements? => Impossible... All components... Not impossible, but it'd take a LOT of time.

Automating this will be difficult as well, it needs to be done on the codebase itself, it can't be done at runtime (plus we need to keep track of used IDs)

we need to handle the component libraries, maintaining uniqueness within the app.

We could add a test-id prop to the components, it'd be an app's responsibility to provide those though, this can't be done by the component libraries themselves. But there should be only as many data-test-id attributes per component as needed and as few as possible (a Button component needs one.. an OrgUnitTree definitely needs more)

could we enforce it as part of style? (So that we can ensure commits with new elements are compliant?)

Maybe there's a way to force devs to provide a data-test-id property to every non-built-in component, but it will make the codebase very messy, resistant to change and will include many components that won't be needed while testing an app .

If we REALLY want to go this way, I think we need to do this:

1. Every component of our apps (& every component provided by component libs) that isn't a page component (page component = a component for a route) should accept data-test-id for both itself and all components it renders itself. (This will be interesting when refactoring stuff / changing features)
2. These data-test-ids should be defined/passed down in the page components

This way:

• we'd have one place where we could find all IDs for a page (adding more shouldn't require too much work, relatively speaking of course)
• it should be fairly simple to extract the used IDs and store them in a file that's tracked/versioned (.test-ids?)
• Components stay reusable

Conclusion

In my opinion automating this is either impossible or will take VERY long, so we should make it a rule to add data-test-ids to new code (everyone should make sure to have that in mind when reviewing PR code)

I do get that it makes sense to use these attributes for testing. But we only really need them on elements that can be interacted with (for automated interaction) and semantically important elements (for checking if something is present in the view or not), right?

So I think it makes a lot more sense to pay attention to that while developing and reviewing PRs

[ismay]

Could we have something like a babel plugin for this? Something that bases the id on a hash of the relevant parts of a component (so we don't reuse ids)? Because doing this manually would be hell. Plus having it directly in the codebase does not sound good to me at all.

Hopefully we could then have the babel plugin do its thing only when necessary, and ignore it in all other situations. Complete speculation this, but just thinking of what implementation I'd prefer if it were possible.

[Philip-Larsen-Donnelly]

I was assuming having it in the codebase would be the only way of keeping it consistent from one build to the next, but if there are clever ways of achieving the same thing then that's fine. If it's not in the codebase and you move a component (e.g. to a different location on the page) might that not change the ID?

What is wrong with having a static identifier in the codebase? (sorry if that's a stupid question to you guys - you may have to humor me here!). If you deliberately code some functionality into a component, why not be able to identify that component unambiguously at runtime?

However, I also don't want to introduce a process that makes the code/development process really messy! I don't know anything about babel plugins, but if we can use that to get something close to consistent IDs (so that the majority of tests remain valid) then that would still be very valuable.

I just want to clarify what the use cases are:

1. External testers: teams such as the PEPFAR (or any other) who need to write and maintain comprehensive automated tests (without modifying the codebase).
2. Test Recording: Converting manual tests to automated tests (where applicable) to build up our automated regression suite, would be extremely useful for the increasing complexity of the release cycles. We should avoid any negative impact on developer testing with things like cypress too, and maybe facilitate it, but I don't see that as a key use case as the devs have control to add IDs as needed there anyway.

[Mohammer5]

Could we have something like a babel plugin for this? Something that bases the id on a hash of the relevant parts of a component (so we don't reuse ids)?

If it's not in the codebase and you move a component (e.g. to a different location on the page) might that not change the ID?

That'd be the consequence, yes. Personally, I don't see a way around doing it manually.

We could do it in a way that all our components require a test-id attribute. All components inside those components could be given a standardized test-id prefixed with the test-id passed to their parent. I. e. the <OrganisationUnitTree /> expects one and all components rendered inside will use that one as a prefix + static. As the rendering is still dynamic, the actual rendered test-ids can vary (when changing org units in the maintenance app for example, which will require the tests to be adjusted, but this would be the case no matter the tech I suppose)

We'd have to use the same approach in our apps though. Which makes it ridiculously hard to add it to existing apps with little effort, it'd be an enormous task.

It's something we could apply when refactoring/adding code.

[ismay]

What is wrong with having a static identifier in the codebase?

That is my personal opinion. I've not had to support something like this before. Might be that that is the only way. But my reason for not liking the idea initially (besides testing which is great of course) is that it seems messy to me. Especially checking if we're not reusing id's. It seems very manual and prone to breakage. Which is why I was wondering aloud if we could automate it, which wouldn't pollute the codebase and be (hopefully) more reliable.

[vilkg]

Unique attributes are extremely important for testability to us internally and to PEPFAR test team, which complained about dhis2 being not designed to be testable during the conference. My main concerns are that no matter which way you choose (manual or automatic, in the code-base or not), id's should be:

• Same across dhis2 versions. Probably not possible, but would be great to back-port. Otherwise should be locked in from the point it was introduced. Of course, shared libraries, like header-bar, should have same identifiers across all apps.
• Unique at least within the app (shouldn't be the same as something in shared libs)
• Applied to components, buttons, dropdowns, input fields within components.
• Preferably, have somewhat representative name, perhaps could start with react component name?

I understand that manual effort to make this happen would be huge and I could definitely help with PR´s if you decide to go that way.

[Philip-Larsen-Donnelly]

In terms of effort, is there possibility of scripting some easy parts and doing the rest manually? If we have mostly manual effort (I assume that means it would have to be in the codebase?) then I think we would also need a way to enforce it.

Finally, to follow on from Gintare's last point about helping with effort: If we can find an acceptable solution, but the only obstacle is effort, particularly if it is just grunt work, we might be able to secure funding and resources to help us with that effort.

[ismay]

I'd be interested in seeing what solutions are out there already. I think it's probably a quite common thing (adding predictable ids for testing). I assume bigger organisations than ours have probably already dealt with this problem at scale. Would be good to learn from their solutions.

[amcgee]

We have introduced some test IDs to the apps that are using Cypress, FWIW. Those have all been done manually.

It's very challenging to do this automatically, particularly since IDs are most useful by instance of a component, not by component. When thinking about component libraries this means the app has to actually specify the test ID, not the library, and so there's not an easy way to guarantee cross-app consistency (with the exception of components which fall outside the "app scope" and therefor eventually live in the common AppShell wrapper - inversion of control helps us a lot here).

There may be some slight utility in component-level test IDs, but you can actually get most of the same information by parsing the React tree at runtime (think react-dev-tools) to search for the component you want. I'd love to learn from other orgs if they have managed it but I've never heard of success at scale. Cypress recommends manual ID specification, last I looked.

[Mohammer5]

We don't have to follow any standards as long as we come up with a proper solution, right?

We could have both data-test-id and data-test-class, similar to the id and class attributes. That way the libraries' component internals would have reusable identifiers across apps and a unique id per component.

This would also allow to simplify a lot of tests (say you're on a page with only one org unit tree, why would it need it's own unique id, querying the data-test-class should suffice until the page has more than one org unit tree which most-likely results in adjustment of the test anyways). It'd simplify developing because IDs only have to be added when necessary.

[ismay]

We don't have to follow any standards as long as we come up with a proper solution, right?

Even if our solution is proper, following standards helps new developers understand what we're doing (besides the other advantages of following what others do, like it being maintained by others, interoperability, etc.).

[Mohammer5]

Even if our solution is proper, following standards helps new developers understand what we're doing (besides the other advantages of following what others do, like it being maintained by others, interoperability, etc.).

I agree with that, but when following standard makes testing much more difficult, I'd prefer to build on existing standards and extend them (which would be: introducing data-test-class but keep data-test-id)

[ismay]

I agree with that, but when following standard makes testing much more difficult, I'd prefer to build on existing standards and extend them (which would be: introducing data-test-class but keep data-test-id)

Yeah definitely. I've been trying to find some documentation / writeups on this, but couldn't find much. I assume the people at Cypress did plenty of research, so that's in favor of manual then I'd say,

[varl]

One option is to go the divide and conquer way, and approach this like a testing pyramid. Do not get too attached to the the terms he uses, but keep the concept in mind.

I do not believe that we should treat all of our apps as a single behemoth to test cohesively, which is the implication of the strategy to automatically generate consistent test id attributes unique across all libraries and applications.

We are actively building a system architecture that does not have the traits which would lend itself to that strategy. Our applications are eventually consistent when it comes to dependencies, and the application platform we are moving towards will not make it always consistent off-the-bat.

I understand that this is a big mental shift, but DHIS2 is not one system anymore, it is a system of systems, much more akin to an operating system than a single piece software. And we need to approach it as such on all levels from development to testing.

A given DHIS2 instance will still have multiple versions of e.g. ui-core or ui-widgets bundled with different applications. Eventually, as all apps are updated on the instance with new builds, they will approach the same ui-core version, but it will never stay entirely consistent (until we replace the existing portal with the app shell so a single instance of it is used at runtime, in like 3 years).

Instead, we need to embrace the boundaries that we establish as part of the overall architecture, and test in accordance to that. I am going to use the term "functional testing", and what I mean by that is "testing done in a browser environment".

• Components implement their own set of data-test-id attributes which are used in the functional tests in isolation for each library. I.e. ui-widgets has a suite of functional tests to verify that the ui-widgets components function as they should.

• Apps implement their own set of data-test-id attributes which the functional tests for the App use to test and verify that the App is functionally correct.

• The Core would not implement "functional tests", rather focus on integration tests, API tests, service tests, etc.

To dissuade from the data-test-id attributes being used external to the scope they are designed for, all data-test-id attributes should be stripped from the production builds of libraries and applications.

This is to ensure that we don't get bug reports that a data-test-id has changed from one version to another, or one build to another, etc. A developer working inside of an application, or library scope, needs to be able to change them and know that nothing external to the scope s/he is operating in relies on those names, so they can be refactored and changed over time.

To verify that a DHIS2.war (The System of Systems bundle) is "correct", it would need to pass its own test suites, and it would only include application builds that have passed their own suite of tests, and those in turn out only include libraries that have passed their own test suites, etc, all the way down.

I do not believe in having a suite of external tests, which can be run against an entire platform, to test all apps, and all components, compatible across versions of apps and DHIS2 versions. That kind of test suite should be limited smoke testing.

[varl]

I just want to clarify what the use cases are:

1. External testers: teams such as the PEPFAR (or any other) who need to write and maintain comprehensive automated tests (without modifying the codebase).

That's great! They should implement those tests in the application repository, and push the tests back to the repo so they can be maintained properly and run before we cut a release of that app. They would need to shift from reactive verification to proactive verification (a good thing). A bonus is that we would get actual use cases recorded as automated tests, which we can then ensure that we do not break.

2. Test Recording: Converting manual tests to automated tests (where applicable) to build up our automated regression suite, would be extremely useful for the increasing complexity of the release cycles.

I think that converting our Zephyr test cases is a manual process, and the tests should be coded by hand, and pushed back to the application repository where the source code for the app lives.

We should avoid any negative impact on developer testing with things like cypress too, and maybe facilitate it, but I don't see that as a key use case as the devs have control to add IDs as needed there anyway.

Agreed, the primary hurdle for improving automated tests right now is that the Cypress framework is still a manual setup for each repository. We should take another look at what remains to be done to be able to roll it out as we have e.g. d2-style.

[varl]

This is a visualisation of the way I see it working in isolation and inline with how continuous application delivery will work.

[Mohammer5]

Continuing this discussion after I've tried using data-test-id for the cypress tests:

To dissuade from the data-test-id attributes being used external to the scope they are designed for, all data-test-id attributes should be stripped from the production builds of libraries and applications.

I agree. This would be nice, especiall with

They should implement those tests in the application repository, and push the tests back to the repo so they can be maintained properly and run before we cut a release of that app.

---

Apps implement their own set of data-test-id attributes which the functional tests for the App use to test and verify that the App is functionally correct.

I don't think it'd be good if apps would supply data-test-ids to ui-core/ui-widget components. What I've done in the import-export-app is adding these ids to components the import export app controls and then using a selector like this: [data-test-id="foo"] .from-ui-core.bar]

This works quite well and also lets us get a group of components (if there are multiple .bars) without having to supply & specify a data-test-id for all of them. If we use class names (the ui-core components can already receive a class name), we wouldn't have to alter the standards and can refrain from using data-test-class (which is what I've proposed above)

---

I think that converting our Zephyr test cases is a manual process, and the tests should be coded by hand, and pushed back to the application repository where the source code for the app lives.

I think this would be the best case. Collecting them all in one repo will cause a lot of overhead (how do we test against local development urls)

But this falls out of scope for this issue. It has come up several times on slack already, so I'll create a new issue for this

[varl]

Proposal

• Use data-test as the attribute.

• Hand-written strings for the data-test attribute (no automatic generation).

• Components in libraries should accept a dataTest prop so that the data-test string can be overridden.

• Ship the data-test attribute with the production build, as there is a strong wish from our users to have stable ids that they can use to build their own test suite.

[ismay]

By the way, in implementing the dataTest attributes for ui-core, the need for a dataTest prop on components came from us wanting to label child components. Say you have:

<Select>
  <Button />
  <Container />  // this is the input
  <Container />  // this is the menu
<Select/>

Without dataTest props, both Container components would just have data-test="dhis2-uicore-container" attribute. With a dataTest prop, it's possible to label them more precisely, as data-test="dhis2-uicore-select-input" and data-test="dhis2-uicore-select-menu", which is nice for testing.

So even though the dataTest props are exposed to the end user, I still think that this doesn't mean that they should normally be set by anyone other than the library maintainers. Keeping them default makes them more predictable across all our apps I'd say. Maybe we should also communicate that clearly to the end user, otherwise they might think that they need to set it themselves.

[ismay]

https://github.com/dhis2/ui-core/pull/641

[morellodev]

Hi all, may I suggest you to try my tiny library https://github.com/dennismorello/react-test-attributes to add data-test-id attributes to arbitrary components? It facilitates the E2E testing process and automatically manages the dev/prod builds.

[varl]

Hi @dennismorello. Thanks for the tip, We'll check it out! 👍

[mrboli]

@dennismorello what's the advantage of using that library over explicitly setting the test id's yourself? Does it automatically do it for all children in the dom?

[sundarrajendran]

Hi all, may I suggest you to try my tiny library https://github.com/dennismorello/react-test-attributes to add data-test-id attributes to arbitrary components? It facilitates the E2E testing process and automatically manages the dev/prod builds.

Hi, can you think of a script and once i run it, unique data-test-id will be automatically appended to each and every element in the jsx. I don't want to manually wrap all the components with component