Skip to content

Commit 4a6a444

Browse files
authored
[ci] refactor: Cleanup tooling for handling arch build output <= Flutter v3.13.x (#11399)
There remained some code, which was left in to support build output for Flutter <= v3.16.x (see: flutter/flutter@792e26d) As the minimum supported Flutter version is 3.35.x as of https://github.com/flutter/packages/blob/eab1265529a0fe75e1f800ecf96c5012be155942/.ci/targets/repo_checks.yaml#L31 we can remove the special handling for versions prior v3.16.x. Contributes to flutter/flutter#129807 ## Pre-Review Checklist [^1]: Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling.
1 parent 19ec8b8 commit 4a6a444

4 files changed

Lines changed: 48 additions & 131 deletions

File tree

packages/pigeon/tool/shared/test_suites.dart

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -409,6 +409,15 @@ Future<int> _runLinuxUnitTests({bool ciMode = false}) async {
409409
return compileCode;
410410
}
411411

412+
// Depending on the Flutter version, the build output path may be different.
413+
// To handle both master and stable, and to future-proof against the changes
414+
// that will happen in https://github.com/flutter/flutter/issues/114349
415+
// - Try arm64, to future-proof against arm64 support.
416+
// - Try x64, to cover pre-arm64 support on arm64 hosts, as well as x64 hosts.
417+
// TODO(gustl22): Remove all this when these tests no longer need to
418+
// support a version of Flutter without
419+
// https://github.com/flutter/flutter/issues/114349, and just construct the
420+
// version of the path with the current architecture.
412421
const buildDirBase = '$examplePath/build/linux';
413422
const buildRelativeBinaryPath = 'debug/plugins/test_plugin/test_plugin_test';
414423
const arm64Path = '$buildDirBase/arm64/$buildRelativeBinaryPath';

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

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ class CMakeProject {
2020
required this.buildMode,
2121
this.processRunner = const ProcessRunner(),
2222
this.platform = const LocalPlatform(),
23-
this.arch,
23+
required this.arch,
2424
});
2525

2626
/// The directory of a Flutter project to run Gradle commands in.
@@ -33,13 +33,11 @@ class CMakeProject {
3333
final Platform platform;
3434

3535
/// The architecture subdirectory of the build.
36-
// TODO(stuartmorgan): Make this non-nullable once Flutter 3.13 is no longer
37-
// supported, since at that point there will always be a subdirectory.
38-
final String? arch;
36+
final String arch;
3937

4038
/// The build mode (e.g., Debug, Release).
4139
///
42-
/// This is a constructor paramater because on Linux many properties depend
40+
/// This is a constructor parameter because on Linux many properties depend
4341
/// on the build mode since it uses a single-configuration generator.
4442
final String buildMode;
4543

@@ -52,10 +50,8 @@ class CMakeProject {
5250
Directory get buildDirectory {
5351
Directory buildDir = flutterProject
5452
.childDirectory('build')
55-
.childDirectory(_platformDirName);
56-
if (arch != null) {
57-
buildDir = buildDir.childDirectory(arch!);
58-
}
53+
.childDirectory(_platformDirName)
54+
.childDirectory(arch);
5955
if (platform.isLinux) {
6056
// Linux uses a single-config generator, so the base build directory
6157
// includes the configuration.

script/tool/lib/src/native_test_command.dart

Lines changed: 29 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -681,12 +681,25 @@ this command.
681681
const x64DirName = 'x64';
682682
const arm64DirName = 'arm64';
683683
if (platform.isWindows) {
684-
arch = _abi == Abi.windowsX64 ? x64DirName : arm64DirName;
684+
arch = switch (_abi) {
685+
Abi.windowsX64 => x64DirName,
686+
Abi.windowsArm64 => arm64DirName,
687+
_ => null,
688+
};
685689
} else if (platform.isLinux) {
686690
// TODO(stuartmorgan): Support arm64 if that ever becomes a supported
687691
// CI configuration for the repository.
688-
arch = 'x64';
692+
// See: https://github.com/flutter/flutter/issues/114349
693+
arch = x64DirName;
694+
}
695+
696+
if (arch == null) {
697+
return _PlatformResult(
698+
RunState.failed,
699+
error: 'Google Test testing is not supported on $platform $_abi.',
700+
);
689701
}
702+
690703
for (final RepositoryPackage example in plugin.getExamples()) {
691704
var project = CMakeProject(
692705
example.directory,
@@ -695,32 +708,20 @@ this command.
695708
platform: platform,
696709
arch: arch,
697710
);
698-
if (platform.isWindows) {
699-
if (arch == arm64DirName && !project.isConfigured()) {
700-
// Check for x64, to handle builds newer than 3.13, but that don't yet
701-
// have https://github.com/flutter/flutter/issues/129807.
702-
// TODO(stuartmorgan): Remove this when CI no longer supports a
703-
// version of Flutter without the issue above fixed.
704-
project = CMakeProject(
705-
example.directory,
706-
buildMode: buildMode,
707-
processRunner: processRunner,
708-
platform: platform,
709-
arch: x64DirName,
710-
);
711-
}
712-
if (!project.isConfigured()) {
713-
// Check again without the arch subdirectory, since 3.13 doesn't
714-
// have it yet.
715-
// TODO(stuartmorgan): Remove this when CI no longer supports Flutter
716-
// 3.13.
717-
project = CMakeProject(
718-
example.directory,
719-
buildMode: buildMode,
720-
processRunner: processRunner,
721-
platform: platform,
722-
);
723-
}
711+
if (platform.isWindows &&
712+
arch == arm64DirName &&
713+
!project.isConfigured()) {
714+
// Check for x64, to handle builds that don't yet have
715+
// https://github.com/flutter/flutter/issues/129807.
716+
// TODO(stuartmorgan): Remove this when CI no longer supports a
717+
// version of Flutter without the issue above fixed.
718+
project = CMakeProject(
719+
example.directory,
720+
buildMode: buildMode,
721+
processRunner: processRunner,
722+
platform: platform,
723+
arch: x64DirName,
724+
);
724725
}
725726
if (!project.isConfigured()) {
726727
printError(

script/tool/test/native_test_command_test.dart

Lines changed: 5 additions & 94 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ const String _archDirArm64 = 'arm64';
6868
void _createFakeCMakeCache(
6969
RepositoryPackage plugin,
7070
Platform platform,
71-
String? archDir,
71+
String archDir,
7272
) {
7373
final project = CMakeProject(
7474
getExampleDir(plugin),
@@ -2231,13 +2231,11 @@ public class FlutterActivityTest {
22312231

22322232
// Returns the ProcessCall to expect for build the Windows unit tests for
22332233
// the given plugin.
2234-
ProcessCall getWindowsBuildCall(RepositoryPackage plugin, String? arch) {
2235-
Directory projectDir = getExampleDir(
2234+
ProcessCall getWindowsBuildCall(RepositoryPackage plugin, String arch) {
2235+
final Directory projectDir = getExampleDir(
22362236
plugin,
2237-
).childDirectory('build').childDirectory('windows');
2238-
if (arch != null) {
2239-
projectDir = projectDir.childDirectory(arch);
2240-
}
2237+
).childDirectory('build').childDirectory('windows').childDirectory(arch);
2238+
22412239
return ProcessCall(_fakeCmakeCommand, <String>[
22422240
'--build',
22432241
projectDir.path,
@@ -2311,47 +2309,6 @@ public class FlutterActivityTest {
23112309
);
23122310
});
23132311

2314-
test('runs unit tests with legacy build output', () async {
2315-
const testBinaryRelativePath =
2316-
'build/windows/Debug/bar/plugin_test.exe';
2317-
final RepositoryPackage plugin = createFakePlugin(
2318-
'plugin',
2319-
packagesDir,
2320-
extraFiles: <String>['example/$testBinaryRelativePath'],
2321-
platformSupport: <String, PlatformDetails>{
2322-
platformWindows: const PlatformDetails(PlatformSupport.inline),
2323-
},
2324-
);
2325-
_createFakeCMakeCache(plugin, mockPlatform, null);
2326-
2327-
final File testBinary = childFileWithSubcomponents(
2328-
plugin.directory,
2329-
<String>['example', ...testBinaryRelativePath.split('/')],
2330-
);
2331-
2332-
final List<String> output = await runCapturingPrint(runner, <String>[
2333-
'native-test',
2334-
'--windows',
2335-
'--no-integration',
2336-
]);
2337-
2338-
expect(
2339-
output,
2340-
containsAllInOrder(<Matcher>[
2341-
contains('Running plugin_test.exe...'),
2342-
contains('No issues found!'),
2343-
]),
2344-
);
2345-
2346-
expect(
2347-
processRunner.recordedCalls,
2348-
orderedEquals(<ProcessCall>[
2349-
getWindowsBuildCall(plugin, null),
2350-
ProcessCall(testBinary.path, const <String>[], null),
2351-
]),
2352-
);
2353-
});
2354-
23552312
test('only runs debug unit tests', () async {
23562313
const debugTestBinaryRelativePath =
23572314
'build/windows/x64/Debug/bar/plugin_test.exe';
@@ -2398,52 +2355,6 @@ public class FlutterActivityTest {
23982355
);
23992356
});
24002357

2401-
test('only runs debug unit tests with legacy build output', () async {
2402-
const debugTestBinaryRelativePath =
2403-
'build/windows/Debug/bar/plugin_test.exe';
2404-
const releaseTestBinaryRelativePath =
2405-
'build/windows/Release/bar/plugin_test.exe';
2406-
final RepositoryPackage plugin = createFakePlugin(
2407-
'plugin',
2408-
packagesDir,
2409-
extraFiles: <String>[
2410-
'example/$debugTestBinaryRelativePath',
2411-
'example/$releaseTestBinaryRelativePath',
2412-
],
2413-
platformSupport: <String, PlatformDetails>{
2414-
platformWindows: const PlatformDetails(PlatformSupport.inline),
2415-
},
2416-
);
2417-
_createFakeCMakeCache(plugin, mockPlatform, null);
2418-
2419-
final File debugTestBinary = childFileWithSubcomponents(
2420-
plugin.directory,
2421-
<String>['example', ...debugTestBinaryRelativePath.split('/')],
2422-
);
2423-
2424-
final List<String> output = await runCapturingPrint(runner, <String>[
2425-
'native-test',
2426-
'--windows',
2427-
'--no-integration',
2428-
]);
2429-
2430-
expect(
2431-
output,
2432-
containsAllInOrder(<Matcher>[
2433-
contains('Running plugin_test.exe...'),
2434-
contains('No issues found!'),
2435-
]),
2436-
);
2437-
2438-
expect(
2439-
processRunner.recordedCalls,
2440-
orderedEquals(<ProcessCall>[
2441-
getWindowsBuildCall(plugin, null),
2442-
ProcessCall(debugTestBinary.path, const <String>[], null),
2443-
]),
2444-
);
2445-
});
2446-
24472358
test('fails if CMake has not been configured', () async {
24482359
createFakePlugin(
24492360
'plugin',

0 commit comments

Comments
 (0)