[pigeon] Add initial FlutterApi integration tests (#2901)

* Add trivial async test as a baseline for the scaffolding

* Kotlin generator async void fix

* Add initial round-trip tests

* Windows fixes

* Cleanup

* Version bump for bug fix

* Fix analysis

* Version bump in generator

Author: stuartmorgan-g

packages/pigeon/CHANGELOG.md

@@ -1,3 +1,7 @@
+## 4.2.9
+
+* [kotlin] Fixes a bug with some methods that return `void`.
+
 ## 4.2.8
 
 * Adds the ability to use `runWithOptions` entrypoint to allow external libraries to use the pigeon easier.

packages/pigeon/lib/generator_tools.dart

@@ -9,7 +9,7 @@ import 'dart:mirrors';
 import 'ast.dart';
 
 /// The current version of pigeon. This must match the version in pubspec.yaml.
-const String pigeonVersion = '4.2.8';
+const String pigeonVersion = '4.2.9';
 
 /// Read all the content from [stdin] to a String.
 String readStdin() {

packages/pigeon/lib/kotlin_generator.dart

@@ -233,15 +233,11 @@ void _writeHostApi(Indent indent, Api api, Root root) {
                       'api.${method.name}(${methodArgument.join(', ')})';
                   if (method.isAsynchronous) {
                     indent.write('$call ');
-                    if (method.returnType.isVoid) {
-                      indent.scoped('{', '}', () {
-                        indent.writeln('reply.reply(null)');
-                      });
-                    } else {
-                      indent.scoped('{', '}', () {
-                        indent.writeln('reply.reply(wrapResult(it))');
-                      });
-                    }
+                    final String resultValue =
+                        method.returnType.isVoid ? 'null' : 'it';
+                    indent.scoped('{', '}', () {
+                      indent.writeln('reply.reply(wrapResult($resultValue))');
+                    });
                   } else if (method.returnType.isVoid) {
                     indent.writeln(call);
                     indent.writeln('wrapped["${Keys.result}"] = null');

packages/pigeon/pigeons/core_tests.dart

@@ -33,6 +33,8 @@ class AllTypesWrapper {
 /// platform_test integration tests.
 @HostApi()
 abstract class HostIntegrationCoreApi {
+  // ========== Syncronous method tests ==========
+
   /// A no-op function taking no arguments and returning no value, to sanity
   /// test basic calling.
   void noop();
@@ -54,13 +56,31 @@ abstract class HostIntegrationCoreApi {
   @ObjCSelector('createNestedObjectWithString:')
   AllTypesWrapper createNestedString(String string);
 
-  // TODO(stuartmorgan): Add wrapper methods to trigger calls back into
-  // FlutterIntegrationCore methods, to allow Dart-driven integration testing
-  // of host->Dart calls. Each wrapper would be implemented by calling the
-  // corresponding FlutterIntegrationCore method, passing arguments and return
-  // values along unchanged. Since these will need to be async, we also need
-  // async host API tests here, so that failures in Dart->host async calling
-  // don't only show up here.
+  // ========== Asyncronous method tests ==========
+
+  /// A no-op function taking no arguments and returning no value, to sanity
+  /// test basic asynchronous calling.
+  @async
+  void noopAsync();
+
+  /// Returns the passed string asynchronously.
+  @async
+  @ObjCSelector('echoAsyncString:')
+  String echoAsyncString(String aString);
+
+  // ========== Flutter API test wrappers ==========
+
+  @async
+  void callFlutterNoop();
+
+  @async
+  @ObjCSelector('callFlutterEchoString:')
+  String callFlutterEchoString(String aString);
+
+  // TODO(stuartmorgan): Add callFlutterEchoString and the associated test once
+  // either https://github.com/flutter/flutter/issues/116117 is fixed, or the
+  // problematic type is moved out of AllTypes and into its own test, since
+  // the type mismatch breaks the second `encode` round.
 }
 
 /// The core interface that the Dart platform_test code implements for host
@@ -74,6 +94,10 @@ abstract class FlutterIntegrationCoreApi {
   /// Returns the passed object, to test serialization and deserialization.
   @ObjCSelector('echoAllTypes:')
   AllTypes echoAllTypes(AllTypes everything);
+
+  /// Returns the passed string, to test serialization and deserialization.
+  @ObjCSelector('echoString:')
+  String echoString(String aString);
 }
 
 /// An API that can be implemented for minimal, compile-only tests.

packages/pigeon/platform_tests/alternate_language_test_plugin/android/src/main/java/com/example/alternate_language_test_plugin/AlternateLanguageTestPlugin.java

@@ -8,14 +8,19 @@
 import androidx.annotation.Nullable;
 import com.example.alternate_language_test_plugin.CoreTests.AllTypes;
 import com.example.alternate_language_test_plugin.CoreTests.AllTypesWrapper;
+import com.example.alternate_language_test_plugin.CoreTests.FlutterIntegrationCoreApi;
 import com.example.alternate_language_test_plugin.CoreTests.HostIntegrationCoreApi;
+import com.example.alternate_language_test_plugin.CoreTests.Result;
 import io.flutter.embedding.engine.plugins.FlutterPlugin;
 
 /** This plugin handles the native side of the integration tests in example/integration_test/. */
 public class AlternateLanguageTestPlugin implements FlutterPlugin, HostIntegrationCoreApi {
+  @Nullable FlutterIntegrationCoreApi flutterApi = null;
+
   @Override
   public void onAttachedToEngine(@NonNull FlutterPluginBinding binding) {
     HostIntegrationCoreApi.setup(binding.getBinaryMessenger(), this);
+    flutterApi = new FlutterIntegrationCoreApi(binding.getBinaryMessenger());
   }
 
   @Override
@@ -46,4 +51,35 @@ public void throwError() {
     AllTypes innerObject = new AllTypes.Builder().setAString(string).build();
     return new AllTypesWrapper.Builder().setValues(innerObject).build();
   }
+
+  @Override
+  public void noopAsync(Result<Void> result) {
+    result.success(null);
+  }
+
+  @Override
+  public void echoAsyncString(@NonNull String aString, Result<String> result) {
+    result.success(aString);
+  }
+
+  @Override
+  public void callFlutterNoop(Result<Void> result) {
+    flutterApi.noop(
+        new FlutterIntegrationCoreApi.Reply<Void>() {
+          public void reply(Void value) {
+            result.success(value);
+          }
+        });
+  }
+
+  @Override
+  public void callFlutterEchoString(@NonNull String aString, Result<String> result) {
+    flutterApi.echoString(
+        aString,
+        new FlutterIntegrationCoreApi.Reply<String>() {
+          public void reply(String value) {
+            result.success(value);
+          }
+        });
+  }
 }

packages/pigeon/platform_tests/alternate_language_test_plugin/ios/Classes/AlternateLanguageTestPlugin.m

@@ -6,14 +6,19 @@
 
 #import "CoreTests.gen.h"
 
+@interface AlternateLanguageTestPlugin ()
+@property(nonatomic) FlutterIntegrationCoreApi *flutterAPI;
+@end
+
 /**
- * This plugin is currently a no-op since only unit tests have been set up.
- * In the future, this will register Pigeon APIs used in integration tests.
+ * This plugin handles the native side of the integration tests in example/integration_test/.
  */
 @implementation AlternateLanguageTestPlugin
 + (void)registerWithRegistrar:(NSObject<FlutterPluginRegistrar> *)registrar {
   AlternateLanguageTestPlugin *plugin = [[AlternateLanguageTestPlugin alloc] init];
-  HostIntegrationCoreApiSetup(registrar.messenger, plugin);
+  HostIntegrationCoreApiSetup([registrar messenger], plugin);
+  plugin.flutterAPI =
+      [[FlutterIntegrationCoreApi alloc] initWithBinaryMessenger:[registrar messenger]];
 }
 
 #pragma mark HostIntegrationCoreApi implementation
@@ -43,4 +48,27 @@ - (nullable AllTypesWrapper *)createNestedObjectWithString:(NSString *)string
   return [AllTypesWrapper makeWithValues:innerObject];
 }
 
+- (void)noopAsyncWithCompletion:(void (^)(FlutterError *_Nullable))completion {
+  completion(nil);
+}
+
+- (void)echoAsyncString:(NSString *)aString
+             completion:(void (^)(NSString *_Nullable, FlutterError *_Nullable))completion {
+  completion(aString, nil);
+}
+
+- (void)callFlutterNoopWithCompletion:(void (^)(FlutterError *_Nullable))completion {
+  [self.flutterAPI noopWithCompletion:^(NSError *error) {
+    completion(error);
+  }];
+}
+
+- (void)callFlutterEchoString:(NSString *)aString
+                   completion:(void (^)(NSString *_Nullable, FlutterError *_Nullable))completion {
+  [self.flutterAPI echoString:aString
+                   completion:^(NSString *value, NSError *error) {
+                     completion(value, error);
+                   }];
+}
+
 @end

packages/pigeon/platform_tests/shared_test_plugin_code/lib/integration_tests.dart

@@ -35,7 +35,7 @@ enum TargetGenerator {
 void runPigeonIntegrationTests(TargetGenerator targetGenerator) {
   IntegrationTestWidgetsFlutterBinding.ensureInitialized();
 
-  group('Host API tests', () {
+  group('Host sync API tests', () {
     testWidgets('basic void->void call works', (WidgetTester _) async {
       final HostIntegrationCoreApi api = HostIntegrationCoreApi();
 
@@ -125,8 +125,65 @@ void runPigeonIntegrationTests(TargetGenerator targetGenerator) {
     });
   });
 
-  group('Flutter API tests', () {
-    // TODO(stuartmorgan): Add Flutter API tests, driven by wrapper host APIs
-    // that forward the arguments and return values in the opposite direction.
+  group('Host async API tests', () {
+    testWidgets('basic void->void call works', (WidgetTester _) async {
+      final HostIntegrationCoreApi api = HostIntegrationCoreApi();
+
+      expect(api.noopAsync(), completes);
+    });
+
+    testWidgets('strings serialize and deserialize correctly',
+        (WidgetTester _) async {
+      final HostIntegrationCoreApi api = HostIntegrationCoreApi();
+
+      const String sentObject = 'Hello, asyncronously!';
+
+      final String echoObject = await api.echoAsyncString(sentObject);
+      expect(echoObject, sentObject);
+    });
   });
+
+  // These tests rely on the ansync Dart->host calls to work correctly, since
+  // the host->Dart call is wrapped in a driving Dart->host call, so any test
+  // added to this group should have coverage of the relevant arguments and
+  // return value in the "Host async API tests" group.
+  group('Flutter API tests', () {
+    setUp(() {
+      FlutterIntegrationCoreApi.setup(_FlutterApiTestImplementation());
+    });
+
+    testWidgets('basic void->void call works', (WidgetTester _) async {
+      final HostIntegrationCoreApi api = HostIntegrationCoreApi();
+
+      expect(api.callFlutterNoop(), completes);
+    });
+
+    testWidgets('strings serialize and deserialize correctly',
+        (WidgetTester _) async {
+      final HostIntegrationCoreApi api = HostIntegrationCoreApi();
+
+      const String sentObject = 'Hello Dart!';
+
+      final String echoObject = await api.callFlutterEchoString(sentObject);
+      expect(echoObject, sentObject);
+    });
+  },
+      // TODO(stuartmorgan): Enable when FlutterApi generation is fixed for
+      // C++. See https://github.com/flutter/flutter/issues/108682.
+      skip: targetGenerator == TargetGenerator.cpp);
+}
+
+class _FlutterApiTestImplementation implements FlutterIntegrationCoreApi {
+  @override
+  AllTypes echoAllTypes(AllTypes everything) {
+    return everything;
+  }
+
+  @override
+  String echoString(String aString) {
+    return aString;
+  }
+
+  @override
+  void noop() {}
 }