[pigeon] Create a message call free InstanceManager when running unit tests

[bparrishMines]

To support hot restart, PigeonInstanceManager.instance makes a call to clear the native InstanceManager when it is instantiated.

This was the source of unit test race condition flakes like flutter/flutter#164132.

The current workaround is to try/catch the async PlatformException at the top of every test file or pass a test InstanceManager to every ProxyApi class:

final MyNativeClass object = MyNativeClass(
  pigeon_instanceManager: PigeonInstanceManager(
    onWeakReferenceRemoved: (_) {}
  ),
);

This PR changes it to detect a test ran as Flutter unit test and sets the default InstanceManager with one that doesn't make any message calls.

Pre-Review Checklist

• I read the Contributor Guide and followed the process outlined there for submitting PRs.
• I read the Tree Hygiene page, which explains my responsibilities.
• I read and followed the relevant style guides and ran the auto-formatter.
• I signed the CLA.
• The title of the PR starts with the name of the package surrounded by square brackets, e.g. [shared_preferences]
• I linked to at least one issue that this PR fixes in the description above.
• I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy, or I have commented below to indicate which version change exemption this PR falls under¹.
• I updated CHANGELOG.md to add a description of the change, following repository CHANGELOG style, or I have commented below to indicate which CHANGELOG exemption this PR falls under¹.
• I updated/added any relevant documentation (doc comments with ///).
• I added new tests to check the change I am making, or I have commented below to indicate which test exemption this PR falls under¹.
• All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

Footnotes

1. Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling. ↩ ↩² ↩³

[bparrishMines]

This test was failing on windows, so I added another cycle.

[stuartmorgan-g]

Hm, is FLUTTER_TEST only set in unit tests? Or will this break flutter test integration test behavior?

[bparrishMines]

I forgot that integration tests also use flutter test. But I just tried it and it looks like this doesn't affect integration tests. It seems the environment variable is only for unit tests.

[bparrishMines]

This is probably the case because integration tests still need to use the real BinaryMessenger. When FLUTTER_TEST is set, it uses a test BinaryMessenger from TestWidgetsFlutterBinding.

Edit:
Actually I found a bit of info on it: https://api.flutter.dev/flutter/flutter_test/LiveTestWidgetsFlutterBinding-class.html. It looks like integration tests from flutter test is treated the same way as running tests with flutter run.

[stuartmorgan-g]

Sounds good; LGTM then.

I wish we didn't have to embed special test logic in the production code, but it seems like the best option we have at the moment.

[bparrishMines]

Hmm would it be better to copy https://github.com/flutter/flutter/blob/master/packages/flutter/lib/src/foundation/_platform_io.dart#L30 then. And wrap the Platform.environment check in an assert so that it gets ignored in release mode.

static PigeonInstanceManager _initInstance() {
    PigeonInstanceManager? instanceManager;
    assert(() {
      if (Platform.environment.containsKey('FLUTTER_TEST')) {
        instanceManager = PigeonInstanceManager(onWeakReferenceRemoved: (_) {});
      }
      return true;
    }());
    if (instanceManager == null) {
      // ...
    }
    return instanceManager!;
  }