From cc90a424c98698ed78281b007a50cfb1da1c004d Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Mon, 4 Nov 2024 17:39:25 -0800 Subject: [PATCH] Extract and restore a test that a blank native assets project still builds (#158141) Closes https://github.com/flutter/flutter/issues/158120. This PR restores the skipped test, moving it (and the test utility only used by the test) into a standalone file that can be more easily understood. As part of the change the version of `native_assets_cli` is now derived from the (checked-in) `package_ffi/pubspec.yaml.tmpl`, meaning that it should be hard to get into a bad state again. /cc @christopherfujino (You are welcome to review, but otherwise will defer to Brandon and Victor). --- .../isolated/native_assets_test.dart | 20 --- .../isolated/native_assets_test_utils.dart | 55 ------ ...e_assets_without_cbuild_assemble_test.dart | 169 ++++++++++++++++++ 3 files changed, 169 insertions(+), 75 deletions(-) create mode 100644 packages/flutter_tools/test/integration.shard/isolated/native_assets_without_cbuild_assemble_test.dart diff --git a/packages/flutter_tools/test/integration.shard/isolated/native_assets_test.dart b/packages/flutter_tools/test/integration.shard/isolated/native_assets_test.dart index b7549804e00..cb5890a2248 100644 --- a/packages/flutter_tools/test/integration.shard/isolated/native_assets_test.dart +++ b/packages/flutter_tools/test/integration.shard/isolated/native_assets_test.dart @@ -223,26 +223,6 @@ void main() { }); } - testWithoutContext('flutter build $buildSubcommand succeeds without libraries', () async { - await inTempDir((Directory tempDirectory) async { - final Directory projectDirectory = await createTestProjectWithNoCBuild(packageName, tempDirectory); - - final ProcessResult result = processManager.runSync( - [ - flutterBin, - 'build', - buildSubcommand, - '--debug', - if (buildSubcommand == 'ios') '--no-codesign', - ], - workingDirectory: projectDirectory.path, - ); - if (result.exitCode != 0) { - throw Exception('flutter build failed: ${result.exitCode}\n${result.stderr}\n${result.stdout}'); - } - }); - }, skip: true); // https://github.com/flutter/flutter/issues/158120 - // This could be an hermetic unit test if the native_assets_builder // could mock process runs and file system. // https://github.com/dart-lang/native/issues/90. diff --git a/packages/flutter_tools/test/integration.shard/isolated/native_assets_test_utils.dart b/packages/flutter_tools/test/integration.shard/isolated/native_assets_test_utils.dart index 349b53f0613..fa9ab50b92c 100644 --- a/packages/flutter_tools/test/integration.shard/isolated/native_assets_test_utils.dart +++ b/packages/flutter_tools/test/integration.shard/isolated/native_assets_test_utils.dart @@ -57,61 +57,6 @@ Future createTestProject(String packageName, Directory tempDirectory) return packageDirectory; } -Future createTestProjectWithNoCBuild(String packageName, Directory tempDirectory) async { - final ProcessResult result = processManager.runSync( - [ - flutterBin, - 'create', - '--no-pub', - packageName, - ], - workingDirectory: tempDirectory.path, - ); - if (result.exitCode != 0) { - throw Exception( - 'flutter create failed: ${result.exitCode}\n${result.stderr}\n${result.stdout}', - ); - } - - final Directory packageDirectory = tempDirectory.childDirectory(packageName); - - final ProcessResult result2 = await processManager.run( - [ - flutterBin, - 'pub', - 'add', - 'native_assets_cli', - ], - workingDirectory: packageDirectory.path, - ); - expect(result2, const ProcessResultMatcher()); - - await pinDependencies(packageDirectory.childFile('pubspec.yaml')); - - final ProcessResult result3 = await processManager.run( - [ - flutterBin, - 'pub', - 'get', - ], - workingDirectory: packageDirectory.path, - ); - expect(result3, const ProcessResultMatcher()); - - // Add build hook that does nothing to the package. - final File buildHook = packageDirectory.childDirectory('hook').childFile('build.dart'); - buildHook.createSync(recursive: true); - buildHook.writeAsStringSync(''' -import 'package:native_assets_cli/native_assets_cli.dart'; - -void main(List args) async { - await build(args, (config, output) async {}); -} -'''); - - return packageDirectory; -} - Future addLinkHookDependency(String packageName, Directory packageDirectory) async { final Directory flutterDirectory = fileSystem.currentDirectory.parent.parent; final Directory linkHookDirectory = flutterDirectory diff --git a/packages/flutter_tools/test/integration.shard/isolated/native_assets_without_cbuild_assemble_test.dart b/packages/flutter_tools/test/integration.shard/isolated/native_assets_without_cbuild_assemble_test.dart new file mode 100644 index 00000000000..8f953c059f4 --- /dev/null +++ b/packages/flutter_tools/test/integration.shard/isolated/native_assets_without_cbuild_assemble_test.dart @@ -0,0 +1,169 @@ +// Copyright 2014 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:process/process.dart'; +import 'package:yaml/yaml.dart'; + +import '../../src/common.dart'; +import '../test_utils.dart'; +import '../transition_test_utils.dart'; +import 'native_assets_test_utils.dart'; + +/// Regression test as part of https://github.com/flutter/flutter/pull/150742. +/// +/// Previously, creating a new (blank, i.e. from `flutter create`) Flutter +/// project, adding `native_assets_cli`, and adding an otherwise valid build hook +/// (`/hook/build.dart`) would fail to build due to the accompanying shell script +/// (at least on macOS) assuming the glob would find at least one output. +/// +/// This test verifies that a blank Flutter project with native_assets_cli can +/// build, and does so across all of the host platform and target platform +/// combinations that could trigger this error. +/// +/// The version of `native_assets_cli` is derived from the template used by +/// `flutter create --type=pacakges_ffi`. See +/// [_getPackageFfiTemplatePubspecVersion]. +void main() { + if (!platform.isMacOS && !platform.isLinux && !platform.isWindows) { + // TODO(dacoharkes): Implement Fuchsia. https://github.com/flutter/flutter/issues/129757 + return; + } + + + const ProcessManager processManager = LocalProcessManager(); + final String constraint = _getPackageFfiTemplatePubspecVersion(); + + setUpAll(() { + processManager.runSync([ + flutterBin, + 'config', + '--enable-native-assets', + ]); + }); + + // Test building a host, iOS, and APK (Android) target where possible. + for (final String buildCommand in [ + // Current (Host) OS. + platform.operatingSystem, + + // On macOS, also test iOS. + if (platform.isMacOS) 'ios', + + // On every host platform, test Android. + 'apk', + ]) { + _testBuildCommand( + buildCommand: buildCommand, + processManager: processManager, + nativeAssetsCliVersionConstraint: constraint, + codeSign: buildCommand != 'ios', + ); + } +} + +void _testBuildCommand({ + required String buildCommand, + required String nativeAssetsCliVersionConstraint, + required ProcessManager processManager, + required bool codeSign, +}) { + testWithoutContext( + 'flutter build "$buildCommand" succeeds without libraries', + () async { + await inTempDir((Directory tempDirectory) async { + const String packageName = 'uses_package_native_assets_cli'; + + // Create a new (plain Dart SDK) project. + await expectLater( + processManager.run( + [ + flutterBin, + 'create', + '--no-pub', + packageName, + ], + workingDirectory: tempDirectory.path, + ), + completion(const ProcessResultMatcher()), + ); + + final Directory packageDirectory = tempDirectory.childDirectory( + packageName, + ); + + // Add native_assets_cli and resolve implicitly (pub add does pub get). + // See https://dart.dev/tools/pub/cmd/pub-add#version-constraint. + await expectLater( + processManager.run( + [ + flutterBin, + 'packages', + 'add', + 'native_assets_cli:$nativeAssetsCliVersionConstraint', + ], + workingDirectory: packageDirectory.path, + ), + completion(const ProcessResultMatcher()), + ); + + // Add a build hook that does nothing to the package. + packageDirectory.childDirectory('hook').childFile('build.dart') + ..createSync(recursive: true) + ..writeAsStringSync(''' +import 'package:native_assets_cli/native_assets_cli.dart'; + +void main(List args) async { + await build(args, (config, output) async {}); +} +'''); + + // Try building. + await expectLater( + processManager.run( + [ + flutterBin, + 'build', + buildCommand, + '--debug', + if (!codeSign) '--no-codesign', + ], + workingDirectory: packageDirectory.path, + ), + completion(const ProcessResultMatcher()), + ); + }); + }, + ); +} + +/// Reads `templates/package_ffi/pubspec.yaml.tmpl` to use the package version. +/// +/// For example, if the template would output: +/// ```yaml +/// dependencies: +/// native_assets_cli: ^0.8.0 +/// ``` +/// +/// ... then this function would return `'^0.8.0'`. +String _getPackageFfiTemplatePubspecVersion() { + final String path = Context().join( + getFlutterRoot(), + 'packages', + 'flutter_tools', + 'templates', + 'package_ffi', + 'pubspec.yaml.tmpl', + ); + final YamlDocument yaml = loadYamlDocument( + io.File(path).readAsStringSync(), + sourceUrl: Uri.parse(path), + ); + final YamlMap rootNode = yaml.contents as YamlMap; + final YamlMap dependencies = rootNode.nodes['dependencies']! as YamlMap; + final String version = dependencies['native_assets_cli']! as String; + return version; +}