Add waitFor method to make integration tests more stable

[nilsreichardt]

Description

Pull Request Description

Summary:

In this pull request, we have introduced a new utility function waitFor to enhance the stability of our integration tests by ensuring certain widgets are present in the widget tree before proceeding with subsequent test operations. This alleviates the issue of tests failing due to widgets not being loaded within the expected time frame.

Demo

v.mp4

Related tickets

Fixes #905
Workaround for flutter/flutter#88765

[github-actions]

Visit the preview URL for this PR (updated for commit d133021):

https://sharezone-test--pr906-make-integration-tes-sevgjz9a.web.app

_{(expires Sun, 10 Sep 2023 13:12:00 GMT)}

_{🔥 via Firebase Hosting GitHub Action 🌎}

_{Sign: 4cb3ae61e1e018abfd9841fd3239f5b49ccc034b}

[Jonas-Sander]

Wouldn't this hang indefinitely if there is an endless/repeating Animation playing?

[Jonas-Sander]

Since it's do while wouldn't it mean that with an endless animation we would never even come to the finder.evaluate() part since it would hang at finder.pumpAndSettle even if the thing we want to find is there? In this case there would be a time-out exception even if the element was there (if I remember correctly how pumpAndSettle functioned)

[nilsreichardt]

Yes, in the case we have an endless animation, we can't use tester.pumpAndSettle() for this place in the app anymore

[nilsreichardt]

Since it's do while wouldn't it mean that with an endless animation we would never even come to the finder.evaluate()

Even without a while loop and just tester.pumpAndSettle() would an endless animation never complete the tester.pumpAndSettle (method call ends after the default timeout of 10 minutes of the pumpAndSettle() method).

[Jonas-Sander]

I mean since it works for this we can merge it like this if u want, but couldn't we just replace pumpAndSettle with pump since its looped anyways?

[nilsreichardt]

Yes, this could work. Locally it worked. Let's try this.

[Jonas-Sander]

I assume you made the PR description with AI? I don't like it, it's way too much text and 2. and 4. are pretty much the same, also 3. is just unnecessary (I'll see if it's documented or not in the diff)

[Jonas-Sander]

So one can't specify sth like findsOneWidget with the new method?

[Jonas-Sander]

Also couldn't we use the waitFor for only the first group name since the others are loaded by then? Otherwise it might take until we hit the timeout maybe several times if there are a few groups where the account is missing.

In this way there would also be clear separation that we waitFor the groups being loaded and then use findsOneWidget so that it is clear that we expect one group with the name for all the groups.

[nilsreichardt]

We can first check if can find Meine Klasse::

// Ensure that the group list is loaded. When the school class is loaded,
// we assume that the courses list is loaded as well.
await waitFor(tester, find.text('Meine Klasse:'));

and then test with expect again as we did before.

[Jonas-Sander]

I like it better now.

Btw, what do you think of this?

// Ensure that the group list is loaded. When the school class is loaded,
// we assume that the courses list is loaded as well.
await tester.pumpUntil(find.text('Meine Kurse:'));

extension on WidgetTester {
  Future<void> pumpUntil(
    Finder finder, {
    Duration timeout = const Duration(seconds: 70),
  }) async {
    final end = binding.clock.now().add(timeout);

    do {
      if (binding.clock.now().isAfter(end)) {
        throw Exception('Timed out waiting for $finder');
      }

      await pump();
      await Future.delayed(const Duration(milliseconds: 200));
    } while (finder.evaluate().isEmpty);
  }
}

would be more in line with the other methods on WidgetTester.

[nilsreichardt]

Btw, what do you think of this?

Yes, looks great. I updated the code 👍