[flutter_plugin_tool] Add support for running Windows unit tests (#4276)

Implements support for `--windows` in `native-test`, for unit tests only. The structure of the new code has most of the new functionality in a generic utility for running GoogleTest test binaries, so that it can be trivially extended to Linux support in a follow-up once the Linux test PoC has landed.

This runs the recently-added `url_launcher_windows` unit test. However, it's not yet run in CI since it needs LUCI bringup; that will be done one this support is in place.

Requires new logic to check if a plugin contains native code, and some new test utility plumbing to generate plugins whose pubspecs indicate that they only contain Dart code to test it, to allow filtering filtering out the FFI-based Windows plugins.

Part of flutter/flutter#82445

Author: stuartmorgan-g

script/tool/CHANGELOG.md

@@ -5,6 +5,7 @@
 - Pubspec validation now checks for `implements` in implementation packages.
 - Pubspec valitation now checks the full relative path of `repository` entries.
 - `build-examples` now supports UWP plugins via a `--winuwp` flag.
+- `native-test` now supports `--windows` for unit tests.
 - **Breaking change**: `publish` no longer accepts `--no-tag-release` or
   `--no-push-flags`. Releases now always tag and push.
 - **Breaking change**: `publish`'s `--package` flag has been replaced with the

script/tool/lib/src/common/file_utils.dart

@@ -0,0 +1,20 @@
+// Copyright 2013 The Flutter Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+import 'package:file/file.dart';
+
+/// Returns a [File] created by appending all but the last item in [components]
+/// to [base] as subdirectories, then appending the last as a file.
+///
+/// Example:
+///   childFileWithSubcomponents(rootDir, ['foo', 'bar', 'baz.txt'])
+/// creates a File representing /rootDir/foo/bar/baz.txt.
+File childFileWithSubcomponents(Directory base, List<String> components) {
+  Directory dir = base;
+  final String basename = components.removeLast();
+  for (final String directoryName in components) {
+    dir = dir.childDirectory(directoryName);
+  }
+  return dir.childFile(basename);
+}

script/tool/lib/src/common/plugin_utils.dart

@@ -17,7 +17,7 @@ enum PlatformSupport {
   federated,
 }
 
-/// Returns whether the given [package] is a Flutter [platform] plugin.
+/// Returns true if [package] is a Flutter [platform] plugin.
 ///
 /// It checks this by looking for the following pattern in the pubspec:
 ///
@@ -30,7 +30,7 @@ enum PlatformSupport {
 /// implementation in order to return true.
 bool pluginSupportsPlatform(
   String platform,
-  RepositoryPackage package, {
+  RepositoryPackage plugin, {
   PlatformSupport? requiredMode,
   String? variant,
 }) {
@@ -41,32 +41,12 @@ bool pluginSupportsPlatform(
       platform == kPlatformWindows ||
       platform == kPlatformLinux);
   try {
-    final YamlMap pubspecYaml =
-        loadYaml(package.pubspecFile.readAsStringSync()) as YamlMap;
-    final YamlMap? flutterSection = pubspecYaml['flutter'] as YamlMap?;
-    if (flutterSection == null) {
-      return false;
-    }
-    final YamlMap? pluginSection = flutterSection['plugin'] as YamlMap?;
-    if (pluginSection == null) {
-      return false;
-    }
-    final YamlMap? platforms = pluginSection['platforms'] as YamlMap?;
-    if (platforms == null) {
-      // Legacy plugin specs are assumed to support iOS and Android. They are
-      // never federated.
-      if (requiredMode == PlatformSupport.federated) {
-        return false;
-      }
-      if (!pluginSection.containsKey('platforms')) {
-        return platform == kPlatformIos || platform == kPlatformAndroid;
-      }
-      return false;
-    }
-    final YamlMap? platformEntry = platforms[platform] as YamlMap?;
+    final YamlMap? platformEntry =
+        _readPlatformPubspecSectionForPlugin(platform, plugin);
     if (platformEntry == null) {
       return false;
     }
+
     // If the platform entry is present, then it supports the platform. Check
     // for required mode if specified.
     if (requiredMode != null) {
@@ -97,9 +77,67 @@ bool pluginSupportsPlatform(
     }
 
     return true;
+  } on YamlException {
+    return false;
+  }
+}
+
+/// Returns true if [plugin] includes native code for [platform], as opposed to
+/// being implemented entirely in Dart.
+bool pluginHasNativeCodeForPlatform(String platform, RepositoryPackage plugin) {
+  if (platform == kPlatformWeb) {
+    // Web plugins are always Dart-only.
+    return false;
+  }
+  try {
+    final YamlMap? platformEntry =
+        _readPlatformPubspecSectionForPlugin(platform, plugin);
+    if (platformEntry == null) {
+      return false;
+    }
+    // All other platforms currently use pluginClass for indicating the native
+    // code in the plugin.
+    final String? pluginClass = platformEntry['pluginClass'] as String?;
+    // TODO(stuartmorgan): Remove the check for 'none' once none of the plugins
+    // in the repository use that workaround. See
+    // https://github.com/flutter/flutter/issues/57497 for context.
+    return pluginClass != null && pluginClass != 'none';
   } on FileSystemException {
     return false;
   } on YamlException {
     return false;
   }
 }
+
+/// Returns the
+///   flutter:
+///     plugin:
+///       platforms:
+///         [platform]:
+/// section from [plugin]'s pubspec.yaml, or null if either it is not present,
+/// or the pubspec couldn't be read.
+YamlMap? _readPlatformPubspecSectionForPlugin(
+    String platform, RepositoryPackage plugin) {
+  try {
+    final File pubspecFile = plugin.pubspecFile;
+    final YamlMap pubspecYaml =
+        loadYaml(pubspecFile.readAsStringSync()) as YamlMap;
+    final YamlMap? flutterSection = pubspecYaml['flutter'] as YamlMap?;
+    if (flutterSection == null) {
+      return null;
+    }
+    final YamlMap? pluginSection = flutterSection['plugin'] as YamlMap?;
+    if (pluginSection == null) {
+      return null;
+    }
+    final YamlMap? platforms = pluginSection['platforms'] as YamlMap?;
+    if (platforms == null) {
+      return null;
+    }
+    return platforms[platform] as YamlMap?;
+  } on FileSystemException {
+    return null;
+  } on YamlException {
+    return null;
+  }
+}

script/tool/lib/src/native_test_command.dart

@@ -40,6 +40,7 @@ class NativeTestCommand extends PackageLoopingCommand {
     argParser.addFlag(kPlatformAndroid, help: 'Runs Android tests');
     argParser.addFlag(kPlatformIos, help: 'Runs iOS tests');
     argParser.addFlag(kPlatformMacos, help: 'Runs macOS tests');
+    argParser.addFlag(kPlatformWindows, help: 'Runs Windows tests');
 
     // By default, both unit tests and integration tests are run, but provide
     // flags to disable one or the other.
@@ -80,6 +81,7 @@ this command.
       kPlatformAndroid: _PlatformDetails('Android', _testAndroid),
       kPlatformIos: _PlatformDetails('iOS', _testIos),
       kPlatformMacos: _PlatformDetails('macOS', _testMacOS),
+      kPlatformWindows: _PlatformDetails('Windows', _testWindows),
     };
     _requestedPlatforms = _platforms.keys
         .where((String platform) => getBoolArg(platform))
@@ -96,6 +98,11 @@ this command.
       throw ToolExit(exitInvalidArguments);
     }
 
+    if (getBoolArg(kPlatformWindows) && getBoolArg(_integrationTestFlag)) {
+      logWarning('This command currently only supports unit tests for Windows. '
+          'See https://github.com/flutter/flutter/issues/70233.');
+    }
+
     // iOS-specific run-level state.
     if (_requestedPlatforms.contains('ios')) {
       String destination = getStringArg(_iosDestinationFlag);
@@ -119,16 +126,20 @@ this command.
   Future<PackageResult> runForPackage(RepositoryPackage package) async {
     final List<String> testPlatforms = <String>[];
     for (final String platform in _requestedPlatforms) {
-      if (pluginSupportsPlatform(platform, package,
+      if (!pluginSupportsPlatform(platform, package,
           requiredMode: PlatformSupport.inline)) {
-        testPlatforms.add(platform);
-      } else {
         print('No implementation for ${_platforms[platform]!.label}.');
+        continue;
       }
+      if (!pluginHasNativeCodeForPlatform(platform, package)) {
+        print('No native code for ${_platforms[platform]!.label}.');
+        continue;
+      }
+      testPlatforms.add(platform);
     }
 
     if (testPlatforms.isEmpty) {
-      return PackageResult.skip('Not implemented for target platform(s).');
+      return PackageResult.skip('Nothing to test for target platform(s).');
     }
 
     final _TestMode mode = _TestMode(
@@ -228,6 +239,8 @@ this command.
       final bool hasIntegrationTests =
           exampleHasNativeIntegrationTests(example);
 
+      // TODO(stuartmorgan): Make !hasUnitTests fatal. See
+      // https://github.com/flutter/flutter/issues/85469
       if (mode.unit && !hasUnitTests) {
         _printNoExampleTestsMessage(example, 'Android unit');
       }
@@ -335,6 +348,9 @@ this command.
     for (final RepositoryPackage example in plugin.getExamples()) {
       final String exampleName = example.displayName;
 
+      // TODO(stuartmorgan): Always check for RunnerTests, and make it fatal if
+      // no examples have it. See
+      // https://github.com/flutter/flutter/issues/85469
       if (testTarget != null) {
         final Directory project = example.directory
             .childDirectory(platform.toLowerCase())
@@ -387,6 +403,71 @@ this command.
     return _PlatformResult(overallResult);
   }
 
+  Future<_PlatformResult> _testWindows(
+      RepositoryPackage plugin, _TestMode mode) async {
+    if (mode.integrationOnly) {
+      return _PlatformResult(RunState.skipped);
+    }
+
+    bool isTestBinary(File file) {
+      return file.basename.endsWith('_test.exe') ||
+          file.basename.endsWith('_tests.exe');
+    }
+
+    return _runGoogleTestTests(plugin,
+        buildDirectoryName: 'windows', isTestBinary: isTestBinary);
+  }
+
+  /// Finds every file in the [buildDirectoryName] subdirectory of [plugin]'s
+  /// build directory for which [isTestBinary] is true, and runs all of them,
+  /// returning the overall result.
+  ///
+  /// The binaries are assumed to be Google Test test binaries, thus returning
+  /// zero for success and non-zero for failure.
+  Future<_PlatformResult> _runGoogleTestTests(
+    RepositoryPackage plugin, {
+    required String buildDirectoryName,
+    required bool Function(File) isTestBinary,
+  }) async {
+    final List<File> testBinaries = <File>[];
+    for (final RepositoryPackage example in plugin.getExamples()) {
+      final Directory buildDir = example.directory
+          .childDirectory('build')
+          .childDirectory(buildDirectoryName);
+      if (!buildDir.existsSync()) {
+        continue;
+      }
+      testBinaries.addAll(buildDir
+          .listSync(recursive: true)
+          .whereType<File>()
+          .where(isTestBinary)
+          .where((File file) {
+        // Only run the debug build of the unit tests, to avoid running the
+        // same tests multiple times.
+        final List<String> components = path.split(file.path);
+        return components.contains('debug') || components.contains('Debug');
+      }));
+    }
+
+    if (testBinaries.isEmpty) {
+      final String binaryExtension = platform.isWindows ? '.exe' : '';
+      printError(
+          'No test binaries found. At least one *_test(s)$binaryExtension '
+          'binary should be built by the example(s)');
+      return _PlatformResult(RunState.failed,
+          error: 'No $buildDirectoryName unit tests found');
+    }
+
+    bool passing = true;
+    for (final File test in testBinaries) {
+      print('Running ${test.basename}...');
+      final int exitCode =
+          await processRunner.runAndStream(test.path, <String>[]);
+      passing &= exitCode == 0;
+    }
+    return _PlatformResult(passing ? RunState.succeeded : RunState.failed);
+  }
+
   /// Prints a standard format message indicating that [platform] tests for
   /// [plugin]'s [example] are about to be run.
   void _printRunningExampleTestsMessage(

script/tool/lib/src/publish_plugin_command.dart

@@ -18,6 +18,7 @@ import 'package:pubspec_parse/pubspec_parse.dart';
 import 'package:yaml/yaml.dart';
 
 import 'common/core.dart';
+import 'common/file_utils.dart';
 import 'common/git_version_finder.dart';
 import 'common/plugin_command.dart';
 import 'common/process_runner.dart';
@@ -154,13 +155,10 @@ class PublishPluginCommand extends PluginCommand {
           await gitVersionFinder.getChangedPubSpecs();
 
       for (final String pubspecPath in changedPubspecs) {
-        // Convert git's Posix-style paths to a path that matches the current
-        // filesystem.
-        final String localStylePubspecPath =
-            path.joinAll(p.posix.split(pubspecPath));
-        final File pubspecFile = packagesDir.fileSystem
-            .directory((await gitDir).path)
-            .childFile(localStylePubspecPath);
+        // git outputs a relativa, Posix-style path.
+        final File pubspecFile = childFileWithSubcomponents(
+            packagesDir.fileSystem.directory((await gitDir).path),
+            p.posix.split(pubspecPath));
         yield PackageEnumerationEntry(RepositoryPackage(pubspecFile.parent),
             excluded: false);
       }

script/tool/test/common/file_utils_test.dart

@@ -0,0 +1,32 @@
+// Copyright 2013 The Flutter Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+import 'package:file/file.dart';
+import 'package:file/memory.dart';
+import 'package:flutter_plugin_tools/src/common/file_utils.dart';
+import 'package:test/test.dart';
+
+void main() {
+  test('works on Posix', () async {
+    final FileSystem fileSystem =
+        MemoryFileSystem(style: FileSystemStyle.posix);
+
+    final Directory base = fileSystem.directory('/').childDirectory('base');
+    final File file =
+        childFileWithSubcomponents(base, <String>['foo', 'bar', 'baz.txt']);
+
+    expect(file.absolute.path, '/base/foo/bar/baz.txt');
+  });
+
+  test('works on Windows', () async {
+    final FileSystem fileSystem =
+        MemoryFileSystem(style: FileSystemStyle.windows);
+
+    final Directory base = fileSystem.directory(r'C:\').childDirectory('base');
+    final File file =
+        childFileWithSubcomponents(base, <String>['foo', 'bar', 'baz.txt']);
+
+    expect(file.absolute.path, r'C:\base\foo\bar\baz.txt');
+  });
+}