From 2aa40e06b79b10aa66097f922564b5c51a9cad21 Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Sat, 26 Aug 2023 20:26:52 -0700 Subject: [PATCH 01/16] Write a minimal engine_tools_lib to use for local-repo Dart tooling. --- tools/engine_tools_lib/README.md | 17 ++ .../lib/engine_tools_lib.dart | 183 ++++++++++++++++++ tools/engine_tools_lib/pubspec.yaml | 41 ++++ .../test/engine_tools_lib_test.dart | 181 +++++++++++++++++ 4 files changed, 422 insertions(+) create mode 100644 tools/engine_tools_lib/README.md create mode 100644 tools/engine_tools_lib/lib/engine_tools_lib.dart create mode 100644 tools/engine_tools_lib/pubspec.yaml create mode 100644 tools/engine_tools_lib/test/engine_tools_lib_test.dart diff --git a/tools/engine_tools_lib/README.md b/tools/engine_tools_lib/README.md new file mode 100644 index 0000000000000..ae46336ed40f4 --- /dev/null +++ b/tools/engine_tools_lib/README.md @@ -0,0 +1,17 @@ +# engine_tools_lib + +This is a repo-internal library for `flutter/engine`, that contains shared code +for writing tools that operate on the engine repository. For example, finding +the latest compiled engine artifacts in the `out/` directory: + +```dart +import 'package:engine_tools_lib/engine_tools_lib.dart'; + +void main() { + final engine = Engine.findWithin(); + final latest = engine.latestOutput(); + if (latest != null) { + print('Latest compile_commands.json: ${latest.compileCommandsJson?.path}'); + } +} +``` diff --git a/tools/engine_tools_lib/lib/engine_tools_lib.dart b/tools/engine_tools_lib/lib/engine_tools_lib.dart new file mode 100644 index 0000000000000..bdacac1903905 --- /dev/null +++ b/tools/engine_tools_lib/lib/engine_tools_lib.dart @@ -0,0 +1,183 @@ +// Copyright 2013 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. + +/// A minimal library for discovering and probing a local engine repository. +/// +/// This library is intended to be used by tools that need to interact with a +/// local engine repository, such as `clang_tidy` or `githooks`. For example, +/// finding the `compile_commands.json` file for the most recently built output: +/// +/// ```dart +/// final Engine engine = Engine.findWithin(); +/// final Output? output = engine.latestOutput(); +/// if (output == null) { +/// print('No output targets found.'); +/// } else { +/// final io.File? compileCommandsJson = output.compileCommandsJson; +/// if (compileCommandsJson == null) { +/// print('No compile_commands.json file found.'); +/// } else { +/// print('Found compile_commands.json file at ${compileCommandsJson.path}'); +/// } +/// } +library; + +import 'dart:io' as io; + +import 'package:path/path.dart' as p; + +/// Represents the `$ENGINE` directory (i.e. a checked-out Flutter engine). +/// +/// If you have a path to the `$ENGINE/src` directory, use [Engine.fromSrcPath]. +/// +/// If you have a path to a directory within the `$ENGINE/src` directory, or +/// want to use the current working directory, use [Engine.findWithin]. +final class Engine { + /// Creates an [Engine] from a path such as `/Users/.../flutter/engine/src`. + /// + /// ```dart + /// final Engine engine = Engine.findWithin('/Users/.../engine/src'); + /// print(engine.srcDir.path); // /Users/.../engine/src + /// ``` + /// + /// Throws an [ArgumentError] if the path does not point to a valid engine. + factory Engine.fromSrcPath(String srcPath) { + // If the path does not end in `/src`, fail. + if (p.basename(srcPath) != 'src') { + throw ArgumentError( + 'The path $srcPath does not end in `${p.separator}src`.\n' + '\n$_kHintFindWithinPath' + ); + } + + // If the directory does not exist, or is not a directory, fail. + final io.Directory srcDir = io.Directory(srcPath); + if (!srcDir.existsSync()) { + throw ArgumentError( + 'The path "$srcPath" does not exist or is not a directory.\n' + '\n$_kHintFindWithinPath' + ); + } + + // Check for the existence of a `flutter` directory within `src`. + final io.Directory flutterDir = io.Directory(p.join(srcPath, 'flutter')); + if (!flutterDir.existsSync()) { + throw ArgumentError( + 'The path "$srcPath" does not contain a "flutter" directory.' + ); + } + + // Check for the existence of a `out` directory within `src`. + final io.Directory outDir = io.Directory(p.join(srcPath, 'out')); + if (!outDir.existsSync()) { + throw ArgumentError( + 'The path "$srcPath" does not contain an "out" directory.' + ); + } + + return Engine._(srcDir, flutterDir, outDir); + } + + /// Creates an [Engine] by looking for a `src/` directory in the given path. + /// + /// ```dart + /// // Use the current working directory. + /// final Engine engine = Engine.findWithin(); + /// print(engine.srcDir.path); // /Users/.../engine/src + /// + /// // Use a specific directory. + /// final Engine engine = Engine.findWithin('/Users/.../engine/src/foo/bar/baz'); + /// print(engine.srcDir.path); // /Users/.../engine/src + /// ``` + /// + /// If a path is not provided, the current working directory is used. + /// + /// Throws an [ArgumentError] if the path is not within a valid engine. + factory Engine.findWithin([String? path]) { + path ??= p.current; + + // Search parent directories for a `src` directory. + io.Directory maybeSrcDir = io.Directory(path); + + if (!maybeSrcDir.existsSync()) { + throw ArgumentError( + 'The path "$path" does not exist or is not a directory.' + ); + } + + do { + if (p.basename(maybeSrcDir.path) == 'src') { + return Engine.fromSrcPath(maybeSrcDir.path); + } + maybeSrcDir = maybeSrcDir.parent; + } while (maybeSrcDir.parent.path != maybeSrcDir.path /* at root */); + + throw ArgumentError( + 'The path "$path" does not contain a "src" directory.' + ); + } + + const Engine._( + this.srcDir, + this.flutterDir, + this.outDir, + ); + + static final String _kHintFindWithinPath = + 'This constructor is intended to be used only a path that points to ' + 'a Flutter engine source directory, i.e. a directory that ends in ' + '${p.separator}src. Use "findWithin" to create an Engine from a ' + 'directory that lives within an engine source directory'; + + /// The path to the `$ENGINE/src` directory. + final io.Directory srcDir; + + /// The path to the `$ENGINE/src/flutter` directory. + final io.Directory flutterDir; + + /// The path to the `$ENGINE/src/out` directory. + final io.Directory outDir; + + /// Returns a list of all output targets in [outDir]. + List outputs() { + return outDir + .listSync() + .whereType() + .map(Output._) + .toList(); + } + + /// Returns the most recently modified output target in [outDir]. + /// + /// If there are no output targets, returns `null`. + Output? latestOutput() { + final List outputs = this.outputs(); + if (outputs.isEmpty) { + return null; + } + outputs.sort((Output a, Output b) { + return b.dir.statSync().modified.compareTo(a.dir.statSync().modified); + }); + return outputs.first; + } +} + +/// Represents a single output target in the `$ENGINE/src/out` directory. +final class Output { + const Output._(this.dir); + + /// The directory containing the output target. + final io.Directory dir; + + /// The `compile_commands.json` file for this output target. + /// + /// Returns `null` if the file does not exist. + io.File? get compileCommandsJson { + final io.File file = io.File(p.join(dir.path, 'compile_commands.json')); + if (!file.existsSync()) { + return null; + } + return file; + } +} diff --git a/tools/engine_tools_lib/pubspec.yaml b/tools/engine_tools_lib/pubspec.yaml new file mode 100644 index 0000000000000..582e999d451a1 --- /dev/null +++ b/tools/engine_tools_lib/pubspec.yaml @@ -0,0 +1,41 @@ +# Copyright 2013 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. + +name: engine_tools_lib +publish_to: none +environment: + sdk: ^3.0.0 + +# Do not add any dependencies that require more than what is provided in +# //third_party/pkg, //third_party/dart/pkg, or +# //third_party/dart/third_party/pkg. In particular, package:test is not usable +# here. + +# If you do add packages here, make sure you can run `pub get --offline`, and +# check the .packages and .package_config to make sure all the paths are +# relative to this directory into //third_party/dart + +dependencies: + meta: any + path: any + +dev_dependencies: + async_helper: any + expect: any + litetest: any + smith: any + +dependency_overrides: + async_helper: + path: ../../../third_party/dart/pkg/async_helper + expect: + path: ../../../third_party/dart/pkg/expect + litetest: + path: ../../testing/litetest + meta: + path: ../../../third_party/dart/pkg/meta + path: + path: ../../../third_party/dart/third_party/pkg/path + smith: + path: ../../../third_party/dart/pkg/smith diff --git a/tools/engine_tools_lib/test/engine_tools_lib_test.dart b/tools/engine_tools_lib/test/engine_tools_lib_test.dart new file mode 100644 index 0000000000000..327475b7dc29b --- /dev/null +++ b/tools/engine_tools_lib/test/engine_tools_lib_test.dart @@ -0,0 +1,181 @@ +// Copyright 2013 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:engine_tools_lib/engine_tools_lib.dart'; +import 'package:litetest/litetest.dart'; +import 'package:path/path.dart' as p; + +void main() { + late io.Directory emptyDir; + + void setUp() { + emptyDir = io.Directory.systemTemp.createTempSync('engine_tools_lib.test'); + } + + void tearDown() { + emptyDir.deleteSync(recursive: true); + } + + group('Engine.fromSrcPath', () { + group('should fail when', () { + test('the path does not end in `${p.separator}src`', () { + setUp(); + try { + expect( + () => Engine.fromSrcPath(emptyDir.path), + throwsArgumentError, + ); + } finally { + tearDown(); + } + }); + + test('the path does not exist', () { + setUp(); + try { + expect( + () => Engine.fromSrcPath(p.join(emptyDir.path, 'src')), + throwsArgumentError, + ); + } finally { + tearDown(); + } + }); + + test('the path does not contain a "flutter" directory', () { + setUp(); + try { + final io.Directory srcDir = io.Directory(p.join(emptyDir.path, 'src'))..createSync(); + expect( + () => Engine.fromSrcPath(srcDir.path), + throwsArgumentError, + ); + } finally { + tearDown(); + } + }); + + test('the path does not contain an "out" directory', () { + setUp(); + try { + final io.Directory srcDir = io.Directory(p.join(emptyDir.path, 'src'))..createSync(); + io.Directory(p.join(srcDir.path, 'flutter')).createSync(); + expect( + () => Engine.fromSrcPath(srcDir.path), + throwsArgumentError, + ); + } finally { + tearDown(); + } + }); + + test('returns an Engine', () { + setUp(); + try { + final io.Directory srcDir = io.Directory(p.join(emptyDir.path, 'src'))..createSync(); + io.Directory(p.join(srcDir.path, 'flutter')).createSync(); + io.Directory(p.join(srcDir.path, 'out')).createSync(); + + final Engine engine = Engine.fromSrcPath(srcDir.path); + + expect(engine.srcDir.path, srcDir.path); + expect(engine.flutterDir.path, p.join(srcDir.path, 'flutter')); + expect(engine.outDir.path, p.join(srcDir.path, 'out')); + } finally { + tearDown(); + } + }); + }); + }); + + group('Engine.findWithin', () { + late io.Directory emptyDir; + + void setUp() { + emptyDir = io.Directory.systemTemp.createTempSync('engine_tools_lib.test'); + } + + void tearDown() { + emptyDir.deleteSync(recursive: true); + } + + group('should fail when', () { + test('the path does not contain a "src" directory', () { + setUp(); + try { + expect( + () => Engine.findWithin(emptyDir.path), + throwsArgumentError, + ); + } finally { + tearDown(); + } + }); + + test('returns an Engine', () { + setUp(); + try { + final io.Directory srcDir = io.Directory(p.join(emptyDir.path, 'src'))..createSync(); + io.Directory(p.join(srcDir.path, 'flutter')).createSync(); + io.Directory(p.join(srcDir.path, 'out')).createSync(); + + final Engine engine = Engine.findWithin(srcDir.path); + + expect(engine.srcDir.path, srcDir.path); + expect(engine.flutterDir.path, p.join(srcDir.path, 'flutter')); + expect(engine.outDir.path, p.join(srcDir.path, 'out')); + } finally { + tearDown(); + } + }); + }); + }); + + test('outputs a list of targets', () { + setUp(); + + try { + // Create a valid engine. + io.Directory(p.join(emptyDir.path, 'src', 'flutter')).createSync(recursive: true); + io.Directory(p.join(emptyDir.path, 'src', 'out')).createSync(recursive: true); + + // Create two targets in out: host_debug and host_debug_unopt_arm64. + io.Directory(p.join(emptyDir.path, 'src', 'out', 'host_debug')).createSync(recursive: true); + io.Directory(p.join(emptyDir.path, 'src', 'out', 'host_debug_unopt_arm64')).createSync(recursive: true); + + final Engine engine = Engine.fromSrcPath(p.join(emptyDir.path, 'src')); + final List outputs = engine.outputs().map((Output o) => p.basename(o.dir.path)).toList()..sort(); + expect(outputs, [ + 'host_debug', + 'host_debug_unopt_arm64', + ]); + } finally { + tearDown(); + } + }); + + test('outputs the latest target and compile_commands.json', () { + setUp(); + + try { + // Create a valid engine. + io.Directory(p.join(emptyDir.path, 'src', 'flutter')).createSync(recursive: true); + io.Directory(p.join(emptyDir.path, 'src', 'out')).createSync(recursive: true); + + // Create two targets in out: host_debug and host_debug_unopt_arm64. + io.Directory(p.join(emptyDir.path, 'src', 'out', 'host_debug')).createSync(recursive: true); + io.Directory(p.join(emptyDir.path, 'src', 'out', 'host_debug_unopt_arm64')).createSync(recursive: true); + io.File(p.join(emptyDir.path, 'src', 'out', 'host_debug_unopt_arm64', 'compile_commands.json')).createSync(); + + final Engine engine = Engine.fromSrcPath(p.join(emptyDir.path, 'src')); + final Output? latestOutput = engine.latestOutput(); + expect(latestOutput, isNotNull); + expect(p.basename(latestOutput!.dir.path), 'host_debug_unopt_arm64'); + expect(latestOutput.compileCommandsJson, isNotNull); + } finally { + tearDown(); + } + }); +} From 67b18ed29abdd53f9629e0eb7641f6e732691e36 Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Sat, 26 Aug 2023 20:27:57 -0700 Subject: [PATCH 02/16] Whitespace. --- .../lib/engine_tools_lib.dart | 22 +++++++++---------- tools/engine_tools_lib/pubspec.yaml | 2 +- .../test/engine_tools_lib_test.dart | 4 ++-- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/tools/engine_tools_lib/lib/engine_tools_lib.dart b/tools/engine_tools_lib/lib/engine_tools_lib.dart index bdacac1903905..aabb927d61eb3 100644 --- a/tools/engine_tools_lib/lib/engine_tools_lib.dart +++ b/tools/engine_tools_lib/lib/engine_tools_lib.dart @@ -3,11 +3,11 @@ // found in the LICENSE file. /// A minimal library for discovering and probing a local engine repository. -/// +/// /// This library is intended to be used by tools that need to interact with a /// local engine repository, such as `clang_tidy` or `githooks`. For example, /// finding the `compile_commands.json` file for the most recently built output: -/// +/// /// ```dart /// final Engine engine = Engine.findWithin(); /// final Output? output = engine.latestOutput(); @@ -28,19 +28,19 @@ import 'dart:io' as io; import 'package:path/path.dart' as p; /// Represents the `$ENGINE` directory (i.e. a checked-out Flutter engine). -/// +/// /// If you have a path to the `$ENGINE/src` directory, use [Engine.fromSrcPath]. /// /// If you have a path to a directory within the `$ENGINE/src` directory, or /// want to use the current working directory, use [Engine.findWithin]. final class Engine { /// Creates an [Engine] from a path such as `/Users/.../flutter/engine/src`. - /// + /// /// ```dart /// final Engine engine = Engine.findWithin('/Users/.../engine/src'); /// print(engine.srcDir.path); // /Users/.../engine/src /// ``` - /// + /// /// Throws an [ArgumentError] if the path does not point to a valid engine. factory Engine.fromSrcPath(String srcPath) { // If the path does not end in `/src`, fail. @@ -80,19 +80,19 @@ final class Engine { } /// Creates an [Engine] by looking for a `src/` directory in the given path. - /// + /// /// ```dart /// // Use the current working directory. /// final Engine engine = Engine.findWithin(); /// print(engine.srcDir.path); // /Users/.../engine/src - /// + /// /// // Use a specific directory. /// final Engine engine = Engine.findWithin('/Users/.../engine/src/foo/bar/baz'); /// print(engine.srcDir.path); // /Users/.../engine/src /// ``` - /// + /// /// If a path is not provided, the current working directory is used. - /// + /// /// Throws an [ArgumentError] if the path is not within a valid engine. factory Engine.findWithin([String? path]) { path ??= p.current; @@ -149,7 +149,7 @@ final class Engine { } /// Returns the most recently modified output target in [outDir]. - /// + /// /// If there are no output targets, returns `null`. Output? latestOutput() { final List outputs = this.outputs(); @@ -171,7 +171,7 @@ final class Output { final io.Directory dir; /// The `compile_commands.json` file for this output target. - /// + /// /// Returns `null` if the file does not exist. io.File? get compileCommandsJson { final io.File file = io.File(p.join(dir.path, 'compile_commands.json')); diff --git a/tools/engine_tools_lib/pubspec.yaml b/tools/engine_tools_lib/pubspec.yaml index 582e999d451a1..5745550f00552 100644 --- a/tools/engine_tools_lib/pubspec.yaml +++ b/tools/engine_tools_lib/pubspec.yaml @@ -4,7 +4,7 @@ name: engine_tools_lib publish_to: none -environment: +environment: sdk: ^3.0.0 # Do not add any dependencies that require more than what is provided in diff --git a/tools/engine_tools_lib/test/engine_tools_lib_test.dart b/tools/engine_tools_lib/test/engine_tools_lib_test.dart index 327475b7dc29b..2382b43519349 100644 --- a/tools/engine_tools_lib/test/engine_tools_lib_test.dart +++ b/tools/engine_tools_lib/test/engine_tools_lib_test.dart @@ -9,7 +9,7 @@ import 'package:path/path.dart' as p; void main() { late io.Directory emptyDir; - + void setUp() { emptyDir = io.Directory.systemTemp.createTempSync('engine_tools_lib.test'); } @@ -92,7 +92,7 @@ void main() { group('Engine.findWithin', () { late io.Directory emptyDir; - + void setUp() { emptyDir = io.Directory.systemTemp.createTempSync('engine_tools_lib.test'); } From 80e9fe38f120c545c33a22370ebe56252c413382 Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Sat, 26 Aug 2023 20:28:18 -0700 Subject: [PATCH 03/16] More whitespace. --- tools/engine_tools_lib/lib/engine_tools_lib.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/engine_tools_lib/lib/engine_tools_lib.dart b/tools/engine_tools_lib/lib/engine_tools_lib.dart index aabb927d61eb3..8ae9cecb2cef4 100644 --- a/tools/engine_tools_lib/lib/engine_tools_lib.dart +++ b/tools/engine_tools_lib/lib/engine_tools_lib.dart @@ -96,7 +96,7 @@ final class Engine { /// Throws an [ArgumentError] if the path is not within a valid engine. factory Engine.findWithin([String? path]) { path ??= p.current; - + // Search parent directories for a `src` directory. io.Directory maybeSrcDir = io.Directory(path); From 928475bdb5d049795ad6cd9c7360e013cf034ad5 Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Sat, 26 Aug 2023 20:32:22 -0700 Subject: [PATCH 04/16] Add to run_tests.py. --- testing/run_tests.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/testing/run_tests.py b/testing/run_tests.py index 7c8842af18927..df8b6e4186bca 100755 --- a/testing/run_tests.py +++ b/testing/run_tests.py @@ -952,6 +952,20 @@ def gather_clang_tidy_tests(build_dir): cwd=test_dir ) +def gather_engine_tools_lib_tests(build_dir): + test_dir = os.path.join(BUILDROOT_DIR, 'flutter', 'tools', 'engine_tools', 'lib') + dart_tests = glob.glob('%s/*_test.dart' % test_dir) + for dart_test_file in dart_tests: + opts = [ + '--disable-dart-dev', dart_test_file, + ] + yield EngineExecutableTask( + build_dir, + os.path.join('dart-sdk', 'bin', 'dart'), + None, + flags=opts, + cwd=test_dir + ) def gather_api_consistency_tests(build_dir): test_dir = os.path.join(BUILDROOT_DIR, 'flutter', 'tools', 'api_check') @@ -1230,6 +1244,7 @@ def main(): tasks += list(gather_litetest_tests(build_dir)) tasks += list(gather_githooks_tests(build_dir)) tasks += list(gather_clang_tidy_tests(build_dir)) + tasks += list(gather_engine_tools_lib_tests(build_dir)) tasks += list(gather_api_consistency_tests(build_dir)) tasks += list(gather_path_ops_tests(build_dir)) tasks += list(gather_const_finder_tests(build_dir)) From 75072818dc0f83b935de80db7811695afdc42d27 Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Sat, 26 Aug 2023 20:33:30 -0700 Subject: [PATCH 05/16] Reformat. --- testing/run_tests.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/testing/run_tests.py b/testing/run_tests.py index df8b6e4186bca..2157defe942e4 100755 --- a/testing/run_tests.py +++ b/testing/run_tests.py @@ -953,11 +953,14 @@ def gather_clang_tidy_tests(build_dir): ) def gather_engine_tools_lib_tests(build_dir): - test_dir = os.path.join(BUILDROOT_DIR, 'flutter', 'tools', 'engine_tools', 'lib') + test_dir = os.path.join( + BUILDROOT_DIR, 'flutter', 'tools', 'engine_tools_lib' + ) dart_tests = glob.glob('%s/*_test.dart' % test_dir) for dart_test_file in dart_tests: opts = [ - '--disable-dart-dev', dart_test_file, + '--disable-dart-dev', + dart_test_file, ] yield EngineExecutableTask( build_dir, From 11f47e9d555c78e3dfe41004d580cc4502fe9989 Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Sat, 26 Aug 2023 20:33:51 -0700 Subject: [PATCH 06/16] Reformat. --- testing/run_tests.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/testing/run_tests.py b/testing/run_tests.py index 2157defe942e4..6371ebd3ef40b 100755 --- a/testing/run_tests.py +++ b/testing/run_tests.py @@ -952,6 +952,7 @@ def gather_clang_tidy_tests(build_dir): cwd=test_dir ) + def gather_engine_tools_lib_tests(build_dir): test_dir = os.path.join( BUILDROOT_DIR, 'flutter', 'tools', 'engine_tools_lib' @@ -970,6 +971,7 @@ def gather_engine_tools_lib_tests(build_dir): cwd=test_dir ) + def gather_api_consistency_tests(build_dir): test_dir = os.path.join(BUILDROOT_DIR, 'flutter', 'tools', 'api_check') dart_tests = glob.glob('%s/test/*_test.dart' % test_dir) From ac8032851d4b2a209ebcbebe831a9f779613b540 Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Sat, 26 Aug 2023 20:34:10 -0700 Subject: [PATCH 07/16] Reformat. --- testing/run_tests.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/testing/run_tests.py b/testing/run_tests.py index 6371ebd3ef40b..cb51f5433d974 100755 --- a/testing/run_tests.py +++ b/testing/run_tests.py @@ -954,9 +954,7 @@ def gather_clang_tidy_tests(build_dir): def gather_engine_tools_lib_tests(build_dir): - test_dir = os.path.join( - BUILDROOT_DIR, 'flutter', 'tools', 'engine_tools_lib' - ) + test_dir = os.path.join(BUILDROOT_DIR, 'flutter', 'tools', 'engine_tools_lib') dart_tests = glob.glob('%s/*_test.dart' % test_dir) for dart_test_file in dart_tests: opts = [ From 16603758eed476bb118d1b7843c7a7c16be18b0d Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Sat, 26 Aug 2023 20:36:09 -0700 Subject: [PATCH 08/16] Add missing quotes. --- tools/engine_tools_lib/lib/engine_tools_lib.dart | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/engine_tools_lib/lib/engine_tools_lib.dart b/tools/engine_tools_lib/lib/engine_tools_lib.dart index 8ae9cecb2cef4..69953e73b1e95 100644 --- a/tools/engine_tools_lib/lib/engine_tools_lib.dart +++ b/tools/engine_tools_lib/lib/engine_tools_lib.dart @@ -21,6 +21,7 @@ /// print('Found compile_commands.json file at ${compileCommandsJson.path}'); /// } /// } +/// ``` library; import 'dart:io' as io; From 4c5d5514670c0b77f80aa244404a756350e79995 Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Sun, 27 Aug 2023 13:23:24 -0700 Subject: [PATCH 09/16] Fix pub get. --- tools/pub_get_offline.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/pub_get_offline.py b/tools/pub_get_offline.py index 36790b281f57f..ade4e8b346f94 100644 --- a/tools/pub_get_offline.py +++ b/tools/pub_get_offline.py @@ -35,6 +35,7 @@ os.path.join(ENGINE_DIR, "tools", "android_lint"), os.path.join(ENGINE_DIR, "tools", "clang_tidy"), os.path.join(ENGINE_DIR, "tools", "const_finder"), + os.path.join(ENGINE_DIR, "tools", "engine_tools_lib"), os.path.join(ENGINE_DIR, "tools", "githooks"), os.path.join(ENGINE_DIR, "tools", "licenses"), os.path.join(ENGINE_DIR, "tools", "path_ops", "dart") From e0078f95f3fcb08c74489366598063823a247507 Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Mon, 28 Aug 2023 10:59:36 -0700 Subject: [PATCH 10/16] Move engine_tools_lib, fix a flaky test. --- testing/run_tests.py | 2 +- tools/{ => pkg}/engine_tools_lib/README.md | 0 .../engine_tools_lib/lib/engine_tools_lib.dart | 0 tools/{ => pkg}/engine_tools_lib/pubspec.yaml | 12 ++++++------ .../engine_tools_lib/test/engine_tools_lib_test.dart | 5 +++++ tools/pub_get_offline.py | 2 +- 6 files changed, 13 insertions(+), 8 deletions(-) rename tools/{ => pkg}/engine_tools_lib/README.md (100%) rename tools/{ => pkg}/engine_tools_lib/lib/engine_tools_lib.dart (100%) rename tools/{ => pkg}/engine_tools_lib/pubspec.yaml (74%) rename tools/{ => pkg}/engine_tools_lib/test/engine_tools_lib_test.dart (94%) diff --git a/testing/run_tests.py b/testing/run_tests.py index cb51f5433d974..596c549661923 100755 --- a/testing/run_tests.py +++ b/testing/run_tests.py @@ -954,7 +954,7 @@ def gather_clang_tidy_tests(build_dir): def gather_engine_tools_lib_tests(build_dir): - test_dir = os.path.join(BUILDROOT_DIR, 'flutter', 'tools', 'engine_tools_lib') + test_dir = os.path.join(BUILDROOT_DIR, 'flutter', 'tools', 'pkg', 'engine_tools_lib') dart_tests = glob.glob('%s/*_test.dart' % test_dir) for dart_test_file in dart_tests: opts = [ diff --git a/tools/engine_tools_lib/README.md b/tools/pkg/engine_tools_lib/README.md similarity index 100% rename from tools/engine_tools_lib/README.md rename to tools/pkg/engine_tools_lib/README.md diff --git a/tools/engine_tools_lib/lib/engine_tools_lib.dart b/tools/pkg/engine_tools_lib/lib/engine_tools_lib.dart similarity index 100% rename from tools/engine_tools_lib/lib/engine_tools_lib.dart rename to tools/pkg/engine_tools_lib/lib/engine_tools_lib.dart diff --git a/tools/engine_tools_lib/pubspec.yaml b/tools/pkg/engine_tools_lib/pubspec.yaml similarity index 74% rename from tools/engine_tools_lib/pubspec.yaml rename to tools/pkg/engine_tools_lib/pubspec.yaml index 5745550f00552..9b6c49a5413c2 100644 --- a/tools/engine_tools_lib/pubspec.yaml +++ b/tools/pkg/engine_tools_lib/pubspec.yaml @@ -28,14 +28,14 @@ dev_dependencies: dependency_overrides: async_helper: - path: ../../../third_party/dart/pkg/async_helper + path: ../../../../third_party/dart/pkg/async_helper expect: - path: ../../../third_party/dart/pkg/expect + path: ../../../../third_party/dart/pkg/expect litetest: - path: ../../testing/litetest + path: ../../../testing/litetest meta: - path: ../../../third_party/dart/pkg/meta + path: ../../../../third_party/dart/pkg/meta path: - path: ../../../third_party/dart/third_party/pkg/path + path: ../../../../third_party/dart/third_party/pkg/path smith: - path: ../../../third_party/dart/pkg/smith + path: ../../../../third_party/dart/pkg/smith diff --git a/tools/engine_tools_lib/test/engine_tools_lib_test.dart b/tools/pkg/engine_tools_lib/test/engine_tools_lib_test.dart similarity index 94% rename from tools/engine_tools_lib/test/engine_tools_lib_test.dart rename to tools/pkg/engine_tools_lib/test/engine_tools_lib_test.dart index 2382b43519349..84aa230d92ccf 100644 --- a/tools/engine_tools_lib/test/engine_tools_lib_test.dart +++ b/tools/pkg/engine_tools_lib/test/engine_tools_lib_test.dart @@ -167,6 +167,11 @@ void main() { // Create two targets in out: host_debug and host_debug_unopt_arm64. io.Directory(p.join(emptyDir.path, 'src', 'out', 'host_debug')).createSync(recursive: true); io.Directory(p.join(emptyDir.path, 'src', 'out', 'host_debug_unopt_arm64')).createSync(recursive: true); + + // Intentionnally make host_debug a day old to ensure it is not picked. + final io.File oldJson = io.File(p.join(emptyDir.path, 'src', 'out', 'host_debug', 'compile_commands.json'))..createSync(); + oldJson.setLastModifiedSync(oldJson.lastModifiedSync().subtract(const Duration(days: 1))); + io.File(p.join(emptyDir.path, 'src', 'out', 'host_debug_unopt_arm64', 'compile_commands.json')).createSync(); final Engine engine = Engine.fromSrcPath(p.join(emptyDir.path, 'src')); diff --git a/tools/pub_get_offline.py b/tools/pub_get_offline.py index ade4e8b346f94..593226fab47e2 100644 --- a/tools/pub_get_offline.py +++ b/tools/pub_get_offline.py @@ -35,7 +35,7 @@ os.path.join(ENGINE_DIR, "tools", "android_lint"), os.path.join(ENGINE_DIR, "tools", "clang_tidy"), os.path.join(ENGINE_DIR, "tools", "const_finder"), - os.path.join(ENGINE_DIR, "tools", "engine_tools_lib"), + os.path.join(ENGINE_DIR, "tools", "pkg", "engine_tools_lib"), os.path.join(ENGINE_DIR, "tools", "githooks"), os.path.join(ENGINE_DIR, "tools", "licenses"), os.path.join(ENGINE_DIR, "tools", "path_ops", "dart") From eb5a7e5bec1273968a0a51af0ef13228eefdfced Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Mon, 28 Aug 2023 11:02:46 -0700 Subject: [PATCH 11/16] Rename engine_tools_lib to engine_repo_tools. --- testing/run_tests.py | 6 +++--- tools/pkg/{engine_tools_lib => engine_repo_tools}/README.md | 4 ++-- .../lib/engine_tools_lib.dart | 0 .../{engine_tools_lib => engine_repo_tools}/pubspec.yaml | 2 +- .../test/engine_tools_lib_test.dart | 6 +++--- tools/pub_get_offline.py | 2 +- 6 files changed, 10 insertions(+), 10 deletions(-) rename tools/pkg/{engine_tools_lib => engine_repo_tools}/README.md (84%) rename tools/pkg/{engine_tools_lib => engine_repo_tools}/lib/engine_tools_lib.dart (100%) rename tools/pkg/{engine_tools_lib => engine_repo_tools}/pubspec.yaml (97%) rename tools/pkg/{engine_tools_lib => engine_repo_tools}/test/engine_tools_lib_test.dart (97%) diff --git a/testing/run_tests.py b/testing/run_tests.py index 596c549661923..6c087e593924c 100755 --- a/testing/run_tests.py +++ b/testing/run_tests.py @@ -953,8 +953,8 @@ def gather_clang_tidy_tests(build_dir): ) -def gather_engine_tools_lib_tests(build_dir): - test_dir = os.path.join(BUILDROOT_DIR, 'flutter', 'tools', 'pkg', 'engine_tools_lib') +def gather_engine_repo_tools_tests(build_dir): + test_dir = os.path.join(BUILDROOT_DIR, 'flutter', 'tools', 'pkg', 'engine_repo_tools') dart_tests = glob.glob('%s/*_test.dart' % test_dir) for dart_test_file in dart_tests: opts = [ @@ -1247,7 +1247,7 @@ def main(): tasks += list(gather_litetest_tests(build_dir)) tasks += list(gather_githooks_tests(build_dir)) tasks += list(gather_clang_tidy_tests(build_dir)) - tasks += list(gather_engine_tools_lib_tests(build_dir)) + tasks += list(gather_engine_repo_tools_tests(build_dir)) tasks += list(gather_api_consistency_tests(build_dir)) tasks += list(gather_path_ops_tests(build_dir)) tasks += list(gather_const_finder_tests(build_dir)) diff --git a/tools/pkg/engine_tools_lib/README.md b/tools/pkg/engine_repo_tools/README.md similarity index 84% rename from tools/pkg/engine_tools_lib/README.md rename to tools/pkg/engine_repo_tools/README.md index ae46336ed40f4..f84d0a9212ac9 100644 --- a/tools/pkg/engine_tools_lib/README.md +++ b/tools/pkg/engine_repo_tools/README.md @@ -1,11 +1,11 @@ -# engine_tools_lib +# engine_repo_tools This is a repo-internal library for `flutter/engine`, that contains shared code for writing tools that operate on the engine repository. For example, finding the latest compiled engine artifacts in the `out/` directory: ```dart -import 'package:engine_tools_lib/engine_tools_lib.dart'; +import 'package:engine_repo_tools/engine_repo_tools.dart'; void main() { final engine = Engine.findWithin(); diff --git a/tools/pkg/engine_tools_lib/lib/engine_tools_lib.dart b/tools/pkg/engine_repo_tools/lib/engine_tools_lib.dart similarity index 100% rename from tools/pkg/engine_tools_lib/lib/engine_tools_lib.dart rename to tools/pkg/engine_repo_tools/lib/engine_tools_lib.dart diff --git a/tools/pkg/engine_tools_lib/pubspec.yaml b/tools/pkg/engine_repo_tools/pubspec.yaml similarity index 97% rename from tools/pkg/engine_tools_lib/pubspec.yaml rename to tools/pkg/engine_repo_tools/pubspec.yaml index 9b6c49a5413c2..455ee7cca5ea6 100644 --- a/tools/pkg/engine_tools_lib/pubspec.yaml +++ b/tools/pkg/engine_repo_tools/pubspec.yaml @@ -2,7 +2,7 @@ # Use of this source code is governed by a BSD-style license that can be # found in the LICENSE file. -name: engine_tools_lib +name: engine_repo_tools publish_to: none environment: sdk: ^3.0.0 diff --git a/tools/pkg/engine_tools_lib/test/engine_tools_lib_test.dart b/tools/pkg/engine_repo_tools/test/engine_tools_lib_test.dart similarity index 97% rename from tools/pkg/engine_tools_lib/test/engine_tools_lib_test.dart rename to tools/pkg/engine_repo_tools/test/engine_tools_lib_test.dart index 84aa230d92ccf..8fc9327120f54 100644 --- a/tools/pkg/engine_tools_lib/test/engine_tools_lib_test.dart +++ b/tools/pkg/engine_repo_tools/test/engine_tools_lib_test.dart @@ -3,7 +3,7 @@ // found in the LICENSE file. import 'dart:io' as io; -import 'package:engine_tools_lib/engine_tools_lib.dart'; +import 'package:engine_repo_tools/engine_repo_tools.dart'; import 'package:litetest/litetest.dart'; import 'package:path/path.dart' as p; @@ -11,7 +11,7 @@ void main() { late io.Directory emptyDir; void setUp() { - emptyDir = io.Directory.systemTemp.createTempSync('engine_tools_lib.test'); + emptyDir = io.Directory.systemTemp.createTempSync('engine_repo_tools.test'); } void tearDown() { @@ -94,7 +94,7 @@ void main() { late io.Directory emptyDir; void setUp() { - emptyDir = io.Directory.systemTemp.createTempSync('engine_tools_lib.test'); + emptyDir = io.Directory.systemTemp.createTempSync('engine_repo_tools.test'); } void tearDown() { diff --git a/tools/pub_get_offline.py b/tools/pub_get_offline.py index 593226fab47e2..8efd6efec285c 100644 --- a/tools/pub_get_offline.py +++ b/tools/pub_get_offline.py @@ -35,7 +35,7 @@ os.path.join(ENGINE_DIR, "tools", "android_lint"), os.path.join(ENGINE_DIR, "tools", "clang_tidy"), os.path.join(ENGINE_DIR, "tools", "const_finder"), - os.path.join(ENGINE_DIR, "tools", "pkg", "engine_tools_lib"), + os.path.join(ENGINE_DIR, "tools", "pkg", "engine_repo_tools"), os.path.join(ENGINE_DIR, "tools", "githooks"), os.path.join(ENGINE_DIR, "tools", "licenses"), os.path.join(ENGINE_DIR, "tools", "path_ops", "dart") From 90ccb77c9fdc52aab26d63012556bd810d354c52 Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Mon, 28 Aug 2023 11:03:42 -0700 Subject: [PATCH 12/16] Finish renaming. --- .../lib/{engine_tools_lib.dart => engine_repo_tools.dart} | 0 .../{engine_tools_lib_test.dart => engine_repo_tools_test.dart} | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename tools/pkg/engine_repo_tools/lib/{engine_tools_lib.dart => engine_repo_tools.dart} (100%) rename tools/pkg/engine_repo_tools/test/{engine_tools_lib_test.dart => engine_repo_tools_test.dart} (100%) diff --git a/tools/pkg/engine_repo_tools/lib/engine_tools_lib.dart b/tools/pkg/engine_repo_tools/lib/engine_repo_tools.dart similarity index 100% rename from tools/pkg/engine_repo_tools/lib/engine_tools_lib.dart rename to tools/pkg/engine_repo_tools/lib/engine_repo_tools.dart diff --git a/tools/pkg/engine_repo_tools/test/engine_tools_lib_test.dart b/tools/pkg/engine_repo_tools/test/engine_repo_tools_test.dart similarity index 100% rename from tools/pkg/engine_repo_tools/test/engine_tools_lib_test.dart rename to tools/pkg/engine_repo_tools/test/engine_repo_tools_test.dart From 3e20c0424f7d58f5480ae382508f564064124377 Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Mon, 28 Aug 2023 11:30:34 -0700 Subject: [PATCH 13/16] All tests passing, Zach's suggestion. --- .../lib/engine_repo_tools.dart | 100 +++++++++++----- .../test/engine_repo_tools_test.dart | 107 +++++++++++++++--- 2 files changed, 160 insertions(+), 47 deletions(-) diff --git a/tools/pkg/engine_repo_tools/lib/engine_repo_tools.dart b/tools/pkg/engine_repo_tools/lib/engine_repo_tools.dart index 69953e73b1e95..fff1b2f3f44a6 100644 --- a/tools/pkg/engine_repo_tools/lib/engine_repo_tools.dart +++ b/tools/pkg/engine_repo_tools/lib/engine_repo_tools.dart @@ -42,40 +42,29 @@ final class Engine { /// print(engine.srcDir.path); // /Users/.../engine/src /// ``` /// - /// Throws an [ArgumentError] if the path does not point to a valid engine. + /// Throws a [InvalidEngineException] if the path is not a valid engine root. factory Engine.fromSrcPath(String srcPath) { // If the path does not end in `/src`, fail. if (p.basename(srcPath) != 'src') { - throw ArgumentError( - 'The path $srcPath does not end in `${p.separator}src`.\n' - '\n$_kHintFindWithinPath' - ); + throw InvalidEngineException.doesNotEndWithSrc(srcPath); } // If the directory does not exist, or is not a directory, fail. final io.Directory srcDir = io.Directory(srcPath); if (!srcDir.existsSync()) { - throw ArgumentError( - 'The path "$srcPath" does not exist or is not a directory.\n' - '\n$_kHintFindWithinPath' - ); + throw InvalidEngineException.notADirectory(srcPath); } // Check for the existence of a `flutter` directory within `src`. final io.Directory flutterDir = io.Directory(p.join(srcPath, 'flutter')); if (!flutterDir.existsSync()) { - throw ArgumentError( - 'The path "$srcPath" does not contain a "flutter" directory.' - ); + throw InvalidEngineException.missingFlutterDirectory(srcPath); } - // Check for the existence of a `out` directory within `src`. + // We do **NOT** check for the existence of a `out` directory within `src`, + // it's not required to exist (i.e. a new checkout of the engine), and we + // don't want to fail if it doesn't exist. final io.Directory outDir = io.Directory(p.join(srcPath, 'out')); - if (!outDir.existsSync()) { - throw ArgumentError( - 'The path "$srcPath" does not contain an "out" directory.' - ); - } return Engine._(srcDir, flutterDir, outDir); } @@ -94,7 +83,7 @@ final class Engine { /// /// If a path is not provided, the current working directory is used. /// - /// Throws an [ArgumentError] if the path is not within a valid engine. + /// Throws a [StateError] if the path is not within a valid engine. factory Engine.findWithin([String? path]) { path ??= p.current; @@ -102,20 +91,22 @@ final class Engine { io.Directory maybeSrcDir = io.Directory(path); if (!maybeSrcDir.existsSync()) { - throw ArgumentError( + throw StateError( 'The path "$path" does not exist or is not a directory.' ); } do { - if (p.basename(maybeSrcDir.path) == 'src') { + try { return Engine.fromSrcPath(maybeSrcDir.path); + } on InvalidEngineException { + // Ignore, we'll keep searching. } maybeSrcDir = maybeSrcDir.parent; } while (maybeSrcDir.parent.path != maybeSrcDir.path /* at root */); - throw ArgumentError( - 'The path "$path" does not contain a "src" directory.' + throw StateError( + 'The path "$path" is not within a Flutter engine source directory.' ); } @@ -125,12 +116,6 @@ final class Engine { this.outDir, ); - static final String _kHintFindWithinPath = - 'This constructor is intended to be used only a path that points to ' - 'a Flutter engine source directory, i.e. a directory that ends in ' - '${p.separator}src. Use "findWithin" to create an Engine from a ' - 'directory that lives within an engine source directory'; - /// The path to the `$ENGINE/src` directory. final io.Directory srcDir; @@ -164,6 +149,63 @@ final class Engine { } } +/// Thrown when an [Engine] could not be created from a path. +sealed class InvalidEngineException implements Exception { + /// Thrown when an [Engine] was created from a path not ending in `src`. + factory InvalidEngineException.doesNotEndWithSrc(String path) { + return InvalidEngineSrcPathException._(path); + } + + /// Thrown when an [Engine] was created from a directory that does not exist. + factory InvalidEngineException.notADirectory(String path) { + return InvalidEngineNotADirectoryException._(path); + } + + /// Thrown when an [Engine] was created from a path not containing `flutter/`. + factory InvalidEngineException.missingFlutterDirectory(String path) { + return InvalidEngineMissingFlutterDirectoryException._(path); + } +} + +/// Thrown when an [Engine] was created from a path not ending in `src`. +final class InvalidEngineSrcPathException implements InvalidEngineException { + InvalidEngineSrcPathException._(this.path); + + /// The path that was used to create the [Engine]. + final String path; + + @override + String toString() { + return 'The path $path does not end in `${p.separator}src`.'; + } +} + +/// Thrown when an [Engine] was created from a path that is not a directory. +final class InvalidEngineNotADirectoryException implements InvalidEngineException { + InvalidEngineNotADirectoryException._(this.path); + + /// The path that was used to create the [Engine]. + final String path; + + @override + String toString() { + return 'The path "$path" does not exist or is not a directory.'; + } +} + +/// Thrown when an [Engine] was created from a path not containing `flutter/`. +final class InvalidEngineMissingFlutterDirectoryException implements InvalidEngineException { + InvalidEngineMissingFlutterDirectoryException._(this.path); + + /// The path that was used to create the [Engine]. + final String path; + + @override + String toString() { + return 'The path "$path" does not contain a "flutter" directory.'; + } +} + /// Represents a single output target in the `$ENGINE/src/out` directory. final class Output { const Output._(this.dir); diff --git a/tools/pkg/engine_repo_tools/test/engine_repo_tools_test.dart b/tools/pkg/engine_repo_tools/test/engine_repo_tools_test.dart index 8fc9327120f54..48c18210b304e 100644 --- a/tools/pkg/engine_repo_tools/test/engine_repo_tools_test.dart +++ b/tools/pkg/engine_repo_tools/test/engine_repo_tools_test.dart @@ -3,6 +3,7 @@ // found in the LICENSE file. import 'dart:io' as io; +import 'package:async_helper/async_helper.dart'; import 'package:engine_repo_tools/engine_repo_tools.dart'; import 'package:litetest/litetest.dart'; import 'package:path/path.dart' as p; @@ -25,7 +26,7 @@ void main() { try { expect( () => Engine.fromSrcPath(emptyDir.path), - throwsArgumentError, + _throwsInvalidEngineException, ); } finally { tearDown(); @@ -37,7 +38,7 @@ void main() { try { expect( () => Engine.fromSrcPath(p.join(emptyDir.path, 'src')), - throwsArgumentError, + _throwsInvalidEngineException, ); } finally { tearDown(); @@ -50,21 +51,7 @@ void main() { final io.Directory srcDir = io.Directory(p.join(emptyDir.path, 'src'))..createSync(); expect( () => Engine.fromSrcPath(srcDir.path), - throwsArgumentError, - ); - } finally { - tearDown(); - } - }); - - test('the path does not contain an "out" directory', () { - setUp(); - try { - final io.Directory srcDir = io.Directory(p.join(emptyDir.path, 'src'))..createSync(); - io.Directory(p.join(srcDir.path, 'flutter')).createSync(); - expect( - () => Engine.fromSrcPath(srcDir.path), - throwsArgumentError, + _throwsInvalidEngineException, ); } finally { tearDown(); @@ -107,7 +94,20 @@ void main() { try { expect( () => Engine.findWithin(emptyDir.path), - throwsArgumentError, + throwsStateError, + ); + } finally { + tearDown(); + } + }); + + test('the path contains a "src" directory but it is not an engine root', () { + setUp(); + try { + final io.Directory srcDir = io.Directory(p.join(emptyDir.path, 'src'))..createSync(); + expect( + () => Engine.findWithin(srcDir.path), + throwsStateError, ); } finally { tearDown(); @@ -130,9 +130,50 @@ void main() { tearDown(); } }); + + test('returns an Engine even if a "src" directory exists deeper in the tree', () { + // It's common to have "src" directories, so if we have something like: + // /Users/.../engine/src/foo/bar/src/baz + // + // And we use `Engine.findWithin('/Users/.../engine/src/flutter/bar/src/baz')`, + // we should still find the engine (in this case, the engine root is + // `/Users/.../engine/src`). + setUp(); + try { + final io.Directory srcDir = io.Directory(p.join(emptyDir.path, 'src'))..createSync(); + io.Directory(p.join(srcDir.path, 'flutter')).createSync(); + io.Directory(p.join(srcDir.path, 'out')).createSync(); + + final io.Directory nestedSrcDir = io.Directory(p.join(srcDir.path, 'flutter', 'bar', 'src', 'baz'))..createSync(recursive: true); + + final Engine engine = Engine.findWithin(nestedSrcDir.path); + + expect(engine.srcDir.path, srcDir.path); + expect(engine.flutterDir.path, p.join(srcDir.path, 'flutter')); + expect(engine.outDir.path, p.join(srcDir.path, 'out')); + } finally { + tearDown(); + } + }); }); }); + test('outputs an empty list of targets', () { + setUp(); + + try { + // Create a valid engine. + io.Directory(p.join(emptyDir.path, 'src', 'flutter')).createSync(recursive: true); + io.Directory(p.join(emptyDir.path, 'src', 'out')).createSync(recursive: true); + + final Engine engine = Engine.fromSrcPath(p.join(emptyDir.path, 'src')); + expect(engine.outputs(), []); + expect(engine.latestOutput(), isNull); + } finally { + tearDown(); + } + }); + test('outputs a list of targets', () { setUp(); @@ -184,3 +225,33 @@ void main() { } }); } + +// This is needed because async_minitest and friends is not a proper testing +// library and is missing a lot of functionality that was exclusively added +// to pkg/test. +void _throwsInvalidEngineException(Object? o) { + _checkThrow(o, (_){}); +} + +// Mostly copied from async_minitest. +void _checkThrow(dynamic v, void Function(dynamic error) onError) { + if (v is Future) { + asyncStart(); + v.then((_) { + Expect.fail('Did not throw'); + }, onError: (Object e, StackTrace s) { + if (e is! T) { + // ignore: only_throw_errors + throw e; + } + onError(e); + asyncEnd(); + }); + return; + } + v as void Function(); + Expect.throws(v, (T e) { + onError(e); + return true; + }); +} From c83a72497b55c89ff5ddc007fbbf1ef4b59cbe84 Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Mon, 28 Aug 2023 11:31:37 -0700 Subject: [PATCH 14/16] Formatting. --- testing/run_tests.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/testing/run_tests.py b/testing/run_tests.py index 6c087e593924c..9a4a893205153 100755 --- a/testing/run_tests.py +++ b/testing/run_tests.py @@ -954,7 +954,9 @@ def gather_clang_tidy_tests(build_dir): def gather_engine_repo_tools_tests(build_dir): - test_dir = os.path.join(BUILDROOT_DIR, 'flutter', 'tools', 'pkg', 'engine_repo_tools') + test_dir = os.path.join( + BUILDROOT_DIR, 'flutter', 'tools', 'pkg', 'engine_repo_tools' + ) dart_tests = glob.glob('%s/*_test.dart' % test_dir) for dart_test_file in dart_tests: opts = [ From c1308514de4f820bf02bfe0ae100922457e9bf5c Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Mon, 28 Aug 2023 12:26:31 -0700 Subject: [PATCH 15/16] Add a note. --- tools/pkg/engine_repo_tools/lib/engine_repo_tools.dart | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tools/pkg/engine_repo_tools/lib/engine_repo_tools.dart b/tools/pkg/engine_repo_tools/lib/engine_repo_tools.dart index fff1b2f3f44a6..70e3758435a83 100644 --- a/tools/pkg/engine_repo_tools/lib/engine_repo_tools.dart +++ b/tools/pkg/engine_repo_tools/lib/engine_repo_tools.dart @@ -123,6 +123,8 @@ final class Engine { final io.Directory flutterDir; /// The path to the `$ENGINE/src/out` directory. + /// + /// **NOTE**: This directory may not exist. final io.Directory outDir; /// Returns a list of all output targets in [outDir]. From 99d44c826f0b0534c9e0da75224a6c4f0978985f Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Mon, 28 Aug 2023 12:28:06 -0700 Subject: [PATCH 16/16] Whitespace. --- tools/pkg/engine_repo_tools/lib/engine_repo_tools.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/pkg/engine_repo_tools/lib/engine_repo_tools.dart b/tools/pkg/engine_repo_tools/lib/engine_repo_tools.dart index 70e3758435a83..820f1bf29045b 100644 --- a/tools/pkg/engine_repo_tools/lib/engine_repo_tools.dart +++ b/tools/pkg/engine_repo_tools/lib/engine_repo_tools.dart @@ -123,7 +123,7 @@ final class Engine { final io.Directory flutterDir; /// The path to the `$ENGINE/src/out` directory. - /// + /// /// **NOTE**: This directory may not exist. final io.Directory outDir;