From b33afff8f262fd64ddbf699e60d0b0455a7ef9c0 Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Wed, 18 Aug 2021 09:51:53 -0400 Subject: [PATCH 1/5] [flutter_plugin_tools] Add native unit test support for Windows Implements support for `--windows` in `native-test`, for unit tests only. This runs the recently-added `url_launcher_windows` unit test. However, it's not yet run in CI since it needs LUCI bringup (new recipe, new builder, etc.), which will be done one this support is in place. Part of https://github.com/flutter/flutter/issues/82445 --- script/tool/CHANGELOG.md | 1 + script/tool/lib/src/common/plugin_utils.dart | 86 ++++++--- script/tool/lib/src/native_test_command.dart | 63 ++++++- .../tool/test/common/plugin_utils_test.dart | 68 +++++++ .../tool/test/native_test_command_test.dart | 171 +++++++++++++++++- script/tool/test/util.dart | 63 +++---- 6 files changed, 386 insertions(+), 66 deletions(-) diff --git a/script/tool/CHANGELOG.md b/script/tool/CHANGELOG.md index a32fb0016cb3..407c99915aad 100644 --- a/script/tool/CHANGELOG.md +++ b/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. ## 0.5.0 diff --git a/script/tool/lib/src/common/plugin_utils.dart b/script/tool/lib/src/common/plugin_utils.dart index 49da67655e91..81fd71eb815b 100644 --- a/script/tool/lib/src/common/plugin_utils.dart +++ b/script/tool/lib/src/common/plugin_utils.dart @@ -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 whether the given [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; + } +} diff --git a/script/tool/lib/src/native_test_command.dart b/script/tool/lib/src/native_test_command.dart index 725cf23a2e9a..2eab19a93859 100644 --- a/script/tool/lib/src/native_test_command.dart +++ b/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 runForPackage(RepositoryPackage package) async { final List testPlatforms = []; 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,45 @@ this command. return _PlatformResult(overallResult); } + Future<_PlatformResult> _testWindows( + RepositoryPackage plugin, _TestMode mode) async { + if (!pluginHasNativeCodeForPlatform(kPlatformWindows, plugin)) { + print('No native code.'); + return _PlatformResult(RunState.skipped); + } + + final List testBinaries = []; + for (final RepositoryPackage example in plugin.getExamples()) { + final Directory buildDir = + example.directory.childDirectory('build').childDirectory('windows'); + if (!buildDir.existsSync()) { + continue; + } + testBinaries.addAll(buildDir + .listSync(recursive: true) + .whereType() + .where((File file) => + file.basename.endsWith('_test.exe') || + file.basename.endsWith('_tests.exe'))); + } + + if (testBinaries.isEmpty) { + printError('No test binaries found. At least one *_test(s).exe should be ' + 'built by the example(s)'); + return _PlatformResult(RunState.failed, + error: 'No Windows unit tests found'); + } + + bool passing = true; + for (final File test in testBinaries) { + print('Running ${test.basename}...'); + final int exitCode = + await processRunner.runAndStream(test.path, []); + 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( diff --git a/script/tool/test/common/plugin_utils_test.dart b/script/tool/test/common/plugin_utils_test.dart index 2e08f725eb4b..ac619e2622e0 100644 --- a/script/tool/test/common/plugin_utils_test.dart +++ b/script/tool/test/common/plugin_utils_test.dart @@ -273,4 +273,72 @@ void main() { isTrue); }); }); + + group('pluginHasNativeCodeForPlatform', () { + test('returns false for web', () async { + final RepositoryPackage plugin = RepositoryPackage(createFakePlugin( + 'plugin', + packagesDir, + platformSupport: { + kPlatformWeb: const PlatformDetails(PlatformSupport.inline), + }, + )); + + expect(pluginHasNativeCodeForPlatform(kPlatformWeb, plugin), isFalse); + }); + + test('returns false for a native-only plugin', () async { + final RepositoryPackage plugin = RepositoryPackage(createFakePlugin( + 'plugin', + packagesDir, + platformSupport: { + kPlatformLinux: const PlatformDetails(PlatformSupport.inline), + kPlatformMacos: const PlatformDetails(PlatformSupport.inline), + kPlatformWindows: const PlatformDetails(PlatformSupport.inline), + }, + )); + + expect(pluginHasNativeCodeForPlatform(kPlatformLinux, plugin), isTrue); + expect(pluginHasNativeCodeForPlatform(kPlatformMacos, plugin), isTrue); + expect(pluginHasNativeCodeForPlatform(kPlatformWindows, plugin), isTrue); + }); + + test('returns true for a native+Dart plugin', () async { + final RepositoryPackage plugin = RepositoryPackage(createFakePlugin( + 'plugin', + packagesDir, + platformSupport: { + kPlatformLinux: const PlatformDetails(PlatformSupport.inline, + hasNativeCode: true, hasDartCode: true), + kPlatformMacos: const PlatformDetails(PlatformSupport.inline, + hasNativeCode: true, hasDartCode: true), + kPlatformWindows: const PlatformDetails(PlatformSupport.inline, + hasNativeCode: true, hasDartCode: true), + }, + )); + + expect(pluginHasNativeCodeForPlatform(kPlatformLinux, plugin), isTrue); + expect(pluginHasNativeCodeForPlatform(kPlatformMacos, plugin), isTrue); + expect(pluginHasNativeCodeForPlatform(kPlatformWindows, plugin), isTrue); + }); + + test('returns false for a Dart-only plugin', () async { + final RepositoryPackage plugin = RepositoryPackage(createFakePlugin( + 'plugin', + packagesDir, + platformSupport: { + kPlatformLinux: const PlatformDetails(PlatformSupport.inline, + hasNativeCode: false, hasDartCode: true), + kPlatformMacos: const PlatformDetails(PlatformSupport.inline, + hasNativeCode: false, hasDartCode: true), + kPlatformWindows: const PlatformDetails(PlatformSupport.inline, + hasNativeCode: false, hasDartCode: true), + }, + )); + + expect(pluginHasNativeCodeForPlatform(kPlatformLinux, plugin), isFalse); + expect(pluginHasNativeCodeForPlatform(kPlatformMacos, plugin), isFalse); + expect(pluginHasNativeCodeForPlatform(kPlatformWindows, plugin), isFalse); + }); + }); } diff --git a/script/tool/test/native_test_command_test.dart b/script/tool/test/native_test_command_test.dart index 7b2a3d3ba39c..2ed0be2e57d7 100644 --- a/script/tool/test/native_test_command_test.dart +++ b/script/tool/test/native_test_command_test.dart @@ -57,7 +57,7 @@ final Map _kDeviceListMap = { void main() { const String _kDestination = '--ios-destination'; - group('test native_test_command', () { + group('test native_test_command on Posix', () { late FileSystem fileSystem; late MockPlatform mockPlatform; late Directory packagesDir; @@ -164,7 +164,7 @@ void main() { output, containsAllInOrder([ contains('No implementation for iOS.'), - contains('SKIPPING: Not implemented for target platform(s).'), + contains('SKIPPING: Nothing to test for target platform(s).'), ])); expect(processRunner.recordedCalls, orderedEquals([])); }); @@ -181,7 +181,7 @@ void main() { output, containsAllInOrder([ contains('No implementation for iOS.'), - contains('SKIPPING: Not implemented for target platform(s).'), + contains('SKIPPING: Nothing to test for target platform(s).'), ])); expect(processRunner.recordedCalls, orderedEquals([])); }); @@ -291,7 +291,7 @@ void main() { output, containsAllInOrder([ contains('No implementation for macOS.'), - contains('SKIPPING: Not implemented for target platform(s).'), + contains('SKIPPING: Nothing to test for target platform(s).'), ])); expect(processRunner.recordedCalls, orderedEquals([])); }); @@ -309,7 +309,7 @@ void main() { output, containsAllInOrder([ contains('No implementation for macOS.'), - contains('SKIPPING: Not implemented for target platform(s).'), + contains('SKIPPING: Nothing to test for target platform(s).'), ])); expect(processRunner.recordedCalls, orderedEquals([])); }); @@ -707,7 +707,7 @@ void main() { output, containsAllInOrder([ contains('No implementation for Android.'), - contains('SKIPPING: Not implemented for target platform(s).'), + contains('SKIPPING: Nothing to test for target platform(s).'), ]), ); }); @@ -1173,6 +1173,7 @@ void main() { '--android', '--ios', '--macos', + '--windows', _kDestination, 'foo_destination', ]); @@ -1183,7 +1184,38 @@ void main() { contains('No implementation for Android.'), contains('No implementation for iOS.'), contains('No implementation for macOS.'), - contains('SKIPPING: Not implemented for target platform(s).'), + contains('SKIPPING: Nothing to test for target platform(s).'), + ])); + + expect(processRunner.recordedCalls, orderedEquals([])); + }); + + test('skips Dart-only plugins', () async { + createFakePlugin( + 'plugin', + packagesDir, + platformSupport: { + kPlatformMacos: const PlatformDetails(PlatformSupport.inline, + hasDartCode: true, hasNativeCode: false), + kPlatformWindows: const PlatformDetails(PlatformSupport.inline, + hasDartCode: true, hasNativeCode: false), + }, + ); + + final List output = await runCapturingPrint(runner, [ + 'native-test', + '--macos', + '--windows', + _kDestination, + 'foo_destination', + ]); + + expect( + output, + containsAllInOrder([ + contains('No native code for macOS.'), + contains('No native code for Windows.'), + contains('SKIPPING: Nothing to test for target platform(s).'), ])); expect(processRunner.recordedCalls, orderedEquals([])); @@ -1295,4 +1327,129 @@ void main() { }); }); }); + + group('test native_test_command on Windows', () { + late FileSystem fileSystem; + late MockPlatform mockPlatform; + late Directory packagesDir; + late CommandRunner runner; + late RecordingProcessRunner processRunner; + + setUp(() { + fileSystem = MemoryFileSystem(style: FileSystemStyle.windows); + mockPlatform = MockPlatform(isWindows: true); + packagesDir = createPackagesDirectory(fileSystem: fileSystem); + processRunner = RecordingProcessRunner(); + final NativeTestCommand command = NativeTestCommand(packagesDir, + processRunner: processRunner, platform: mockPlatform); + + runner = CommandRunner( + 'native_test_command', 'Test for native_test_command'); + runner.addCommand(command); + }); + + group('Windows', () { + test('runs unit tests', () async { + const String testBinaryRelativePath = + 'build/windows/foo/bar/plugin_test.exe'; + final Directory pluginDirectory = + createFakePlugin('plugin', packagesDir, extraFiles: [ + 'example/$testBinaryRelativePath' + ], platformSupport: { + kPlatformWindows: const PlatformDetails(PlatformSupport.inline), + }); + + final File testBinary = pluginDirectory + .childDirectory('example') + .childFile(testBinaryRelativePath.split('/').join(r'\')); + + final List output = await runCapturingPrint(runner, [ + 'native-test', + '--windows', + '--no-integration', + ]); + + expect( + output, + containsAllInOrder([ + contains('Running plugin_test.exe...'), + contains('No issues found!'), + ]), + ); + + expect( + processRunner.recordedCalls, + orderedEquals([ + ProcessCall(testBinary.path, const [], null), + ])); + }); + + test('fails if there are no unit tests', () async { + createFakePlugin('plugin', packagesDir, + platformSupport: { + kPlatformWindows: const PlatformDetails(PlatformSupport.inline), + }); + + Error? commandError; + final List output = await runCapturingPrint(runner, [ + 'native-test', + '--windows', + '--no-integration', + ], errorHandler: (Error e) { + commandError = e; + }); + + expect(commandError, isA()); + expect( + output, + containsAllInOrder([ + contains('No test binaries found.'), + ]), + ); + + expect(processRunner.recordedCalls, orderedEquals([])); + }); + + test('fails if a unit test fails', () async { + const String testBinaryRelativePath = + 'build/windows/foo/bar/plugin_test.exe'; + final Directory pluginDirectory = + createFakePlugin('plugin', packagesDir, extraFiles: [ + 'example/$testBinaryRelativePath' + ], platformSupport: { + kPlatformWindows: const PlatformDetails(PlatformSupport.inline), + }); + + final File testBinary = pluginDirectory + .childDirectory('example') + .childFile(testBinaryRelativePath.split('/').join(r'\')); + + processRunner.mockProcessesForExecutable[testBinary.path] = + [MockProcess(exitCode: 1)]; + + Error? commandError; + final List output = await runCapturingPrint(runner, [ + 'native-test', + '--windows', + '--no-integration', + ], errorHandler: (Error e) { + commandError = e; + }); + + expect(commandError, isA()); + expect( + output, + containsAllInOrder([ + contains('Running plugin_test.exe...'), + ]), + ); + + expect( + processRunner.recordedCalls, + orderedEquals([ + ProcessCall(testBinary.path, const [], null), + ])); + }); + }); + }); } diff --git a/script/tool/test/util.dart b/script/tool/test/util.dart index 9b92a5d94ac8..47e946e74a86 100644 --- a/script/tool/test/util.dart +++ b/script/tool/test/util.dart @@ -47,6 +47,8 @@ class PlatformDetails { const PlatformDetails( this.type, { this.variants = const [], + this.hasNativeCode = true, + this.hasDartCode = false, }); /// The type of support for the platform. @@ -54,6 +56,16 @@ class PlatformDetails { /// Any 'supportVariants' to list in the pubspec. final List variants; + + /// Whether or not the plugin includes native code. + /// + /// Ignored for web, which does not have native code. + final bool hasNativeCode; + + /// Whether or not the plugin includes Dart code. + /// + /// Ignored for web, which always has native code. + final bool hasDartCode; } /// Creates a plugin package with the given [name] in [packagesDirectory]. @@ -209,49 +221,38 @@ String _pluginPlatformSection( default_package: ${packageName}_$platform '''; } else { + final List lines = [ + ' $platform:', + ]; switch (platform) { case kPlatformAndroid: - entry = ''' - android: - package: io.flutter.plugins.fake - pluginClass: FakePlugin -'''; - break; + lines.add(' package: io.flutter.plugins.fake'); + continue nativeByDefault; + nativeByDefault: case kPlatformIos: - entry = ''' - ios: - pluginClass: FLTFakePlugin -'''; - break; case kPlatformLinux: - entry = ''' - linux: - pluginClass: FakePlugin -'''; - break; case kPlatformMacos: - entry = ''' - macos: - pluginClass: FakePlugin -'''; + case kPlatformWindows: + if (support.hasNativeCode) { + final String className = + platform == kPlatformIos ? 'FLTFakePlugin' : 'FakePlugin'; + lines.add(' pluginClass: $className'); + } + if (support.hasDartCode) { + lines.add(' dartPluginClass: FakeDartPlugin'); + } break; case kPlatformWeb: - entry = ''' - web: - pluginClass: FakePlugin - fileName: ${packageName}_web.dart -'''; - break; - case kPlatformWindows: - entry = ''' - windows: - pluginClass: FakePlugin -'''; + lines.addAll([ + ' pluginClass: FakePlugin', + ' fileName: ${packageName}_web.dart', + ]); break; default: assert(false, 'Unrecognized platform: $platform'); break; } + entry = lines.join('\n') + '\n'; } // Add any variants. From c881e572d37612181adf63a2943a3f41c3704817 Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Fri, 20 Aug 2021 14:43:30 -0400 Subject: [PATCH 2/5] Only run debug, and refactor for later sharing with Linux --- script/tool/lib/src/native_test_command.dart | 46 +++++++++++++++---- .../tool/test/native_test_command_test.dart | 43 ++++++++++++++++- 2 files changed, 77 insertions(+), 12 deletions(-) diff --git a/script/tool/lib/src/native_test_command.dart b/script/tool/lib/src/native_test_command.dart index 2eab19a93859..5120ad10b872 100644 --- a/script/tool/lib/src/native_test_command.dart +++ b/script/tool/lib/src/native_test_command.dart @@ -405,31 +405,57 @@ this command. Future<_PlatformResult> _testWindows( RepositoryPackage plugin, _TestMode mode) async { - if (!pluginHasNativeCodeForPlatform(kPlatformWindows, plugin)) { - print('No native code.'); + 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 testBinaries = []; for (final RepositoryPackage example in plugin.getExamples()) { - final Directory buildDir = - example.directory.childDirectory('build').childDirectory('windows'); + final Directory buildDir = example.directory + .childDirectory('build') + .childDirectory(buildDirectoryName); if (!buildDir.existsSync()) { continue; } testBinaries.addAll(buildDir .listSync(recursive: true) .whereType() - .where((File file) => - file.basename.endsWith('_test.exe') || - file.basename.endsWith('_tests.exe'))); + .where(isTestBinary) + .where((File file) { + // Only run the debug build of the unit tests, to avoid running the + // same tests multiple times. + final List components = path.split(file.path); + return components.contains('debug') || components.contains('Debug'); + })); } if (testBinaries.isEmpty) { - printError('No test binaries found. At least one *_test(s).exe should be ' - 'built by the example(s)'); + 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 Windows unit tests found'); + error: 'No $buildDirectoryName unit tests found'); } bool passing = true; diff --git a/script/tool/test/native_test_command_test.dart b/script/tool/test/native_test_command_test.dart index 2ed0be2e57d7..4b33a0bd9f74 100644 --- a/script/tool/test/native_test_command_test.dart +++ b/script/tool/test/native_test_command_test.dart @@ -1351,7 +1351,7 @@ void main() { group('Windows', () { test('runs unit tests', () async { const String testBinaryRelativePath = - 'build/windows/foo/bar/plugin_test.exe'; + 'build/windows/foo/Debug/bar/plugin_test.exe'; final Directory pluginDirectory = createFakePlugin('plugin', packagesDir, extraFiles: [ 'example/$testBinaryRelativePath' @@ -1384,6 +1384,45 @@ void main() { ])); }); + test('only runs debug unit tests', () async { + const String debugTestBinaryRelativePath = + 'build/windows/foo/Debug/bar/plugin_test.exe'; + const String releaseTestBinaryRelativePath = + 'build/windows/foo/Release/bar/plugin_test.exe'; + final Directory pluginDirectory = + createFakePlugin('plugin', packagesDir, extraFiles: [ + 'example/$debugTestBinaryRelativePath', + 'example/$releaseTestBinaryRelativePath' + ], platformSupport: { + kPlatformWindows: const PlatformDetails(PlatformSupport.inline), + }); + + final File debugTestBinary = pluginDirectory + .childDirectory('example') + .childFile(debugTestBinaryRelativePath.split('/').join(r'\')); + + final List output = await runCapturingPrint(runner, [ + 'native-test', + '--windows', + '--no-integration', + ]); + + expect( + output, + containsAllInOrder([ + contains('Running plugin_test.exe...'), + contains('No issues found!'), + ]), + ); + + // Only the debug version should be run. + expect( + processRunner.recordedCalls, + orderedEquals([ + ProcessCall(debugTestBinary.path, const [], null), + ])); + }); + test('fails if there are no unit tests', () async { createFakePlugin('plugin', packagesDir, platformSupport: { @@ -1412,7 +1451,7 @@ void main() { test('fails if a unit test fails', () async { const String testBinaryRelativePath = - 'build/windows/foo/bar/plugin_test.exe'; + 'build/windows/foo/Debug/bar/plugin_test.exe'; final Directory pluginDirectory = createFakePlugin('plugin', packagesDir, extraFiles: [ 'example/$testBinaryRelativePath' From 95b38edc3d1d20de901f0cb629623b79d139821a Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Fri, 27 Aug 2021 16:32:10 -0400 Subject: [PATCH 3/5] Review feedback --- script/tool/lib/src/common/plugin_utils.dart | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/script/tool/lib/src/common/plugin_utils.dart b/script/tool/lib/src/common/plugin_utils.dart index 81fd71eb815b..06af675e71ef 100644 --- a/script/tool/lib/src/common/plugin_utils.dart +++ b/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: /// @@ -82,8 +82,8 @@ bool pluginSupportsPlatform( } } -/// Returns whether the given [plugin] includes native code for [platform], as -/// opposed to being implemented entirely in Dart. +/// 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. From 94318cb2b591abfc892cd02427f4cd83c0e6b13f Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Sat, 28 Aug 2021 13:10:34 -0400 Subject: [PATCH 4/5] Add a file utility for composing path components --- script/tool/lib/src/common/file_utils.dart | 20 ++++++++++ .../tool/lib/src/publish_plugin_command.dart | 12 +++--- script/tool/test/common/file_utils_test.dart | 37 +++++++++++++++++++ .../tool/test/native_test_command_test.dart | 16 ++++---- script/tool/test/util.dart | 10 ++--- 5 files changed, 72 insertions(+), 23 deletions(-) create mode 100644 script/tool/lib/src/common/file_utils.dart create mode 100644 script/tool/test/common/file_utils_test.dart diff --git a/script/tool/lib/src/common/file_utils.dart b/script/tool/lib/src/common/file_utils.dart new file mode 100644 index 000000000000..3c2f2f18f954 --- /dev/null +++ b/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 components) { + Directory dir = base; + final String basename = components.removeLast(); + for (final String directoryName in components) { + dir = dir.childDirectory(directoryName); + } + return dir.childFile(basename); +} diff --git a/script/tool/lib/src/publish_plugin_command.dart b/script/tool/lib/src/publish_plugin_command.dart index aafe7868d8d0..e210152ecf09 100644 --- a/script/tool/lib/src/publish_plugin_command.dart +++ b/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); } diff --git a/script/tool/test/common/file_utils_test.dart b/script/tool/test/common/file_utils_test.dart new file mode 100644 index 000000000000..7f2bc0a2d1a0 --- /dev/null +++ b/script/tool/test/common/file_utils_test.dart @@ -0,0 +1,37 @@ +// 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 'dart:io' as io; + +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'; + +import '../mocks.dart'; +import '../util.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, ['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, ['foo', 'bar', 'baz.txt']); + + expect(file.absolute.path, r'C:\base\foo\bar\baz.txt'); + }); +} diff --git a/script/tool/test/native_test_command_test.dart b/script/tool/test/native_test_command_test.dart index 4b33a0bd9f74..3613a808d9b8 100644 --- a/script/tool/test/native_test_command_test.dart +++ b/script/tool/test/native_test_command_test.dart @@ -9,6 +9,7 @@ import 'package:args/command_runner.dart'; import 'package:file/file.dart'; import 'package:file/memory.dart'; import 'package:flutter_plugin_tools/src/common/core.dart'; +import 'package:flutter_plugin_tools/src/common/file_utils.dart'; import 'package:flutter_plugin_tools/src/common/plugin_utils.dart'; import 'package:flutter_plugin_tools/src/native_test_command.dart'; import 'package:test/test.dart'; @@ -1359,9 +1360,8 @@ void main() { kPlatformWindows: const PlatformDetails(PlatformSupport.inline), }); - final File testBinary = pluginDirectory - .childDirectory('example') - .childFile(testBinaryRelativePath.split('/').join(r'\')); + final File testBinary = childFileWithSubcomponents(pluginDirectory, + ['example', ...testBinaryRelativePath.split('/')]); final List output = await runCapturingPrint(runner, [ 'native-test', @@ -1397,9 +1397,8 @@ void main() { kPlatformWindows: const PlatformDetails(PlatformSupport.inline), }); - final File debugTestBinary = pluginDirectory - .childDirectory('example') - .childFile(debugTestBinaryRelativePath.split('/').join(r'\')); + final File debugTestBinary = childFileWithSubcomponents(pluginDirectory, + ['example', ...debugTestBinaryRelativePath.split('/')]); final List output = await runCapturingPrint(runner, [ 'native-test', @@ -1459,9 +1458,8 @@ void main() { kPlatformWindows: const PlatformDetails(PlatformSupport.inline), }); - final File testBinary = pluginDirectory - .childDirectory('example') - .childFile(testBinaryRelativePath.split('/').join(r'\')); + final File testBinary = childFileWithSubcomponents(pluginDirectory, + ['example', ...testBinaryRelativePath.split('/')]); processRunner.mockProcessesForExecutable[testBinary.path] = [MockProcess(exitCode: 1)]; diff --git a/script/tool/test/util.dart b/script/tool/test/util.dart index 152107f26bb3..e053100172c6 100644 --- a/script/tool/test/util.dart +++ b/script/tool/test/util.dart @@ -10,6 +10,7 @@ import 'package:args/command_runner.dart'; import 'package:file/file.dart'; import 'package:file/memory.dart'; import 'package:flutter_plugin_tools/src/common/core.dart'; +import 'package:flutter_plugin_tools/src/common/file_utils.dart'; import 'package:flutter_plugin_tools/src/common/plugin_utils.dart'; import 'package:flutter_plugin_tools/src/common/process_runner.dart'; import 'package:meta/meta.dart'; @@ -142,15 +143,10 @@ Directory createFakePackage( } } - final FileSystem fileSystem = packageDirectory.fileSystem; final p.Context posixContext = p.posix; for (final String file in extraFiles) { - final List newFilePath = [ - packageDirectory.path, - ...posixContext.split(file) - ]; - final File newFile = fileSystem.file(fileSystem.path.joinAll(newFilePath)); - newFile.createSync(recursive: true); + childFileWithSubcomponents(packageDirectory, posixContext.split(file)) + .createSync(recursive: true); } return packageDirectory; From 74d9eb2b1163223ef9e941290ae2a497cf362460 Mon Sep 17 00:00:00 2001 From: Stuart Morgan Date: Mon, 30 Aug 2021 13:25:37 -0400 Subject: [PATCH 5/5] Remove copypasta to fix anazyler --- script/tool/test/common/file_utils_test.dart | 5 ----- 1 file changed, 5 deletions(-) diff --git a/script/tool/test/common/file_utils_test.dart b/script/tool/test/common/file_utils_test.dart index 7f2bc0a2d1a0..e3986842a969 100644 --- a/script/tool/test/common/file_utils_test.dart +++ b/script/tool/test/common/file_utils_test.dart @@ -2,16 +2,11 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -import 'dart:io' as io; - 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'; -import '../mocks.dart'; -import '../util.dart'; - void main() { test('works on Posix', () async { final FileSystem fileSystem =