Skip to content

Commit cc90a42

Browse files
matanlureymatanlurey
authored
Extract and restore a test that a blank native assets project still builds (#158141)
Closes flutter/flutter#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).
1 parent abcdcee commit cc90a42

File tree

3 files changed

+169
-75
lines changed

3 files changed

+169
-75
lines changed

‎packages/flutter_tools/test/integration.shard/isolated/native_assets_test.dart‎

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -223,26 +223,6 @@ void main() {
223223
});
224224
}
225225

226-
testWithoutContext('flutter build $buildSubcommand succeeds without libraries', () async {
227-
await inTempDir((Directory tempDirectory) async {
228-
final Directory projectDirectory = await createTestProjectWithNoCBuild(packageName, tempDirectory);
229-
230-
final ProcessResult result = processManager.runSync(
231-
<String>[
232-
flutterBin,
233-
'build',
234-
buildSubcommand,
235-
'--debug',
236-
if (buildSubcommand == 'ios') '--no-codesign',
237-
],
238-
workingDirectory: projectDirectory.path,
239-
);
240-
if (result.exitCode != 0) {
241-
throw Exception('flutter build failed: ${result.exitCode}\n${result.stderr}\n${result.stdout}');
242-
}
243-
});
244-
}, skip: true); // https://github.com/flutter/flutter/issues/158120
245-
246226
// This could be an hermetic unit test if the native_assets_builder
247227
// could mock process runs and file system.
248228
// https://github.com/dart-lang/native/issues/90.

‎packages/flutter_tools/test/integration.shard/isolated/native_assets_test_utils.dart‎

Lines changed: 0 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -57,61 +57,6 @@ Future<Directory> createTestProject(String packageName, Directory tempDirectory)
5757
return packageDirectory;
5858
}
5959

60-
Future<Directory> createTestProjectWithNoCBuild(String packageName, Directory tempDirectory) async {
61-
final ProcessResult result = processManager.runSync(
62-
<String>[
63-
flutterBin,
64-
'create',
65-
'--no-pub',
66-
packageName,
67-
],
68-
workingDirectory: tempDirectory.path,
69-
);
70-
if (result.exitCode != 0) {
71-
throw Exception(
72-
'flutter create failed: ${result.exitCode}\n${result.stderr}\n${result.stdout}',
73-
);
74-
}
75-
76-
final Directory packageDirectory = tempDirectory.childDirectory(packageName);
77-
78-
final ProcessResult result2 = await processManager.run(
79-
<String>[
80-
flutterBin,
81-
'pub',
82-
'add',
83-
'native_assets_cli',
84-
],
85-
workingDirectory: packageDirectory.path,
86-
);
87-
expect(result2, const ProcessResultMatcher());
88-
89-
await pinDependencies(packageDirectory.childFile('pubspec.yaml'));
90-
91-
final ProcessResult result3 = await processManager.run(
92-
<String>[
93-
flutterBin,
94-
'pub',
95-
'get',
96-
],
97-
workingDirectory: packageDirectory.path,
98-
);
99-
expect(result3, const ProcessResultMatcher());
100-
101-
// Add build hook that does nothing to the package.
102-
final File buildHook = packageDirectory.childDirectory('hook').childFile('build.dart');
103-
buildHook.createSync(recursive: true);
104-
buildHook.writeAsStringSync('''
105-
import 'package:native_assets_cli/native_assets_cli.dart';
106-
107-
void main(List<String> args) async {
108-
await build(args, (config, output) async {});
109-
}
110-
''');
111-
112-
return packageDirectory;
113-
}
114-
11560
Future<void> addLinkHookDependency(String packageName, Directory packageDirectory) async {
11661
final Directory flutterDirectory = fileSystem.currentDirectory.parent.parent;
11762
final Directory linkHookDirectory = flutterDirectory

‎packages/flutter_tools/test/integration.shard/isolated/native_assets_without_cbuild_assemble_test.dart‎

Lines changed: 169 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,169 @@
1+
// Copyright 2014 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 'dart:io' as io;
6+
7+
import 'package:file/file.dart';
8+
import 'package:process/process.dart';
9+
import 'package:yaml/yaml.dart';
10+
11+
import '../../src/common.dart';
12+
import '../test_utils.dart';
13+
import '../transition_test_utils.dart';
14+
import 'native_assets_test_utils.dart';
15+
16+
/// Regression test as part of https://github.com/flutter/flutter/pull/150742.
17+
///
18+
/// Previously, creating a new (blank, i.e. from `flutter create`) Flutter
19+
/// project, adding `native_assets_cli`, and adding an otherwise valid build hook
20+
/// (`/hook/build.dart`) would fail to build due to the accompanying shell script
21+
/// (at least on macOS) assuming the glob would find at least one output.
22+
///
23+
/// This test verifies that a blank Flutter project with native_assets_cli can
24+
/// build, and does so across all of the host platform and target platform
25+
/// combinations that could trigger this error.
26+
///
27+
/// The version of `native_assets_cli` is derived from the template used by
28+
/// `flutter create --type=pacakges_ffi`. See
29+
/// [_getPackageFfiTemplatePubspecVersion].
30+
void main() {
31+
if (!platform.isMacOS && !platform.isLinux && !platform.isWindows) {
32+
// TODO(dacoharkes): Implement Fuchsia. https://github.com/flutter/flutter/issues/129757
33+
return;
34+
}
35+
36+
37+
const ProcessManager processManager = LocalProcessManager();
38+
final String constraint = _getPackageFfiTemplatePubspecVersion();
39+
40+
setUpAll(() {
41+
processManager.runSync(<String>[
42+
flutterBin,
43+
'config',
44+
'--enable-native-assets',
45+
]);
46+
});
47+
48+
// Test building a host, iOS, and APK (Android) target where possible.
49+
for (final String buildCommand in <String>[
50+
// Current (Host) OS.
51+
platform.operatingSystem,
52+
53+
// On macOS, also test iOS.
54+
if (platform.isMacOS) 'ios',
55+
56+
// On every host platform, test Android.
57+
'apk',
58+
]) {
59+
_testBuildCommand(
60+
buildCommand: buildCommand,
61+
processManager: processManager,
62+
nativeAssetsCliVersionConstraint: constraint,
63+
codeSign: buildCommand != 'ios',
64+
);
65+
}
66+
}
67+
68+
void _testBuildCommand({
69+
required String buildCommand,
70+
required String nativeAssetsCliVersionConstraint,
71+
required ProcessManager processManager,
72+
required bool codeSign,
73+
}) {
74+
testWithoutContext(
75+
'flutter build "$buildCommand" succeeds without libraries',
76+
() async {
77+
await inTempDir((Directory tempDirectory) async {
78+
const String packageName = 'uses_package_native_assets_cli';
79+
80+
// Create a new (plain Dart SDK) project.
81+
await expectLater(
82+
processManager.run(
83+
<String>[
84+
flutterBin,
85+
'create',
86+
'--no-pub',
87+
packageName,
88+
],
89+
workingDirectory: tempDirectory.path,
90+
),
91+
completion(const ProcessResultMatcher()),
92+
);
93+
94+
final Directory packageDirectory = tempDirectory.childDirectory(
95+
packageName,
96+
);
97+
98+
// Add native_assets_cli and resolve implicitly (pub add does pub get).
99+
// See https://dart.dev/tools/pub/cmd/pub-add#version-constraint.
100+
await expectLater(
101+
processManager.run(
102+
<String>[
103+
flutterBin,
104+
'packages',
105+
'add',
106+
'native_assets_cli:$nativeAssetsCliVersionConstraint',
107+
],
108+
workingDirectory: packageDirectory.path,
109+
),
110+
completion(const ProcessResultMatcher()),
111+
);
112+
113+
// Add a build hook that does nothing to the package.
114+
packageDirectory.childDirectory('hook').childFile('build.dart')
115+
..createSync(recursive: true)
116+
..writeAsStringSync('''
117+
import 'package:native_assets_cli/native_assets_cli.dart';
118+
119+
void main(List<String> args) async {
120+
await build(args, (config, output) async {});
121+
}
122+
''');
123+
124+
// Try building.
125+
await expectLater(
126+
processManager.run(
127+
<String>[
128+
flutterBin,
129+
'build',
130+
buildCommand,
131+
'--debug',
132+
if (!codeSign) '--no-codesign',
133+
],
134+
workingDirectory: packageDirectory.path,
135+
),
136+
completion(const ProcessResultMatcher()),
137+
);
138+
});
139+
},
140+
);
141+
}
142+
143+
/// Reads `templates/package_ffi/pubspec.yaml.tmpl` to use the package version.
144+
///
145+
/// For example, if the template would output:
146+
/// ```yaml
147+
/// dependencies:
148+
/// native_assets_cli: ^0.8.0
149+
/// ```
150+
///
151+
/// ... then this function would return `'^0.8.0'`.
152+
String _getPackageFfiTemplatePubspecVersion() {
153+
final String path = Context().join(
154+
getFlutterRoot(),
155+
'packages',
156+
'flutter_tools',
157+
'templates',
158+
'package_ffi',
159+
'pubspec.yaml.tmpl',
160+
);
161+
final YamlDocument yaml = loadYamlDocument(
162+
io.File(path).readAsStringSync(),
163+
sourceUrl: Uri.parse(path),
164+
);
165+
final YamlMap rootNode = yaml.contents as YamlMap;
166+
final YamlMap dependencies = rootNode.nodes['dependencies']! as YamlMap;
167+
final String version = dependencies['native_assets_cli']! as String;
168+
return version;
169+
}

0 commit comments

Comments
 (0)