Add support for Dart-side `group()`s and multiple `test()`s

[bartekpacia]

A subtask of #1501.

[bartekpacia]

A problem with this is that the execution model of tests will change.

Developers usually expect that their test file executes once, roughly from top to bottom. But when we implement the feature that this issue talks about, it'll become more complex.

Example

Let's consider a test file:

void main() {
  late int photoCount;

  group('test that photos are downloaded', () {
    test('download photos', () {
      photoCount = ...;
    });

    test('tap on the last but one photo', () {
      // do something that depends on photoCount
    });
  });
}

The user might reasonably expect that photoCount will be set during the second test, because it's set by the previous test. But it'll not be set, because every test case would be run in a new app process.

[shilangyu]

Maybe I am not speaking for the majority here, but in the example you gave I think it would be totally reasonable for it to fail. Relying on execution order of individual tests sounds like an anti pattern to me. Test tooling seems to even support options that would break dependency on order (--test-randomize-ordering-seed, and maybe --concurrency/--total-shards?)

[bartekpacia]

Relying on execution order of individual tests sounds like an anti pattern to me.

It is, but I've seen our QAs do it quite a few times. I'm afraid that those additional restrictions will be tricky for them.

[bartekpacia]

We could also run a whole test file in a new app process, instead of running each test case in a new app process. This would maybe feel more natural?

I think it'd also make the distinction between the two types of "grouping" more clear:

• grouping by filesystem hierarchy - each file runs in a new process
• grouping by package:test hierarchy in individual file - all groups and tests run in the same process

[bartekpacia]

Idea: lint to not create global state ("non-local-to-test-case state") in Patrol tests.

[bartekpacia]

Also, a big consideration is patrol develop. It must continue to work.

I propose the following:

• the default behavior stays the same. patrol develop -t file executes whole file.
• we add --group and --test options to patrol develop, to allow for more granular selection of test(s) to execute. Example: patrol develop -t file --group 'smoke tests' --test '(first)'

[bartekpacia]

Test names should be the same on iOS and Android.

Currently, native test name is: file path + groups + test case name.

The problem is with encoding this in Swift/Objective-C. We can't have names like on Android:

Related issue: #1486

[bartekpacia]

Test names proposal

Restrictions:

• Must be the same across platforms (see #1486)
• Must be a valid Objective-C method name (only alphanumeric and _)

An example

It's a good practice to make group()s and test()s read like a sentence:

// integration_test/examples/some_example_test.dart

group('HomeScreen', () {
  group('when not signed in', () {
    test('shows sign-in dialog'), () {
      // ...
     });

    test('does something else'), () {
      // ...
     });

  });

  group('when signed in'), () {
    test('shows newsletter dialog'), () {
      // ..
    });

    test('does a nice thing'), () {
      // ..
    });
});

It'd result in 4 test cases:

examples__some_example__HomeScreenWhenNotSignedInShowsSignInDialog
examples__some_example__HomeScreenWhenNotSignedInDoesSomethingElse
examples__some_example__HomeScreenWhenSignedInShowsNewsletterDialog
examples__some_example__HomeScreenWhenSignedInDoesANiceThing

[fylyppo]

Because of the restrictions this proposal it's ok but I don't like these restrictions at all. It makes it ugly and for my test cases it'll be unreadable. My example test case name: "Verify navigation from BottomNavigationBarContainer when accessory is not connected". So in my case maybe better solution is to separate words with underscore

[bartekpacia]

We could do:

examples__some_example__HomeScreen_WhenNotSignedIn_ShowsSignInDialog

Explainer: we replace filesystem / with a double underscore __, and concatenate group()s and test()s with a single underscore _. We also trim the trailing _test.dart from the filename, because it's redundant (so some_example_test.dart -> some_example).

[fylyppo]

okay, it's better idea for me

[bartekpacia]

This will require us to prohibit using _ in group()s and test()s.

[ashwinichannaveerappa]

@bartekpacia is group() and test() feature is already available? I tried using it, but its not working the app gets stuck at login screen

group("Verify that user..", () { patrolTest("Test 1", ($) async { await loadLocalization();await baseClass.initializeApp();await baseClass.loginWithValidCredentials($); patrolTest(" Test 2",($) async {});});

[mkhtradm01]

@bartekpacia is group() and test() feature is already available? I tried using it, but its not working the app gets stuck at login screen

group("Verify that user..", () { patrolTest("Test 1", ($) async { await loadLocalization();await baseClass.initializeApp();await baseClass.loginWithValidCredentials($); patrolTest(" Test 2",($) async {});});

I am also experiencing this issue when using a group(). We're using bdd_widget_test to generate our test files from cucumber format files (feature_name.feature) using Gherkin syntax, but the moment we run single or multiple test file(s) patrol runs smoothly and finishes the test but then it never completes (it get hanged).

@bartekpacia could #1634 resolve this when released?

[bartekpacia]

Currently you cannot use group()s in tests, but #1634, once merged, will resolve it.

[bartekpacia]

Let's consider this done. Minor follow-ups were split into #1713 and #1714.

[roscadalexandru]

I am using the latest versions of patrol and patrol_cli. I have this file with 2 tests inside a group. I am calling setUpAll to instantiate the bloc I need only once for all tests. I am doing this because what happens in the first test updates the state of the bloc so the second test can also take advantage of the new state. But it seems that the bloc is being reset and it doesn't keep its state. Am I using groups with patrol wrong? I am a bit confused.

Update:

import 'package:flutter_test/flutter_test.dart';
import 'package:patrol/patrol.dart';

void main() {
  late int number;

  setUpAll(() => number = 0);

  group('GroupTest', () {
    patrolTest('FirstPatrol', nativeAutomation: true, ($) async {
      number++;
      expect(number, 1);
    });
    patrolTest('SecondPatrol', ($) async {
      number++;
      expect(number, 2);
    });
    group('InsideGroupTest', () {
      patrolTest('FirstPatrol', ($) async {
        number++;
        expect(number, 3);
      });
      patrolTest('SecondPatrol', ($) async {
        number++;
        expect(number, 4);
      });
    });
  });
}

I found out by running this code that if nativeAutomation is set to true, for the next tests setUpAll will be recalled once again. The last 3 tests will fail because each time the number will be smaller by one than the expected value.

[roscadalexandru]

@bartekpacia any thought on my last comment?

[github-actions[bot]]

This issue has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar problem, please file a new issue. Make sure to follow the template and provide all the information necessary to reproduce the issue.