Skip to content

Commit 5306c02

Browse files
authored
[flutter_plugin_tool] Add support for running Windows unit tests (flutter#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#82445
1 parent 18129d6 commit 5306c02

9 files changed

+510
-81
lines changed

script/tool/CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
- Pubspec validation now checks for `implements` in implementation packages.
66
- Pubspec valitation now checks the full relative path of `repository` entries.
77
- `build-examples` now supports UWP plugins via a `--winuwp` flag.
8+
- `native-test` now supports `--windows` for unit tests.
89
- **Breaking change**: `publish` no longer accepts `--no-tag-release` or
910
`--no-push-flags`. Releases now always tag and push.
1011
- **Breaking change**: `publish`'s `--package` flag has been replaced with the
+20
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
// Copyright 2013 The Flutter Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
import 'package:file/file.dart';
6+
7+
/// Returns a [File] created by appending all but the last item in [components]
8+
/// to [base] as subdirectories, then appending the last as a file.
9+
///
10+
/// Example:
11+
/// childFileWithSubcomponents(rootDir, ['foo', 'bar', 'baz.txt'])
12+
/// creates a File representing /rootDir/foo/bar/baz.txt.
13+
File childFileWithSubcomponents(Directory base, List<String> components) {
14+
Directory dir = base;
15+
final String basename = components.removeLast();
16+
for (final String directoryName in components) {
17+
dir = dir.childDirectory(directoryName);
18+
}
19+
return dir.childFile(basename);
20+
}

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

+63-25
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ enum PlatformSupport {
1717
federated,
1818
}
1919

20-
/// Returns whether the given [package] is a Flutter [platform] plugin.
20+
/// Returns true if [package] is a Flutter [platform] plugin.
2121
///
2222
/// It checks this by looking for the following pattern in the pubspec:
2323
///
@@ -30,7 +30,7 @@ enum PlatformSupport {
3030
/// implementation in order to return true.
3131
bool pluginSupportsPlatform(
3232
String platform,
33-
RepositoryPackage package, {
33+
RepositoryPackage plugin, {
3434
PlatformSupport? requiredMode,
3535
String? variant,
3636
}) {
@@ -41,32 +41,12 @@ bool pluginSupportsPlatform(
4141
platform == kPlatformWindows ||
4242
platform == kPlatformLinux);
4343
try {
44-
final YamlMap pubspecYaml =
45-
loadYaml(package.pubspecFile.readAsStringSync()) as YamlMap;
46-
final YamlMap? flutterSection = pubspecYaml['flutter'] as YamlMap?;
47-
if (flutterSection == null) {
48-
return false;
49-
}
50-
final YamlMap? pluginSection = flutterSection['plugin'] as YamlMap?;
51-
if (pluginSection == null) {
52-
return false;
53-
}
54-
final YamlMap? platforms = pluginSection['platforms'] as YamlMap?;
55-
if (platforms == null) {
56-
// Legacy plugin specs are assumed to support iOS and Android. They are
57-
// never federated.
58-
if (requiredMode == PlatformSupport.federated) {
59-
return false;
60-
}
61-
if (!pluginSection.containsKey('platforms')) {
62-
return platform == kPlatformIos || platform == kPlatformAndroid;
63-
}
64-
return false;
65-
}
66-
final YamlMap? platformEntry = platforms[platform] as YamlMap?;
44+
final YamlMap? platformEntry =
45+
_readPlatformPubspecSectionForPlugin(platform, plugin);
6746
if (platformEntry == null) {
6847
return false;
6948
}
49+
7050
// If the platform entry is present, then it supports the platform. Check
7151
// for required mode if specified.
7252
if (requiredMode != null) {
@@ -97,9 +77,67 @@ bool pluginSupportsPlatform(
9777
}
9878

9979
return true;
80+
} on YamlException {
81+
return false;
82+
}
83+
}
84+
85+
/// Returns true if [plugin] includes native code for [platform], as opposed to
86+
/// being implemented entirely in Dart.
87+
bool pluginHasNativeCodeForPlatform(String platform, RepositoryPackage plugin) {
88+
if (platform == kPlatformWeb) {
89+
// Web plugins are always Dart-only.
90+
return false;
91+
}
92+
try {
93+
final YamlMap? platformEntry =
94+
_readPlatformPubspecSectionForPlugin(platform, plugin);
95+
if (platformEntry == null) {
96+
return false;
97+
}
98+
// All other platforms currently use pluginClass for indicating the native
99+
// code in the plugin.
100+
final String? pluginClass = platformEntry['pluginClass'] as String?;
101+
// TODO(stuartmorgan): Remove the check for 'none' once none of the plugins
102+
// in the repository use that workaround. See
103+
// https://github.com/flutter/flutter/issues/57497 for context.
104+
return pluginClass != null && pluginClass != 'none';
100105
} on FileSystemException {
101106
return false;
102107
} on YamlException {
103108
return false;
104109
}
105110
}
111+
112+
/// Returns the
113+
/// flutter:
114+
/// plugin:
115+
/// platforms:
116+
/// [platform]:
117+
/// section from [plugin]'s pubspec.yaml, or null if either it is not present,
118+
/// or the pubspec couldn't be read.
119+
YamlMap? _readPlatformPubspecSectionForPlugin(
120+
String platform, RepositoryPackage plugin) {
121+
try {
122+
final File pubspecFile = plugin.pubspecFile;
123+
final YamlMap pubspecYaml =
124+
loadYaml(pubspecFile.readAsStringSync()) as YamlMap;
125+
final YamlMap? flutterSection = pubspecYaml['flutter'] as YamlMap?;
126+
if (flutterSection == null) {
127+
return null;
128+
}
129+
final YamlMap? pluginSection = flutterSection['plugin'] as YamlMap?;
130+
if (pluginSection == null) {
131+
return null;
132+
}
133+
final YamlMap? platforms = pluginSection['platforms'] as YamlMap?;
134+
if (platforms == null) {
135+
return null;
136+
}
137+
return platforms[platform] as YamlMap?;
138+
} on FileSystemException {
139+
return null;
140+
} on YamlException {
141+
return null;
142+
}
143+
}

script/tool/lib/src/native_test_command.dart

+85-4
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ class NativeTestCommand extends PackageLoopingCommand {
4040
argParser.addFlag(kPlatformAndroid, help: 'Runs Android tests');
4141
argParser.addFlag(kPlatformIos, help: 'Runs iOS tests');
4242
argParser.addFlag(kPlatformMacos, help: 'Runs macOS tests');
43+
argParser.addFlag(kPlatformWindows, help: 'Runs Windows tests');
4344

4445
// By default, both unit tests and integration tests are run, but provide
4546
// flags to disable one or the other.
@@ -80,6 +81,7 @@ this command.
8081
kPlatformAndroid: _PlatformDetails('Android', _testAndroid),
8182
kPlatformIos: _PlatformDetails('iOS', _testIos),
8283
kPlatformMacos: _PlatformDetails('macOS', _testMacOS),
84+
kPlatformWindows: _PlatformDetails('Windows', _testWindows),
8385
};
8486
_requestedPlatforms = _platforms.keys
8587
.where((String platform) => getBoolArg(platform))
@@ -96,6 +98,11 @@ this command.
9698
throw ToolExit(exitInvalidArguments);
9799
}
98100

101+
if (getBoolArg(kPlatformWindows) && getBoolArg(_integrationTestFlag)) {
102+
logWarning('This command currently only supports unit tests for Windows. '
103+
'See https://github.com/flutter/flutter/issues/70233.');
104+
}
105+
99106
// iOS-specific run-level state.
100107
if (_requestedPlatforms.contains('ios')) {
101108
String destination = getStringArg(_iosDestinationFlag);
@@ -119,16 +126,20 @@ this command.
119126
Future<PackageResult> runForPackage(RepositoryPackage package) async {
120127
final List<String> testPlatforms = <String>[];
121128
for (final String platform in _requestedPlatforms) {
122-
if (pluginSupportsPlatform(platform, package,
129+
if (!pluginSupportsPlatform(platform, package,
123130
requiredMode: PlatformSupport.inline)) {
124-
testPlatforms.add(platform);
125-
} else {
126131
print('No implementation for ${_platforms[platform]!.label}.');
132+
continue;
127133
}
134+
if (!pluginHasNativeCodeForPlatform(platform, package)) {
135+
print('No native code for ${_platforms[platform]!.label}.');
136+
continue;
137+
}
138+
testPlatforms.add(platform);
128139
}
129140

130141
if (testPlatforms.isEmpty) {
131-
return PackageResult.skip('Not implemented for target platform(s).');
142+
return PackageResult.skip('Nothing to test for target platform(s).');
132143
}
133144

134145
final _TestMode mode = _TestMode(
@@ -228,6 +239,8 @@ this command.
228239
final bool hasIntegrationTests =
229240
exampleHasNativeIntegrationTests(example);
230241

242+
// TODO(stuartmorgan): Make !hasUnitTests fatal. See
243+
// https://github.com/flutter/flutter/issues/85469
231244
if (mode.unit && !hasUnitTests) {
232245
_printNoExampleTestsMessage(example, 'Android unit');
233246
}
@@ -335,6 +348,9 @@ this command.
335348
for (final RepositoryPackage example in plugin.getExamples()) {
336349
final String exampleName = example.displayName;
337350

351+
// TODO(stuartmorgan): Always check for RunnerTests, and make it fatal if
352+
// no examples have it. See
353+
// https://github.com/flutter/flutter/issues/85469
338354
if (testTarget != null) {
339355
final Directory project = example.directory
340356
.childDirectory(platform.toLowerCase())
@@ -387,6 +403,71 @@ this command.
387403
return _PlatformResult(overallResult);
388404
}
389405

406+
Future<_PlatformResult> _testWindows(
407+
RepositoryPackage plugin, _TestMode mode) async {
408+
if (mode.integrationOnly) {
409+
return _PlatformResult(RunState.skipped);
410+
}
411+
412+
bool isTestBinary(File file) {
413+
return file.basename.endsWith('_test.exe') ||
414+
file.basename.endsWith('_tests.exe');
415+
}
416+
417+
return _runGoogleTestTests(plugin,
418+
buildDirectoryName: 'windows', isTestBinary: isTestBinary);
419+
}
420+
421+
/// Finds every file in the [buildDirectoryName] subdirectory of [plugin]'s
422+
/// build directory for which [isTestBinary] is true, and runs all of them,
423+
/// returning the overall result.
424+
///
425+
/// The binaries are assumed to be Google Test test binaries, thus returning
426+
/// zero for success and non-zero for failure.
427+
Future<_PlatformResult> _runGoogleTestTests(
428+
RepositoryPackage plugin, {
429+
required String buildDirectoryName,
430+
required bool Function(File) isTestBinary,
431+
}) async {
432+
final List<File> testBinaries = <File>[];
433+
for (final RepositoryPackage example in plugin.getExamples()) {
434+
final Directory buildDir = example.directory
435+
.childDirectory('build')
436+
.childDirectory(buildDirectoryName);
437+
if (!buildDir.existsSync()) {
438+
continue;
439+
}
440+
testBinaries.addAll(buildDir
441+
.listSync(recursive: true)
442+
.whereType<File>()
443+
.where(isTestBinary)
444+
.where((File file) {
445+
// Only run the debug build of the unit tests, to avoid running the
446+
// same tests multiple times.
447+
final List<String> components = path.split(file.path);
448+
return components.contains('debug') || components.contains('Debug');
449+
}));
450+
}
451+
452+
if (testBinaries.isEmpty) {
453+
final String binaryExtension = platform.isWindows ? '.exe' : '';
454+
printError(
455+
'No test binaries found. At least one *_test(s)$binaryExtension '
456+
'binary should be built by the example(s)');
457+
return _PlatformResult(RunState.failed,
458+
error: 'No $buildDirectoryName unit tests found');
459+
}
460+
461+
bool passing = true;
462+
for (final File test in testBinaries) {
463+
print('Running ${test.basename}...');
464+
final int exitCode =
465+
await processRunner.runAndStream(test.path, <String>[]);
466+
passing &= exitCode == 0;
467+
}
468+
return _PlatformResult(passing ? RunState.succeeded : RunState.failed);
469+
}
470+
390471
/// Prints a standard format message indicating that [platform] tests for
391472
/// [plugin]'s [example] are about to be run.
392473
void _printRunningExampleTestsMessage(

script/tool/lib/src/publish_plugin_command.dart

+5-7
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import 'package:pubspec_parse/pubspec_parse.dart';
1818
import 'package:yaml/yaml.dart';
1919

2020
import 'common/core.dart';
21+
import 'common/file_utils.dart';
2122
import 'common/git_version_finder.dart';
2223
import 'common/plugin_command.dart';
2324
import 'common/process_runner.dart';
@@ -154,13 +155,10 @@ class PublishPluginCommand extends PluginCommand {
154155
await gitVersionFinder.getChangedPubSpecs();
155156

156157
for (final String pubspecPath in changedPubspecs) {
157-
// Convert git's Posix-style paths to a path that matches the current
158-
// filesystem.
159-
final String localStylePubspecPath =
160-
path.joinAll(p.posix.split(pubspecPath));
161-
final File pubspecFile = packagesDir.fileSystem
162-
.directory((await gitDir).path)
163-
.childFile(localStylePubspecPath);
158+
// git outputs a relativa, Posix-style path.
159+
final File pubspecFile = childFileWithSubcomponents(
160+
packagesDir.fileSystem.directory((await gitDir).path),
161+
p.posix.split(pubspecPath));
164162
yield PackageEnumerationEntry(RepositoryPackage(pubspecFile.parent),
165163
excluded: false);
166164
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
// Copyright 2013 The Flutter Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
import 'package:file/file.dart';
6+
import 'package:file/memory.dart';
7+
import 'package:flutter_plugin_tools/src/common/file_utils.dart';
8+
import 'package:test/test.dart';
9+
10+
void main() {
11+
test('works on Posix', () async {
12+
final FileSystem fileSystem =
13+
MemoryFileSystem(style: FileSystemStyle.posix);
14+
15+
final Directory base = fileSystem.directory('/').childDirectory('base');
16+
final File file =
17+
childFileWithSubcomponents(base, <String>['foo', 'bar', 'baz.txt']);
18+
19+
expect(file.absolute.path, '/base/foo/bar/baz.txt');
20+
});
21+
22+
test('works on Windows', () async {
23+
final FileSystem fileSystem =
24+
MemoryFileSystem(style: FileSystemStyle.windows);
25+
26+
final Directory base = fileSystem.directory(r'C:\').childDirectory('base');
27+
final File file =
28+
childFileWithSubcomponents(base, <String>['foo', 'bar', 'baz.txt']);
29+
30+
expect(file.absolute.path, r'C:\base\foo\bar\baz.txt');
31+
});
32+
}

0 commit comments

Comments
 (0)