Should we recommend users to add keys to whole screens/pages

[bartekpacia]

I often see that testers need a way to simply assert "I am on this screen", but almost always they don't have a way to do this reliably.

For example, I've very recently seen this test in one of the projects.

patrolTest(
  'logs in with correct credentials',
  nativeAutomation: true,
  config: patrolConfig,
  ($) async {
    await $.pumpWidgetAndSettle(IntegrationTestApp());
    await fillLoginForm(
      $: $,
      email: const String.fromEnvironment('EMAIL'),
      password: const String.fromEnvironment('PASSWORD'),
    );
    await $(K.loginSubmitButton).tap();
    await $('Notifications').waitUntilVisible(); // what is this for?
  },
);

The test is OK, but I have the problem with the last line. Even though the "Notifications" text has nothing to do with this test, it was used as a way to make sure that we've reached the home screen (on which the "Notifications" text always is). After a week, this test started failing, because the "Notifications" text was no longer on the home screen because of the development.

I think that using arbitrary widgets (and might change at any time during the development) that are present on some screen to assert that some screen is visible is a bad pattern. I see 2 solutions to this:

1. Devs should always inform testers if they change something that might affect the tests. I think this is non-scalable and relies on human nature too much.
2. Every screen and page (maybe we should call them "top-level widgets"?) should be keyed, which would let testers reliably test for the top-level widget's visibility.

Below is what I'd expect the aforementioned test to look like after going the solution (2):

patrolTest(
  'logs in with correct credentials',
  nativeAutomation: true,
  config: patrolConfig,
  ($) async {
    await $.pumpWidgetAndSettle(IntegrationTestApp());
    await fillLoginForm(
      $: $,
      email: const String.fromEnvironment('EMAIL'),
      password: const String.fromEnvironment('PASSWORD'),
    );
    await $(K.loginSubmitButton).tap();
    await $(K.dashboardScreen).waitUntilVisible(); // reads better, works better
  },
);

I'd like to recommend our users to add keys to top-level widgets (screens/pages), but before I do this, I'd like to hear you: @jBorkowska @jakubfijalkowski @mateuszwojtczak @shilangyu.

[pietraniktx]

In my opinion this is not ideal either, because this solution seems prone to false passes. What it the top level widget's rendered but other widgets inside have not, and "Notifications" string is not visible? My solution would be to add the key to the Text widget and make it more change resistant if ie. "Notifications" would change to "What's new".

Also, I think that POM comes handy in situations like this. If you check if "Notifications" is displayed, why not check other widgets on the screen - what if coming from a specific place in the app to the screen in topic makes some widget not render properly?
If I had an object with keys to all the widgets on said screen I could simply make a loop on it to assert all of them are in place.

[jakubfijalkowski]

Depending on what you want to test - seeing that you've moved after the login screen might be sufficient for this particular test. Testing if you see a "notifications" widget is something different than that. Both are valid, except that the second one might be harder to debug (and tests slightly different thing).

[pietraniktx]

True, but I think integration/e2e tests should not isolate features to this extent. I would not write two separate tests to check if logging in works and the other one to check if screen has rendered after login.

[jakubfijalkowski]

I would separate the checks at least - so that we get a different failure when the screen is wrong and when the login does not work at all.

[pietraniktx]

Oh yes, this I would recommend!

[bartekpacia]

I agree.

make it more change resistant

I think it comes down to:

• educating developers to be careful when making changes to keyed widgets
• educating testers to only use keyed widgets

What else can we do?

[pietraniktx]

Ad1. Yes, devs should always keep notice if they are deleting the keys or moving them to other places, and if they don't we should have a right to beat them with the stick or tickle them mercyless.
Ad2. Not so sure. Now when I'm thinking about it - how much time would it take for a tester to change the string in the test? Sometimes adding the key to specific widget may be a pain for no flutter-fluent individual. There are some smart (imo) test automation people that would say that you only need the test-id if the test starts to fail and it becomes an issue. Speed of test cases delivery is also significant factor in automation project.

[shilangyu]

I see 2 solutions to this:

1. Devs should always inform testers if they change something that might affect the tests

I don't think this option is feasible. As a developer, sometimes I can notice that my change will break UI tests and adjust accordingly (for example moving the key to a new widget). But since I am not the one writing these tests, it would be impossible for me to keep track of all UI tests and how my changes will affect them. Unit tests are cheap to run so I can immediately get feedback whether I broke something; with UI tests we do not have this luxury.

I think it comes down to:

• educating developers to be careful when making changes to keyed widgets

That's more realistic, since it is easy to notice a K.something and move it to a new place. However, these keys can often be deeper in the tree (invisible to the changes we make) but still be affected indirectly by some other changes.

Though this is touching on a more general topic than "adding keys to whole screens": how to make tests resilient to refactors. Regarding the original question I would say keeping a key for the whole page sounds good, but as @pietraniktx has said it depends what you are actually testing.