Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 13 commits into from
Jun 18, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions packages/pigeon/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
## 25.5.0

* [dart] Changes the default InstanceManager and its initialization to no longer make a message call
when used in a Flutter unit test.

## 25.4.0

* [gobject] Adds type id constants in header files so that they can be used by the user.
Expand Down
3 changes: 3 additions & 0 deletions packages/pigeon/lib/src/dart/dart_generator.dart
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,9 @@ class DartGenerator extends StructuredGenerator<InternalDartOptions> {
required String dartPackageName,
}) {
indent.writeln("import 'dart:async';");
if (root.containsProxyApi) {
indent.writeln("import 'dart:io' show Platform;");
}
indent.writeln(
"import 'dart:typed_data' show Float64List, Int32List, Int64List, Uint8List;",
);
Expand Down
3 changes: 3 additions & 0 deletions packages/pigeon/lib/src/dart/templates.dart
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,9 @@ class $dartInstanceManagerClassName {
late final void Function(int) onWeakReferenceRemoved;

static $dartInstanceManagerClassName _initInstance() {
if (Platform.environment['FLUTTER_TEST'] == 'true') {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

@bparrishMines bparrishMines Jun 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

@bparrishMines bparrishMines Jun 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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!;
  }

return $dartInstanceManagerClassName(onWeakReferenceRemoved: (_) {});
}
WidgetsFlutterBinding.ensureInitialized();
final $dartInstanceManagerApiClassName api = $dartInstanceManagerApiClassName();
// Clears the native `$dartInstanceManagerClassName` on the initial use of the Dart one.
Expand Down
2 changes: 1 addition & 1 deletion packages/pigeon/lib/src/generator_tools.dart
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import 'generator.dart';
/// The current version of pigeon.
///
/// This must match the version in pubspec.yaml.
const String pigeonVersion = '25.4.0';
const String pigeonVersion = '25.5.0';

/// Read all the content from [stdin] to a String.
String readStdin() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
// ignore_for_file: public_member_api_docs, non_constant_identifier_names, avoid_as, unused_import, unnecessary_parenthesis, prefer_null_aware_operators, omit_local_variable_types, unused_shown_name, unnecessary_import, no_leading_underscores_for_local_identifiers

import 'dart:async';
import 'dart:io' show Platform;
import 'dart:typed_data' show Float64List, Int32List, Int64List, Uint8List;

import 'package:flutter/foundation.dart'
Expand Down Expand Up @@ -126,6 +127,9 @@ class PigeonInstanceManager {
late final void Function(int) onWeakReferenceRemoved;

static PigeonInstanceManager _initInstance() {
if (Platform.environment['FLUTTER_TEST'] == 'true') {
return PigeonInstanceManager(onWeakReferenceRemoved: (_) {});
}
WidgetsFlutterBinding.ensureInitialized();
final _PigeonInternalInstanceManagerApi api =
_PigeonInternalInstanceManagerApi();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ void main() {
instanceManager.addDartCreatedInstance(object);

object = null;
await forceGC();
await forceGC(fullGcCycles: 2);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


expect(weakReferencedRemovedCalled, isTrue);
});
Expand All @@ -188,10 +188,49 @@ void main() {
instanceManager.addHostCreatedInstance(object, 0);

object = null;
await forceGC();
await forceGC(fullGcCycles: 2);

expect(weakReferencedRemovedCalled, isFalse);
});

testWidgets(
'instantiating default InstanceManager does not make a message call',
(WidgetTester tester) async {
bool messageCallMade = false;
TestDefaultBinaryMessengerBinding
.instance.defaultBinaryMessenger.allMessagesHandler = (_, __, ___) {
messageCallMade = true;
return null;
};

// Initialize default InstanceManager
// ignore: unnecessary_statements
PigeonInstanceManager.instance;

expect(messageCallMade, isFalse);
},
);

testWidgets(
'default InstanceManager does not make message call when weak reference is removed',
(WidgetTester tester) async {
bool messageCallMade = false;
TestDefaultBinaryMessengerBinding
.instance.defaultBinaryMessenger.allMessagesHandler = (_, __, ___) {
messageCallMade = true;
return null;
};

final PigeonInstanceManager instanceManager =
PigeonInstanceManager.instance;

final int identifier =
instanceManager.addDartCreatedInstance(CopyableObject());
instanceManager.onWeakReferenceRemoved(identifier);

expect(messageCallMade, isFalse);
},
);
});
}

Expand Down
2 changes: 1 addition & 1 deletion packages/pigeon/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ name: pigeon
description: Code generator tool to make communication between Flutter and the host platform type-safe and easier.
repository: https://github.com/flutter/packages/tree/main/packages/pigeon
issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+pigeon%22
version: 25.4.0 # This must match the version in lib/src/generator_tools.dart
version: 25.5.0 # This must match the version in lib/src/generator_tools.dart

environment:
sdk: ^3.6.0
Expand Down