firka/flutter
3
0
Fork 1
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Overhaul update_engine_version.{sh|ps1} to reflect the new computation flow (#164513)

Browse Source 
Closes https://github.com/flutter/flutter/issues/164030.
Closes https://github.com/flutter/flutter/issues/164315.
Towards https://github.com/flutter/flutter/issues/163896.

Significantly simplified! We removed:

- Conditional checks for monorepo (it's always a monorepo)
- Conditional checks for `LUCI_CONTEXT` (LUCI takes care of itself now,
see https://github.com/flutter/cocoon/pull/4261)
- ... and made the branching logic easier to follow as a result

You can see the results first hand in the tests, which are now much
simpler.

Canonical docs:
https://github.com/flutter/flutter/blob/master/docs/tool/Engine-artifacts.md.
This commit is contained in:
Matan Lurey
2025-03-03 17:08:59 -08:00
committed by GitHub
parent ad3d8f5934
commit 2f2837b30d
4 changed files with 235 additions and 323 deletions
Download Patch File Download Diff File Expand all files Collapse all files

379
dev/tools/test/update_engine_version_test.dart
View File

@@ -9,7 +9,6 @@ import 'dart:io' as io;
import 'package:file/file.dart';
import 'package:file/local.dart';
import 'package:file_testing/file_testing.dart';
import 'package:platform/platform.dart';
import 'package:test/test.dart';
@@ -31,11 +30,13 @@ void main() {
// linux: https://learn.microsoft.com/en-us/powershell/scripting/install/installing-powershell-on-linux
//
// Then, set this variable to true:
const bool usePowershellOnPosix = false;
final bool usePowershellOnPosix = () {
// Intentionally not a const so that linting doesn't go wild across the test.
return false;
}();
const FileSystem localFs = LocalFileSystem();
final _FlutterRootUnderTest flutterRoot = _FlutterRootUnderTest.findWithin(
// ignore: avoid_redundant_argument_values
forcePowershell: usePowershellOnPosix,
);
@@ -61,24 +62,30 @@ void main() {
includeParentEnvironment: false,
);
if (result.exitCode != 0) {
print('exitCode: ${result.exitCode}');
fail('Failed running "$executable $args" (exit code = ${result.exitCode})');
}
printIfNotEmpty('stdout', (result.stdout as String).trim());
printIfNotEmpty('stderr', (result.stderr as String).trim());
return result;
}
setUpAll(() async {
if (usePowershellOnPosix) {
final io.ProcessResult result = io.Process.runSync('pwsh', <String>['--version']);
print('Using Powershell (${result.stdout}) on POSIX for local debugging and testing');
}
});
setUp(() async {
tmpDir = localFs.systemTempDirectory.createTempSync('update_engine_version_test.');
testRoot = _FlutterRootUnderTest.fromPath(
tmpDir.childDirectory('flutter').path,
// ignore: avoid_redundant_argument_values
forcePowershell: usePowershellOnPosix,
);
environment = <String, String>{};
if (const LocalPlatform().isWindows) {
if (const LocalPlatform().isWindows || usePowershellOnPosix) {
// Copy a minimal set of environment variables needed to run the update_engine_version script in PowerShell.
const List<String> powerShellVariables = <String>['SystemRoot', 'Path', 'PATHEXT'];
for (final String key in powerShellVariables) {
@@ -121,7 +128,6 @@ void main() {
'internal',
localFs.path.basename(testRoot.binInternalUpdateEngineVersion.path),
),
localFs.path.join('bin', 'internal', 'engine.realm'),
localFs.path.join('bin', 'internal', 'engine.version'),
localFs.path.join('engine', 'src', '.gn'),
'DEPS',
@@ -139,8 +145,7 @@ void main() {
);
// Now do cleanup so even if the next step fails, we still deleted tmp.
print(tmpDir);
// tmpDir.deleteSync(recursive: true);
tmpDir.deleteSync(recursive: true);
final Set<String> unexpectedFiles = currentFiles.difference(expectedFiles);
if (unexpectedFiles.isNotEmpty) {
@@ -156,17 +161,22 @@ void main() {
}
});
/// Runs `bin/internal/update_engine_version.{sh|ps1}` and returns the process result.
///
/// If the exit code is 0, it is considered a success, and files should exist as a side-effect.
///
/// - On Windows, `powershell` is used (to run `update_engine_version.ps1`);
/// - On POSIX, if [usePowershellOnPosix] is set, `pwsh` is used (to run `update_engine_version.ps1`);
/// - Otherwise, `update_engine_version.sh` is used.
io.ProcessResult runUpdateEngineVersion() {
final String executable;
final List<String> args;
if (const LocalPlatform().isWindows) {
executable = 'powershell';
args = <String>[testRoot.binInternalUpdateEngineVersion.path];
// ignore: dead_code
} else if (usePowershellOnPosix) {
executable = 'pwsh';
args = <String>[testRoot.binInternalUpdateEngineVersion.path];
// ignore: dead_code
} else {
executable = testRoot.binInternalUpdateEngineVersion.path;
args = <String>[];
@@ -174,234 +184,123 @@ void main() {
return run(executable, args);
}
void setupRepo({required String branch}) {
for (final File f in <File>[testRoot.deps, testRoot.engineSrcGn]) {
f.createSync(recursive: true);
}
/// Initializes a blank git repo in [testRoot.root].
void initGitRepoWithBlankInitialCommit() {
run('git', <String>['init', '--initial-branch', 'master']);
run('git', <String>['config', '--local', 'user.email', 'test@example.com']);
run('git', <String>['config', '--local', 'user.name', 'Test User']);
run('git', <String>['add', '.']);
run('git', <String>['commit', '-m', 'Initial commit']);
if (branch != 'master') {
run('git', <String>['checkout', '-b', branch]);
}
/// Creates a `bin/internal/engine.version` file in [testRoot].
///
/// If [gitTrack] is `false`, the files are left untracked by git.
void pinEngineVersionForReleaseBranch({required String engineHash, bool gitTrack = true}) {
testRoot.binInternalEngineVersion.writeAsStringSync(engineHash);
if (gitTrack) {
run('git', <String>['add', '-f', 'bin/internal/engine.version']);
run('git', <String>['commit', '-m', 'tracking engine.version']);
}
}
const String engineVersionTrackedContents = 'already existing contents';
void setupTrackedEngineVersion() {
testRoot.binInternalEngineVersion.writeAsStringSync(engineVersionTrackedContents);
run('git', <String>['add', '-f', 'bin/internal/engine.version']);
run('git', <String>['commit', '-m', 'tracking engine.version']);
}
/// Sets up and fetches a [remote] (such as `upstream` or `origin`) for [testRoot.root].
///
/// The remote points at itself (`testRoot.root.path`) for ease of testing.
void setupRemote({required String remote}) {
run('git', <String>['remote', 'add', remote, testRoot.root.path]);
run('git', <String>['fetch', remote]);
}
/// Returns the SHA computed by `merge-base HEAD {{ref}}/master`.
String gitMergeBase({required String ref}) {
final io.ProcessResult mergeBaseHeadOrigin = run('git', <String>[
'merge-base',
'HEAD',
'$ref/master',
]);
return mergeBaseHeadOrigin.stdout as String;
}
group('if FLUTTER_PREBUILT_ENGINE_VERSION is set', () {
setUp(() {
environment['FLUTTER_PREBUILT_ENGINE_VERSION'] = '123abc';
setupRepo(branch: 'master');
initGitRepoWithBlankInitialCommit();
});
test('writes it to engine.version with no git interaction', () async {
test('writes it to cache/engine.stamp with no git interaction', () async {
runUpdateEngineVersion();
expect(testRoot.binInternalEngineVersion, exists);
expect(
testRoot.binInternalEngineVersion.readAsStringSync(),
equalsIgnoringWhitespace('123abc'),
);
expect(testRoot.binCacheEngineStamp.readAsStringSync(), equalsIgnoringWhitespace('123abc'));
});
});
test('writes nothing, even if files are set, if we are on "stable"', () async {
setupRepo(branch: 'stable');
setupTrackedEngineVersion();
setupRemote(remote: 'upstream');
runUpdateEngineVersion();
expect(testRoot.binInternalEngineVersion, exists);
expect(
testRoot.binInternalEngineVersion.readAsStringSync(),
equalsIgnoringWhitespace(engineVersionTrackedContents),
);
expect(
testRoot.binCacheEngineStamp.readAsStringSync(),
equalsIgnoringWhitespace(engineVersionTrackedContents),
);
});
test('writes nothing, even if files are set, if we are on "3.29.0"', () async {
setupRepo(branch: '3.29.0');
setupTrackedEngineVersion();
setupRemote(remote: 'upstream');
runUpdateEngineVersion();
expect(testRoot.binInternalEngineVersion, exists);
expect(
testRoot.binInternalEngineVersion.readAsStringSync(),
equalsIgnoringWhitespace(engineVersionTrackedContents),
);
expect(
testRoot.binCacheEngineStamp.readAsStringSync(),
equalsIgnoringWhitespace(engineVersionTrackedContents),
);
});
test('writes nothing, even if files are set, if we are on "beta"', () async {
setupRepo(branch: 'beta');
setupTrackedEngineVersion();
setupRemote(remote: 'upstream');
runUpdateEngineVersion();
expect(testRoot.binInternalEngineVersion, exists);
expect(
testRoot.binInternalEngineVersion.readAsStringSync(),
equalsIgnoringWhitespace(engineVersionTrackedContents),
);
expect(
testRoot.binCacheEngineStamp.readAsStringSync(),
equalsIgnoringWhitespace(engineVersionTrackedContents),
);
});
group('if DEPS and engine/src/.gn are present, engine.version is derived from', () {
setUp(() async {
setupRepo(branch: 'master');
expect(testRoot.binCacheEngineStamp, _hasFileContentsMatching('123abc'));
});
test('merge-base HEAD upstream/master on non-LUCI when upstream is set', () async {
setupRemote(remote: 'upstream');
final io.ProcessResult mergeBaseHeadUpstream = run('git', <String>[
'merge-base',
'HEAD',
'upstream/master',
]);
test('takes precedence over bin/internal/engine.version, even if set', () async {
pinEngineVersionForReleaseBranch(engineHash: '456def');
runUpdateEngineVersion();
expect(testRoot.binInternalEngineVersion, exists);
expect(
testRoot.binInternalEngineVersion.readAsStringSync(),
equalsIgnoringWhitespace(mergeBaseHeadUpstream.stdout as String),
);
expect(
testRoot.binCacheEngineStamp.readAsStringSync(),
equalsIgnoringWhitespace(mergeBaseHeadUpstream.stdout as String),
);
});
test('merge-base HEAD origin/master on non-LUCI when upstream is not set', () async {
setupRemote(remote: 'origin');
final io.ProcessResult mergeBaseHeadOrigin = run('git', <String>[
'merge-base',
'HEAD',
'origin/master',
]);
runUpdateEngineVersion();
expect(testRoot.binInternalEngineVersion, exists);
expect(
testRoot.binInternalEngineVersion.readAsStringSync(),
equalsIgnoringWhitespace(mergeBaseHeadOrigin.stdout as String),
);
expect(
testRoot.binCacheEngineStamp.readAsStringSync(),
equalsIgnoringWhitespace(mergeBaseHeadOrigin.stdout as String),
);
});
test('rev-parse HEAD when running on LUCI', () async {
environment['LUCI_CONTEXT'] = '_NON_NULL_AND_NON_EMPTY_STRING';
runUpdateEngineVersion();
final io.ProcessResult revParseHead = run('git', <String>['rev-parse', 'HEAD']);
expect(testRoot.binInternalEngineVersion, exists);
expect(
testRoot.binInternalEngineVersion.readAsStringSync(),
equalsIgnoringWhitespace(revParseHead.stdout as String),
);
expect(
testRoot.binCacheEngineStamp.readAsStringSync(),
equalsIgnoringWhitespace(revParseHead.stdout as String),
);
expect(testRoot.binCacheEngineStamp, _hasFileContentsMatching('123abc'));
});
});
group('if DEPS or engine/src/.gn are omitted', () {
group('if bin/internal/engine.version is set', () {
setUp(() {
for (final File f in <File>[testRoot.deps, testRoot.engineSrcGn]) {
f.createSync(recursive: true);
}
setupRepo(branch: 'master');
initGitRepoWithBlankInitialCommit();
});
test('and tracked it is used', () async {
setupRemote(remote: 'upstream');
pinEngineVersionForReleaseBranch(engineHash: 'abc123');
runUpdateEngineVersion();
expect(testRoot.binCacheEngineStamp, _hasFileContentsMatching('abc123'));
});
test('but not tracked, it is ignored', () async {
setupRemote(remote: 'upstream');
pinEngineVersionForReleaseBranch(engineHash: 'abc123', gitTrack: false);
runUpdateEngineVersion();
expect(testRoot.binCacheEngineStamp, _hasFileContentsMatching(gitMergeBase(ref: 'upstream')));
});
});
group('resolves engine artifacts with git merge-base', () {
setUp(() {
initGitRepoWithBlankInitialCommit();
});
test('default to upstream/master if available', () async {
setupRemote(remote: 'upstream');
runUpdateEngineVersion();
expect(testRoot.binCacheEngineStamp, _hasFileContentsMatching(gitMergeBase(ref: 'upstream')));
});
test('fallsback to origin/master', () async {
setupRemote(remote: 'origin');
});
test('[DEPS] engine.version is blank', () async {
testRoot.deps.deleteSync();
runUpdateEngineVersion();
expect(testRoot.binInternalEngineVersion, exists);
expect(testRoot.binInternalEngineVersion.readAsStringSync(), equalsIgnoringWhitespace(''));
expect(testRoot.binCacheEngineStamp.readAsStringSync(), equalsIgnoringWhitespace(''));
});
test('[engine/src/.gn] engine.version is blank', () async {
testRoot.engineSrcGn.deleteSync();
runUpdateEngineVersion();
expect(testRoot.binInternalEngineVersion, exists);
expect(testRoot.binInternalEngineVersion.readAsStringSync(), equalsIgnoringWhitespace(''));
expect(testRoot.binCacheEngineStamp.readAsStringSync(), equalsIgnoringWhitespace(''));
expect(testRoot.binCacheEngineStamp, _hasFileContentsMatching(gitMergeBase(ref: 'origin')));
});
});
group('engine.realm', () {
setUp(() {
for (final File f in <File>[testRoot.deps, testRoot.engineSrcGn]) {
f.createSync(recursive: true);
}
setupRepo(branch: 'master');
setupRemote(remote: 'origin');
initGitRepoWithBlankInitialCommit();
environment['FLUTTER_PREBUILT_ENGINE_VERSION'] = '123abc';
});
test('is empty if the FLUTTER_REALM environment variable is not set', () {
expect(environment, isNot(contains('FLUTTER_REALM')));
test('is empty by default', () async {
runUpdateEngineVersion();
expect(testRoot.binCacheEngineRealm, exists);
expect(testRoot.binCacheEngineRealm.readAsStringSync(), equalsIgnoringWhitespace(''));
expect(testRoot.binInternalEngineRealm, exists);
expect(testRoot.binInternalEngineRealm.readAsStringSync(), equalsIgnoringWhitespace(''));
expect(testRoot.binCacheEngineRealm, _hasFileContentsMatching(''));
});
test('contains the FLUTTER_REALM environment variable', () async {
test('is the value in FLUTTER_REALM if set', () async {
environment['FLUTTER_REALM'] = 'flutter_archives_v2';
runUpdateEngineVersion();
expect(testRoot.binCacheEngineRealm, exists);
expect(
testRoot.binCacheEngineRealm.readAsStringSync(),
equalsIgnoringWhitespace('flutter_archives_v2'),
);
expect(testRoot.binInternalEngineRealm, exists);
expect(
testRoot.binInternalEngineRealm.readAsStringSync(),
equalsIgnoringWhitespace('flutter_archives_v2'),
);
expect(testRoot.binCacheEngineRealm, _hasFileContentsMatching('flutter_archives_v2'));
});
});
}
@@ -414,13 +313,7 @@ void main() {
/// ```txt
/// ├── bin
/// │ ├── internal
/// │ │ ├── engine.version
/// │ │ ├── engine.realm
/// │ │ └── update_engine_version.{sh|ps1}
/// │ └── engine
/// │ └── src
/// │ └── .gn
/// └── DEPS
/// ```
final class _FlutterRootUnderTest {
/// Creates a root-under test using [path] as the root directory.
@@ -435,15 +328,10 @@ final class _FlutterRootUnderTest {
final Directory root = fileSystem.directory(path);
return _FlutterRootUnderTest._(
root,
deps: root.childFile('DEPS'),
engineSrcGn: root.childFile(fileSystem.path.join('engine', 'src', '.gn')),
binInternalEngineVersion: root.childFile(
fileSystem.path.join('bin', 'internal', 'engine.version'),
),
binCacheEngineRealm: root.childFile(fileSystem.path.join('bin', 'cache', 'engine.realm')),
binInternalEngineRealm: root.childFile(
fileSystem.path.join('bin', 'internal', 'engine.realm'),
),
binCacheEngineStamp: root.childFile(fileSystem.path.join('bin', 'cache', 'engine.stamp')),
binInternalUpdateEngineVersion: root.childFile(
fileSystem.path.join(
@@ -473,55 +361,30 @@ final class _FlutterRootUnderTest {
const _FlutterRootUnderTest._(
this.root, {
required this.deps,
required this.engineSrcGn,
required this.binCacheEngineStamp,
required this.binInternalEngineVersion,
required this.binCacheEngineRealm,
required this.binInternalEngineRealm,
required this.binInternalUpdateEngineVersion,
});
final Directory root;
/// `DEPS`.
///
/// The presenence of this file is an indicator we are in a fused (mono) repo.
final File deps;
/// `engine/src/.gn`.
///
/// The presenence of this file is an indicator we are in a fused (mono) repo.
final File engineSrcGn;
/// `bin/internal/engine.version`.
///
/// This file contains a SHA of which engine binaries to download.
/// This file contains a pinned SHA of which engine binaries to download.
///
/// Currently, the SHA is either _computed_ or _pre-determined_, based on if
/// the file is checked-in and tracked. That behavior is changing, and in the
/// future this will be a checked-in file and not computed.
///
/// See also: https://github.com/flutter/flutter/issues/164315.
final File binInternalEngineVersion; // TODO(matanlurey): Update these docs.
/// If omitted, the file is ignored.
final File binInternalEngineVersion;
/// `bin/cache/engine.stamp`.
///
/// This file contains a _computed_ SHA of which engine binaries to download.
final File binCacheEngineStamp;
/// `bin/internal/engine.realm`.
///
/// If non-empty, the value comes from the environment variable `FLUTTER_REALM`,
/// which instructs the tool where the SHA stored in [binInternalEngineVersion]
/// should be fetched from (it differs for presubmits run for flutter/flutter
/// and builds downloaded by end-users or by postsubmits).
final File binInternalEngineRealm;
/// `bin/cache/engine.realm`.
///
/// If non-empty, the value comes from the environment variable `FLUTTER_REALM`,
/// which instructs the tool where the SHA stored in [binInternalEngineVersion]
/// which instructs the tool where the SHA stored in [binCacheEngineStamp]
/// should be fetched from (it differs for presubmits run for flutter/flutter
/// and builds downloaded by end-users or by postsubmits).
final File binCacheEngineRealm;
@@ -540,3 +403,51 @@ extension on File {
copySync(newPath);
}
}
/// Returns a matcher, that, given [contents]:
///
/// 1. Asserts the 'actual' entity is a [File];
/// 2. Asserts that the file exists;
/// 3. Asserts that the file's contents, after applying [collapseWhitespace], is the same as
/// [contents], after applying [collapseWhitespace].
///
/// This replaces multiple other matchers, and still provides a high-quality error message
/// when it fails.
Matcher _hasFileContentsMatching(String contents) {
return _ExistsWithStringContentsIgnoringWhitespace(contents);
}
final class _ExistsWithStringContentsIgnoringWhitespace extends Matcher {
_ExistsWithStringContentsIgnoringWhitespace(String contents)
: _expected = collapseWhitespace(contents);
final String _expected;
@override
bool matches(Object? item, _) {
if (item is! File || !item.existsSync()) {
return false;
}
final String actual = item.readAsStringSync();
return collapseWhitespace(actual) == collapseWhitespace(_expected);
}
@override
Description describe(Description description) {
return description.add('a file exists that matches (ignoring whitespace): $_expected');
}
@override
Description describeMismatch(Object? item, Description mismatch, _, _) {
if (item is! File) {
return mismatch.add('is not a file (${item.runtimeType})');
}
if (!item.existsSync()) {
return mismatch.add('does not exist');
}
return mismatch
.add('is ')
.addDescriptionOf(collapseWhitespace(item.readAsStringSync()))
.add(' with whitespace compressed');
}
}