Create a @ByTestId annotation that selects element with attribute "data-test-id".

[elvisun]

Create a @ByTestId annotation that selects element with attribute data-test-id.

Rationale behind this proposal: Currently lots of test selectors are written using (layers of) css classes, tightly coupling tests to component styles and DOM structure, making them very brittle. I'm working on a draft Testing on the Toilet episode that explains this problem in further detail. go/change-resilient-ui-testing-tott

Adding a data-test-id attribute to an element to help target-select it in tests is the solution we came to at Firebase console's design review. It allows a change-resilient way of writing selectors that survive any style changes or DOM restructuring. The original design doc is here: go/change-resilient-ui-testing-dd

This pattern has been adopted by some teams within google, also there are a few external articles covering this pattern: https://kentcdodds.com/blog/making-your-ui-tests-resilient-to-change https://medium.com/@colecodes/test-your-dom-with-data-attributes-44fccc43ed4b

To make selection of elements tagged with "data-test-id" easier, we are proposing this new annotation here.

I've already added this new annotation to Java's page loader library in cl/253257572. Would love Dart to have the same love!

[elvisun]

Friendly ping.

I also updated go/webdriver-practices's documentation as well to support this change

[mk13]

Sorry, will take a look at it next week. My notifications are botched at the moment.

[mk13]

I'm a little hesitant about making this a part of the core Dart PageLoader library. We already have @ByDebugId (which I personally don't really like either, but keeping for backwards compatibility with PageLoader v2) - this is fairly similar to 'data-test-id' in principle of how it they're being used.

Main issues I have with this:

1. Despite being an 'id', there is no guarantee of being it unique. It would require developers to be certain that 'data-test-id' is unique in value. Reading on the referred blogpost and specifically referring to this point:

   // change this:
   const submitButton = rootNode.querySelector('.btn')
   // to this:
   const submitButton = rootNode.querySelectorAll('.btn')[1]

We can fall into the same pitfall if there is no safeguard to prevent the same data-test-id from having this same issue.

2. What is wrong with using the id attribute with @ById annotation?

Finally, our API does support users creating their own extensible annotations. Should a team/project want to specifically use data-test-id in their code as the primary form of selector in tests, they can create a new annotation and import it into their test cases and page objects rather than being universal to core PageLoader3. I want to minimize the number of annotations/selectors within the core unless it is also universal to HTML.

For example:

// file: test_id.dart

class ByTestId implements CssFinder { ... }

then:

// file: my_po.dart
import 'package:pageloader3/pageloader.dart';

import 'test_id.dart';

@PageObject()
abstract class MyPO {
  // ...

  @ByTestId('special-div')
  PageLoaderElement get specialDiv;

  @ByTagName('blah')
  PageLoaderElement get blah;
}

I'm perfectly fine with the idea of @ByTestId being used within a specific project/ or within a team, but I'm don't think if it belongs into the core library.

[elvisun]

Hey Max, thanks a lot for the review! To answer your questions:

1. The uniqueness of data-test-id is guarded by tests by definition, as it's only used in tests. If it's not unique tests would fail. This will make sure developers use a different ID each time. data-test-id is also typically bounded by the page object scope or unit test scope, so they don't need to be strictly unique across the entire application.

2. According to the design review we had inside Firebase console, the use of IDs could result in unintended side-effects. If two elements have the same data-test-id, only tests would fail. But if two elements have the same ID, someone on completely random part of the application could be writing getElementById or some #css and break production in unexpected ways, these breakages wouldn't necessarily be guarded by any tests. As a result, larger teams usually lean towards a hard ban for using IDs, then we are back to the problem - there's no good way to uniquely reference elements.

3. I just feel like this is a common use case that have been adopted by some teams, and more teams could benefit from it. It could also raise awareness of an alternatives to those teams. While teams that don't use this could just ignore it, it doesn't add to their production binary size or anything... So it could be here for the same reason as byDebugId.

At the end, if you don't feel like this aligns with the philosophy of the core library, I'm ok with reverting this change as well, it's up to you :)

Thanks again Max!

[elvisun]

Awesome thank you Max! There's also no need for an additional sync just for this as it's not critical.