From f13ccb269af7429ce7abc2e3205796554db28976 Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Thu, 29 Aug 2024 15:35:36 -0700 Subject: [PATCH 1/8] Migrate path_ops. --- testing/run_tests.py | 8 ++++-- tools/path_ops/README.md | 19 +++++++++++++ tools/path_ops/dart/pubspec.yaml | 2 +- tools/path_ops/dart/test/path_ops_test.dart | 31 +++++++++++++++------ 4 files changed, 47 insertions(+), 13 deletions(-) diff --git a/testing/run_tests.py b/testing/run_tests.py index 1564da7698527..6fc459e4ab6ae 100755 --- a/testing/run_tests.py +++ b/testing/run_tests.py @@ -133,7 +133,10 @@ def run_cmd( # pylint: disable=too-many-arguments ) print_divider('<') - logger.info('Command run successfully in %.2f seconds: %s', end_time - start_time, command_string) + logger.info( + 'Command run successfully in %.2f seconds: %s (in %s)', end_time - start_time, command_string, + cwd + ) def is_mac(): @@ -919,8 +922,7 @@ def gather_dart_package_tests(build_dir, package_path, extra_opts): # # Until then, assert that no extra_opts are passed and explain the limitation. assert not extra_opts, '%s uses package:test and cannot use CLI args' % package_path - # TODO(https://github.com/flutter/flutter/issues/154263): Restore `--disable-dart-dev`. - opts = ['test'] + opts = ['test', '--reporter=expanded'] yield EngineExecutableTask( build_dir, os.path.join('dart-sdk', 'bin', 'dart'), None, flags=opts, cwd=package_path ) diff --git a/tools/path_ops/README.md b/tools/path_ops/README.md index 3bc886dc06db0..e64985fe1edbc 100644 --- a/tools/path_ops/README.md +++ b/tools/path_ops/README.md @@ -10,3 +10,22 @@ This library is a subset of the functionality provided by Skia's `PathKit` library. It is primarily intended for use with the `vector_graphics` optimizing compiler. That library uses this one to optimize certain masking and clipping operations at compile time. + +## Testing + +The `path_ops` library uses dynamic libraries (i.e. `libpath_ops.dylib` or the +equivalent on your platform) to link against the C++ code in +[`path_ops.cpp`](path_ops.cpp). When run through +[`run_tests.py`](../../testing/run_tests.py), paths are set automatically to +make finding the dynamic library work. + +However, if run directly from the command line or IDE, an environment variable +`DYLD_LIBRARY_PATH` (or equivalent on your platform) must be set to the +directory containing the dynamic library. For example, on macOS: + +```sh +export DYLD_LIBRARY_PATH=BUILD_DIR +``` + +... where `BUILD_DIR` is the output directory for the engine build, such as +`../out/host_debug_unopt_arm64`. diff --git a/tools/path_ops/dart/pubspec.yaml b/tools/path_ops/dart/pubspec.yaml index 7916d5072e006..49e358c0d06e1 100644 --- a/tools/path_ops/dart/pubspec.yaml +++ b/tools/path_ops/dart/pubspec.yaml @@ -12,4 +12,4 @@ environment: resolution: workspace dev_dependencies: - litetest: any + test: any diff --git a/tools/path_ops/dart/test/path_ops_test.dart b/tools/path_ops/dart/test/path_ops_test.dart index d12dcdf2de225..f92e3fbc31e44 100644 --- a/tools/path_ops/dart/test/path_ops_test.dart +++ b/tools/path_ops/dart/test/path_ops_test.dart @@ -2,10 +2,13 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -import 'package:litetest/litetest.dart'; +import 'dart:io' as io; + import 'package:path_ops/path_ops.dart'; +import 'package:test/test.dart'; void main() { + print(io.Directory.current); test('Path tests', () { final Path path = Path() ..lineTo(10, 0) @@ -68,10 +71,13 @@ void main() { expect(intersection.points, [ 34.06542205810547, 128.0, // move 48.90797424316406, 48.59233856201172, // line - 57.80497360229492, 39.73065185546875, 68.189697265625, 32.3614387512207, 79.66168212890625, 26.885154724121094, // cubic + 57.80497360229492, 39.73065185546875, 68.189697265625, 32.3614387512207, + 79.66168212890625, 26.885154724121094, // cubic 151.7936248779297, 58.72270584106445, // line - 150.66123962402344, 59.74142837524414, 149.49365234375, 60.752471923828125, 148.32867431640625, 61.76123809814453, // cubic - 132.3506317138672, 75.59684753417969, 116.86703491210938, 89.0042953491211, 199.52090454101562, 115.93260192871094, // cubic + 150.66123962402344, 59.74142837524414, 149.49365234375, + 60.752471923828125, 148.32867431640625, 61.76123809814453, // cubic + 132.3506317138672, 75.59684753417969, 116.86703491210938, + 89.0042953491211, 199.52090454101562, 115.93260192871094, // cubic 199.36000061035156, 128.0, // line 34.06542205810547, 128.0, // line // close @@ -85,9 +91,11 @@ void main() { final Path a = Path(FillType.evenOdd) ..moveTo(9.989999771118164, 20.0) ..cubicTo(4.46999979019165, 20.0, 0.0, 15.520000457763672, 0.0, 10.0) - ..cubicTo(0.0, 4.480000019073486, 4.46999979019165, 0.0, 9.989999771118164, 0.0) + ..cubicTo( + 0.0, 4.480000019073486, 4.46999979019165, 0.0, 9.989999771118164, 0.0) ..cubicTo(15.520000457763672, 0.0, 20.0, 4.480000019073486, 20.0, 10.0) - ..cubicTo(20.0, 15.520000457763672, 15.520000457763672, 20.0, 9.989999771118164, 20.0) + ..cubicTo(20.0, 15.520000457763672, 15.520000457763672, 20.0, + 9.989999771118164, 20.0) ..close() ..moveTo(10.0, 18.0) ..cubicTo(5.579999923706055, 18.0, 2.0, 14.420000076293945, 2.0, 10.0) @@ -107,9 +115,14 @@ void main() { ..lineTo(9.0, 13.0) ..lineTo(11.0, 13.0) ..close(); - final Path b = Path()..moveTo(0, 0)..lineTo(0, 20)..lineTo(20, 20)..lineTo(20, 0)..close(); + final Path b = Path() + ..moveTo(0, 0) + ..lineTo(0, 20) + ..lineTo(20, 20) + ..lineTo(20, 0) + ..close(); - final Path intersection = a.applyOp(b, PathOp.intersect); - expect(intersection.fillType, a.fillType); + final Path intersection = a.applyOp(b, PathOp.intersect); + expect(intersection.fillType, a.fillType); }); } From 0f5e41bdd4b8621739c694ec9c4ac53c9e4f1645 Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Thu, 29 Aug 2024 15:46:27 -0700 Subject: [PATCH 2/8] Move more tests to package:test. --- pubspec.yaml | 3 +- testing/pkg_test_demo/README.md | 32 ------------------- testing/pkg_test_demo/pubspec.yaml | 16 ---------- testing/pkg_test_demo/test/example_test.dart | 17 ---------- testing/run_tests.py | 1 - testing/scenario_app/pubspec.yaml | 7 +--- .../test/adb_log_filter_test.dart | 2 +- testing/skia_gold_client/pubspec.yaml | 2 +- .../test/release_version_test.dart | 2 +- .../test/skia_gold_client_test.dart | 2 +- testing/smoke_test_failure/fail_test.dart | 2 +- testing/smoke_test_failure/pubspec.yaml | 2 +- tools/golden_tests_harvester/pubspec.yaml | 2 +- .../test/golden_tests_harvester_test.dart | 2 +- tools/pub_get_offline.py | 2 -- 15 files changed, 11 insertions(+), 83 deletions(-) delete mode 100644 testing/pkg_test_demo/README.md delete mode 100644 testing/pkg_test_demo/pubspec.yaml delete mode 100644 testing/pkg_test_demo/test/example_test.dart diff --git a/pubspec.yaml b/pubspec.yaml index 171e51258b8b7..ab0aa7e2a103f 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -87,7 +87,6 @@ workspace: - testing/benchmark - testing/dart - testing/litetest - - testing/pkg_test_demo - testing/scenario_app - testing/skia_gold_client - testing/smoke_test_failure @@ -263,6 +262,8 @@ dependency_overrides: path: ./third_party/dart/third_party/pkg/shelf/pkgs/shelf_static shelf_web_socket: path: ./third_party/dart/third_party/pkg/shelf/pkgs/shelf_web_socket + sky_engine: + path: ./sky/packages/sky_engine smith: path: ./third_party/dart/pkg/smith source_map_stack_trace: diff --git a/testing/pkg_test_demo/README.md b/testing/pkg_test_demo/README.md deleted file mode 100644 index 85f548c9d52b4..0000000000000 --- a/testing/pkg_test_demo/README.md +++ /dev/null @@ -1,32 +0,0 @@ -# Demo of `package:test` with `DEPS`-vendored packages - -Historically, `flutter/engine` used a homegrown test framework, -[`package:litetest`](../litetest/) to avoid depending on the unwieldy set of -dependencies that `package:test` brings in. However, `package:test` is now -vendored in `DEPS` (used by the Dart SDK).' - -This demo shows that: - -- It's possible to use `package:test` with entirely local dependencies. -- The functionality of `package:test` (such as filtering, IDE integration, etc.) - is available. - -See for details. - -## Usage - -Navigate to this directory: - -```sh -cd testing/pkg_test_demo -``` - -And run the tests using `dart test`[^1]: - -```sh -dart test -``` - -[^1]: - In practice, you'll want to use the `dart` binary that is vendored in the - pre-built SDK. diff --git a/testing/pkg_test_demo/pubspec.yaml b/testing/pkg_test_demo/pubspec.yaml deleted file mode 100644 index 22bb07bd48e7b..0000000000000 --- a/testing/pkg_test_demo/pubspec.yaml +++ /dev/null @@ -1,16 +0,0 @@ -# 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: pkg_test_demo -publish_to: none - -# Required for workspace support. -environment: - sdk: ^3.5.0-294.0.dev - -# This package is managed as part of the engine workspace. -resolution: workspace - -dev_dependencies: - test: any diff --git a/testing/pkg_test_demo/test/example_test.dart b/testing/pkg_test_demo/test/example_test.dart deleted file mode 100644 index 01343d549eb58..0000000000000 --- a/testing/pkg_test_demo/test/example_test.dart +++ /dev/null @@ -1,17 +0,0 @@ -// 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 'package:test/test.dart'; - -void main() { - test('String.split() splits the string on the delimiter', () { - const String string = 'foo,bar,baz'; - expect(string.split(','), equals(['foo', 'bar', 'baz'])); - }); - - test('String.trim() removes surrounding whitespace', () { - const String string = ' foo '; - expect(string.trim(), equals('foo')); - }); -} diff --git a/testing/run_tests.py b/testing/run_tests.py index 6fc459e4ab6ae..4489fc7543f60 100755 --- a/testing/run_tests.py +++ b/testing/run_tests.py @@ -978,7 +978,6 @@ def build_dart_host_test_list(build_dir): ], ), (os.path.join('flutter', 'testing', 'litetest'), []), - (os.path.join('flutter', 'testing', 'pkg_test_demo'), []), (os.path.join('flutter', 'testing', 'skia_gold_client'), []), (os.path.join('flutter', 'testing', 'scenario_app'), []), ( diff --git a/testing/scenario_app/pubspec.yaml b/testing/scenario_app/pubspec.yaml index 773a68cd89f3d..f729a481226f7 100644 --- a/testing/scenario_app/pubspec.yaml +++ b/testing/scenario_app/pubspec.yaml @@ -24,9 +24,4 @@ dependencies: sky_engine: any dev_dependencies: - litetest: any - -# TODO(matanlurey): Figure out how sky_engine fits into the pub workspace model. -dependency_overrides: - sky_engine: - path: ../../sky/packages/sky_engine + test: any diff --git a/testing/scenario_app/test/adb_log_filter_test.dart b/testing/scenario_app/test/adb_log_filter_test.dart index c1e573157f15d..086fa342dce10 100644 --- a/testing/scenario_app/test/adb_log_filter_test.dart +++ b/testing/scenario_app/test/adb_log_filter_test.dart @@ -1,4 +1,4 @@ -import 'package:litetest/litetest.dart'; +import 'package:test/test.dart'; import '../bin/utils/adb_logcat_filtering.dart'; import 'src/fake_adb_logcat.dart'; diff --git a/testing/skia_gold_client/pubspec.yaml b/testing/skia_gold_client/pubspec.yaml index b9804a73026e2..d0e133633eb53 100644 --- a/testing/skia_gold_client/pubspec.yaml +++ b/testing/skia_gold_client/pubspec.yaml @@ -22,5 +22,5 @@ dependencies: process: any dev_dependencies: - litetest: any process_fakes: any + test: any diff --git a/testing/skia_gold_client/test/release_version_test.dart b/testing/skia_gold_client/test/release_version_test.dart index 72f7e73a31f53..36fc156abe2fb 100644 --- a/testing/skia_gold_client/test/release_version_test.dart +++ b/testing/skia_gold_client/test/release_version_test.dart @@ -2,8 +2,8 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -import 'package:litetest/litetest.dart'; import 'package:skia_gold_client/src/release_version.dart'; +import 'package:test/test.dart'; void main() { test('should accept a major and minor version', () { diff --git a/testing/skia_gold_client/test/skia_gold_client_test.dart b/testing/skia_gold_client/test/skia_gold_client_test.dart index b3265866da3da..5365369326ba1 100644 --- a/testing/skia_gold_client/test/skia_gold_client_test.dart +++ b/testing/skia_gold_client/test/skia_gold_client_test.dart @@ -8,11 +8,11 @@ import 'dart:io' as io; import 'dart:typed_data'; import 'package:engine_repo_tools/engine_repo_tools.dart'; -import 'package:litetest/litetest.dart'; import 'package:path/path.dart' as p; import 'package:process_fakes/process_fakes.dart'; import 'package:skia_gold_client/skia_gold_client.dart'; import 'package:skia_gold_client/src/release_version.dart'; +import 'package:test/test.dart'; void main() { /// A mock commit hash that is used to simulate a successful git call. diff --git a/testing/smoke_test_failure/fail_test.dart b/testing/smoke_test_failure/fail_test.dart index 90087f62df184..5d88caaaa5d47 100644 --- a/testing/smoke_test_failure/fail_test.dart +++ b/testing/smoke_test_failure/fail_test.dart @@ -2,7 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -import 'package:litetest/litetest.dart'; +import 'package:test/test.dart'; void main() { test('test smoke test -- this test SHOULD FAIL', () { diff --git a/testing/smoke_test_failure/pubspec.yaml b/testing/smoke_test_failure/pubspec.yaml index 19d32475c9671..bfb5ab78aaaf5 100644 --- a/testing/smoke_test_failure/pubspec.yaml +++ b/testing/smoke_test_failure/pubspec.yaml @@ -13,4 +13,4 @@ environment: resolution: workspace dependencies: - litetest: any + test: any diff --git a/tools/golden_tests_harvester/pubspec.yaml b/tools/golden_tests_harvester/pubspec.yaml index b72b06fe134d1..c512a9c1c4ca3 100644 --- a/tools/golden_tests_harvester/pubspec.yaml +++ b/tools/golden_tests_harvester/pubspec.yaml @@ -19,4 +19,4 @@ dependencies: skia_gold_client: any dev_dependencies: - litetest: any + test: any diff --git a/tools/golden_tests_harvester/test/golden_tests_harvester_test.dart b/tools/golden_tests_harvester/test/golden_tests_harvester_test.dart index bdbbabe7fb9e1..0a12b8d87483b 100644 --- a/tools/golden_tests_harvester/test/golden_tests_harvester_test.dart +++ b/tools/golden_tests_harvester/test/golden_tests_harvester_test.dart @@ -6,8 +6,8 @@ import 'dart:async'; import 'dart:io' as io; import 'package:golden_tests_harvester/golden_tests_harvester.dart'; -import 'package:litetest/litetest.dart'; import 'package:path/path.dart' as p; +import 'package:test/test.dart'; void main() async { Future withTempDirectory( diff --git a/tools/pub_get_offline.py b/tools/pub_get_offline.py index 2195a676a7db6..ad5c88c348d06 100644 --- a/tools/pub_get_offline.py +++ b/tools/pub_get_offline.py @@ -30,7 +30,6 @@ os.path.join(ENGINE_DIR, 'testing', 'benchmark'), os.path.join(ENGINE_DIR, 'testing', 'dart'), os.path.join(ENGINE_DIR, 'testing', 'litetest'), - os.path.join(ENGINE_DIR, 'testing', 'pkg_test_demo'), os.path.join(ENGINE_DIR, 'testing', 'scenario_app'), os.path.join(ENGINE_DIR, 'testing', 'skia_gold_client'), os.path.join(ENGINE_DIR, 'testing', 'smoke_test_failure'), @@ -104,7 +103,6 @@ def check_package_config(package): os.path.join(ENGINE_DIR, 'shell', 'platform', 'fuchsia'), os.path.join(ENGINE_DIR, 'shell', 'vmservice'), os.path.join(ENGINE_DIR, 'sky', 'packages'), - os.path.join(ENGINE_DIR, 'testing', 'pkg_test_demo'), os.path.join(ENGINE_DIR, 'third_party'), os.path.join(ENGINE_DIR, 'web_sdk'), ] From 77e33427a88a7b0885c02f4a712b0a186a9f7a66 Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Thu, 29 Aug 2024 15:50:27 -0700 Subject: [PATCH 3/8] ++ --- tools/path_ops/dart/test/path_ops_test.dart | 29 ++++++--------------- 1 file changed, 8 insertions(+), 21 deletions(-) diff --git a/tools/path_ops/dart/test/path_ops_test.dart b/tools/path_ops/dart/test/path_ops_test.dart index f92e3fbc31e44..d24dcfc804ca5 100644 --- a/tools/path_ops/dart/test/path_ops_test.dart +++ b/tools/path_ops/dart/test/path_ops_test.dart @@ -2,13 +2,10 @@ // 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:path_ops/path_ops.dart'; import 'package:test/test.dart'; void main() { - print(io.Directory.current); test('Path tests', () { final Path path = Path() ..lineTo(10, 0) @@ -71,13 +68,10 @@ void main() { expect(intersection.points, [ 34.06542205810547, 128.0, // move 48.90797424316406, 48.59233856201172, // line - 57.80497360229492, 39.73065185546875, 68.189697265625, 32.3614387512207, - 79.66168212890625, 26.885154724121094, // cubic + 57.80497360229492, 39.73065185546875, 68.189697265625, 32.3614387512207, 79.66168212890625, 26.885154724121094, // cubic 151.7936248779297, 58.72270584106445, // line - 150.66123962402344, 59.74142837524414, 149.49365234375, - 60.752471923828125, 148.32867431640625, 61.76123809814453, // cubic - 132.3506317138672, 75.59684753417969, 116.86703491210938, - 89.0042953491211, 199.52090454101562, 115.93260192871094, // cubic + 150.66123962402344, 59.74142837524414, 149.49365234375, 60.752471923828125, 148.32867431640625, 61.76123809814453, // cubic + 132.3506317138672, 75.59684753417969, 116.86703491210938, 89.0042953491211, 199.52090454101562, 115.93260192871094, // cubic 199.36000061035156, 128.0, // line 34.06542205810547, 128.0, // line // close @@ -91,11 +85,9 @@ void main() { final Path a = Path(FillType.evenOdd) ..moveTo(9.989999771118164, 20.0) ..cubicTo(4.46999979019165, 20.0, 0.0, 15.520000457763672, 0.0, 10.0) - ..cubicTo( - 0.0, 4.480000019073486, 4.46999979019165, 0.0, 9.989999771118164, 0.0) + ..cubicTo(0.0, 4.480000019073486, 4.46999979019165, 0.0, 9.989999771118164, 0.0) ..cubicTo(15.520000457763672, 0.0, 20.0, 4.480000019073486, 20.0, 10.0) - ..cubicTo(20.0, 15.520000457763672, 15.520000457763672, 20.0, - 9.989999771118164, 20.0) + ..cubicTo(20.0, 15.520000457763672, 15.520000457763672, 20.0, 9.989999771118164, 20.0) ..close() ..moveTo(10.0, 18.0) ..cubicTo(5.579999923706055, 18.0, 2.0, 14.420000076293945, 2.0, 10.0) @@ -115,14 +107,9 @@ void main() { ..lineTo(9.0, 13.0) ..lineTo(11.0, 13.0) ..close(); - final Path b = Path() - ..moveTo(0, 0) - ..lineTo(0, 20) - ..lineTo(20, 20) - ..lineTo(20, 0) - ..close(); + final Path b = Path()..moveTo(0, 0)..lineTo(0, 20)..lineTo(20, 20)..lineTo(20, 0)..close(); - final Path intersection = a.applyOp(b, PathOp.intersect); - expect(intersection.fillType, a.fillType); + final Path intersection = a.applyOp(b, PathOp.intersect); + expect(intersection.fillType, a.fillType); }); } From 4fbfaeeea8dd34a66ec97517c05eece07a4442cc Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Tue, 3 Sep 2024 12:26:11 -0700 Subject: [PATCH 4/8] Make the command failed message include the CWD as its now relevant. --- testing/run_tests.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testing/run_tests.py b/testing/run_tests.py index 4489fc7543f60..e712149dd60a5 100755 --- a/testing/run_tests.py +++ b/testing/run_tests.py @@ -124,7 +124,7 @@ def run_cmd( # pylint: disable=too-many-arguments allowed_failure = True if not allowed_failure: - raise RuntimeError('Command "%s" exited with code %s.' % (command_string, process.returncode)) + raise RuntimeError('Command "%s" (in %s) exited with code %s.' % (command_string, cwd, process.returncode)) for forbidden_string in forbidden_output: if forbidden_string in output: From 4bb71a044f7f371299f22a122774c9000db8feaa Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Tue, 3 Sep 2024 13:38:55 -0700 Subject: [PATCH 5/8] ++ --- testing/run_tests.py | 4 +- tools/clang_tidy/lib/src/options.dart | 2 +- tools/clang_tidy/pubspec.yaml | 5 +- tools/clang_tidy/test/clang_tidy_test.dart | 55 ++++++++++++------- .../test/header_filter_regex_test.dart | 14 ++--- 5 files changed, 48 insertions(+), 32 deletions(-) diff --git a/testing/run_tests.py b/testing/run_tests.py index e712149dd60a5..10ddf9e7ce53f 100755 --- a/testing/run_tests.py +++ b/testing/run_tests.py @@ -124,7 +124,9 @@ def run_cmd( # pylint: disable=too-many-arguments allowed_failure = True if not allowed_failure: - raise RuntimeError('Command "%s" (in %s) exited with code %s.' % (command_string, cwd, process.returncode)) + raise RuntimeError( + 'Command "%s" (in %s) exited with code %s.' % (command_string, cwd, process.returncode) + ) for forbidden_string in forbidden_output: if forbidden_string in output: diff --git a/tools/clang_tidy/lib/src/options.dart b/tools/clang_tidy/lib/src/options.dart index 160210f1ef5cc..c4d1d8d476576 100644 --- a/tools/clang_tidy/lib/src/options.dart +++ b/tools/clang_tidy/lib/src/options.dart @@ -10,7 +10,7 @@ import 'package:path/path.dart' as path; import 'lint_target.dart'; -final Engine _engineRoot = Engine.findWithin(path.dirname(path.fromUri(io.Platform.script))); +final Engine _engineRoot = Engine.findWithin(); /// Adds warnings as errors for only specific runs. This is helpful if migrating one platform at a time. String? _platformSpecificWarningsAsErrors(ArgResults options) { diff --git a/tools/clang_tidy/pubspec.yaml b/tools/clang_tidy/pubspec.yaml index 194b9212dacfd..bf9fa508f0f32 100644 --- a/tools/clang_tidy/pubspec.yaml +++ b/tools/clang_tidy/pubspec.yaml @@ -22,9 +22,6 @@ dependencies: process_runner: any dev_dependencies: - async_helper: any - expect: any - litetest: any process_fakes: any - smith: any + test: any yaml: any diff --git a/tools/clang_tidy/test/clang_tidy_test.dart b/tools/clang_tidy/test/clang_tidy_test.dart index 14eddb23f3b49..c0f6d19012ca0 100644 --- a/tools/clang_tidy/test/clang_tidy_test.dart +++ b/tools/clang_tidy/test/clang_tidy_test.dart @@ -10,12 +10,11 @@ import 'package:clang_tidy/src/command.dart'; import 'package:clang_tidy/src/lint_target.dart'; import 'package:clang_tidy/src/options.dart'; import 'package:engine_repo_tools/engine_repo_tools.dart'; -import 'package:litetest/litetest.dart'; import 'package:path/path.dart' as path; import 'package:process/process.dart'; import 'package:process_fakes/process_fakes.dart'; import 'package:process_runner/process_runner.dart'; - +import 'package:test/test.dart'; /// A test fixture for the `clang-tidy` tool. final class Fixture { @@ -94,8 +93,7 @@ Use -header-filter=.* to display errors from all non-system headers. Use -system 1 warning treated as error'''; void _withTempFile(String prefix, void Function(String path) func) { - final String filePath = - path.join(io.Directory.systemTemp.path, '$prefix-temp-file'); + final String filePath = path.join(io.Directory.systemTemp.path, '$prefix-temp-file'); final io.File file = io.File(filePath); file.createSync(); try { @@ -105,16 +103,31 @@ void _withTempFile(String prefix, void Function(String path) func) { } } -Future main(List args) async { - final String? buildCommands = - args.firstOrNull ?? - Engine.findWithin().latestOutput()?.compileCommandsJson.path; - - if (buildCommands == null || args.length > 1) { - io.stderr.writeln( - 'Usage: clang_tidy_test.dart [path/to/compile_commands.json]', - ); - return 1; +final _engineRoot = Engine.findWithin(); + +void main() { + // This test requires a compile_commands.json file to exist. + // + // On CI, we'll want to provide exactly which build to use, i.e.: + // COMPILE_COMMANDS_PATH=/path/to/compile_commands.json dart test + // + // Locally, we can fall back to the latest build output if one isn't provided. + // + // Otherwise, fail. + final String buildCommands; + if (io.Platform.environment['COMPILE_COMMANDS_PATH'] case final String compileCommandsPath) { + buildCommands = compileCommandsPath; + } else if (io.Platform.environment['LUCI_CONTEXT'] == null) { + final String? inferredPath = _engineRoot.latestOutput()?.compileCommandsJson.path; + io.stderr.writeln('No COMPILE_COMMANDS_PATH found in environment.'); + if (inferredPath != null) { + io.stderr.writeln('Since this is a local run, inferring the last build output: $inferredPath'); + buildCommands = inferredPath; + } else { + fail('No outputs or build commands found.'); + } + } else { + fail('No COMPILE_COMMANDS_PATH found in environment.'); } test('--help gives help, and uses host_debug by default outside of an engine root', () async { @@ -253,7 +266,7 @@ Future main(List args) async { final int result = await fixture.tool.run(); expect(result, equals(1)); - expect(fixture.errBuffer.toString().split('\n')[0], hasMatch( + expect(fixture.errBuffer.toString().split('\n')[0], matches( r"ERROR: Build commands path .*/does/not/exist doesn't exist.", )); }); @@ -270,7 +283,7 @@ Future main(List args) async { final int result = await fixture.tool.run(); expect(result, equals(1)); - expect(fixture.errBuffer.toString().split('\n')[0], hasMatch( + expect(fixture.errBuffer.toString().split('\n')[0], matches( r'ERROR: Build commands path .*/does/not/exist' r'[/\\]out[/\\]ios_debug_unopt[/\\]compile_commands.json' r" doesn't exist.", @@ -475,7 +488,13 @@ Future main(List args) async { ); // This file needs to exist, and be UTF8 line-parsable. - final String filePath = io.Platform.script.toFilePath(); + final String filePath = path.join( + _engineRoot.flutterDir.path, + 'tools', + 'clang_tidy', + 'test', + 'clang_tidy_test.dart', + ); final List buildCommandsData = >[ { 'directory': '/unused', @@ -657,6 +676,4 @@ Future main(List args) async { expect(result, 0); }); - - return 0; } diff --git a/tools/clang_tidy/test/header_filter_regex_test.dart b/tools/clang_tidy/test/header_filter_regex_test.dart index dbf47daf4ba6d..d17f7e42231df 100644 --- a/tools/clang_tidy/test/header_filter_regex_test.dart +++ b/tools/clang_tidy/test/header_filter_regex_test.dart @@ -5,14 +5,14 @@ import 'dart:io'; import 'package:engine_repo_tools/engine_repo_tools.dart'; -import 'package:litetest/litetest.dart'; import 'package:path/path.dart' as path; +import 'package:test/test.dart'; import 'package:yaml/yaml.dart' as yaml; /// Tests that `HeaderFilterRegex` works as expected in `.clang_tidy`. void main() { // Find the root of the repo. - final Engine engine = Engine.findWithin(path.dirname(Platform.script.path)); + final Engine engine = Engine.findWithin(); // Find the `.clang_tidy` file and "parse" it (it's YAML). final yaml.YamlDocument dotClangTidy = yaml.loadYamlDocument( @@ -62,11 +62,11 @@ void main() { // Create a fake file in that path, and assert that it matches the regex. for (final String rootDir in rootDirs) { final String file = path.join('..', '..', 'flutter', rootDir, 'foo'); - if (!regexValue.hasMatch(file)) { - // This is because reason: ... doesn't work in pkg/litetest. - stderr.writeln('Expected "$file" to be caught by HeaderFilterRegex (${regexValue.pattern}).'); - } - expect(regexValue.hasMatch(file), isTrue); + expect( + file, + matches(regexValue), + reason: '$rootDir/foo should be allowed by the regex but was not.', + ); } }); } From b383e68c48bb522c07f345f514435bd45fda4d81 Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Tue, 3 Sep 2024 14:24:19 -0700 Subject: [PATCH 6/8] ++ --- testing/skia_gold_client/lib/skia_gold_client.dart | 4 ---- 1 file changed, 4 deletions(-) diff --git a/testing/skia_gold_client/lib/skia_gold_client.dart b/testing/skia_gold_client/lib/skia_gold_client.dart index c7866c098fb04..5d64eb76423e1 100644 --- a/testing/skia_gold_client/lib/skia_gold_client.dart +++ b/testing/skia_gold_client/lib/skia_gold_client.dart @@ -189,10 +189,6 @@ interface class SkiaGoldClient { /// The path to the local [Directory] where the `goldctl` tool is hosted. String get _goldctl { - assert( - isAvailable(environment: _environment), - 'Trying to use `goldctl` in an environment where it is not available', - ); final String? result = _environment[_kGoldctlKey]; if (result == null || result.isEmpty) { throw StateError('The environment variable $_kGoldctlKey is not set.'); From 25cb261a33cea93ad2581a2c5bea80e88d8f4378 Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Tue, 3 Sep 2024 14:33:38 -0700 Subject: [PATCH 7/8] Tweak clang_tidy_test. --- tools/clang_tidy/test/clang_tidy_test.dart | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/tools/clang_tidy/test/clang_tidy_test.dart b/tools/clang_tidy/test/clang_tidy_test.dart index c0f6d19012ca0..f00833841f9d2 100644 --- a/tools/clang_tidy/test/clang_tidy_test.dart +++ b/tools/clang_tidy/test/clang_tidy_test.dart @@ -108,26 +108,22 @@ final _engineRoot = Engine.findWithin(); void main() { // This test requires a compile_commands.json file to exist. // - // On CI, we'll want to provide exactly which build to use, i.e.: + // We can provide exactly which build to use, i.e.: // COMPILE_COMMANDS_PATH=/path/to/compile_commands.json dart test // - // Locally, we can fall back to the latest build output if one isn't provided. - // - // Otherwise, fail. + // Or, we can fall back to the latest build output if one isn't provided. final String buildCommands; if (io.Platform.environment['COMPILE_COMMANDS_PATH'] case final String compileCommandsPath) { buildCommands = compileCommandsPath; - } else if (io.Platform.environment['LUCI_CONTEXT'] == null) { + } else { final String? inferredPath = _engineRoot.latestOutput()?.compileCommandsJson.path; io.stderr.writeln('No COMPILE_COMMANDS_PATH found in environment.'); if (inferredPath != null) { - io.stderr.writeln('Since this is a local run, inferring the last build output: $inferredPath'); + io.stderr.writeln('Inferring the last build output: $inferredPath'); buildCommands = inferredPath; } else { fail('No outputs or build commands found.'); } - } else { - fail('No COMPILE_COMMANDS_PATH found in environment.'); } test('--help gives help, and uses host_debug by default outside of an engine root', () async { From 755cfb14ebbef0fc831f28e2beafb11b2f2be971 Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Wed, 4 Sep 2024 12:38:50 -0700 Subject: [PATCH 8/8] Remove fake matchers. --- .../test/golden_tests_harvester_test.dart | 134 ++++++++++-------- 1 file changed, 77 insertions(+), 57 deletions(-) diff --git a/tools/golden_tests_harvester/test/golden_tests_harvester_test.dart b/tools/golden_tests_harvester/test/golden_tests_harvester_test.dart index 0a12b8d87483b..9831ec64b4ede 100644 --- a/tools/golden_tests_harvester/test/golden_tests_harvester_test.dart +++ b/tools/golden_tests_harvester/test/golden_tests_harvester_test.dart @@ -24,13 +24,20 @@ void main() async { test('should fail on a missing directory', () async { await withTempDirectory((io.Directory tempDirectory) async { final StringSink stderr = StringBuffer(); - final ArgumentError error = await _expectThrow(() async { - await Harvester.create( + expect( + () async { + await Harvester.create( io.Directory(p.join(tempDirectory.path, 'non_existent')), stderr, - addImageToSkiaGold: _alwaysThrowsAddImg); - }); - expect(error.message, contains('non_existent')); + addImageToSkiaGold: _alwaysThrowsAddImg, + ); + }, + throwsA(isA().having( + (error) => error.message, + 'message', + contains('non_existent'), + )), + ); expect(stderr.toString(), isEmpty); }); }); @@ -39,14 +46,22 @@ void main() async { () async { await withTempDirectory((io.Directory tempDirectory) async { final StringSink stderr = StringBuffer(); - - final StateError error = await _expectThrow(() async { - await Harvester.create( + await expectLater( + () async { + await Harvester.create( tempDirectory, stderr, - addImageToSkiaGold: _alwaysThrowsAddImg); - }); - expect(error.toString(), contains('digest.json')); + addImageToSkiaGold: _alwaysThrowsAddImg, + ); + }, + throwsA( + isA().having( + (error) => error.toString(), + 'toString()', + contains('digest.json'), + ), + ), + ); expect(stderr.toString(), isEmpty); }); }); @@ -58,15 +73,22 @@ void main() async { io.File(p.join(tempDirectory.path, 'digest.json')); await digestsFile .writeAsString('{"dimensions": "not a map", "entries": []}'); - - final FormatException error = - await _expectThrow(() async { - await Harvester.create( + await expectLater( + () async { + await Harvester.create( tempDirectory, stderr, - addImageToSkiaGold: _alwaysThrowsAddImg); - }); - expect(error.message, contains('dimensions')); + addImageToSkiaGold: _alwaysThrowsAddImg, + ); + }, + throwsA( + isA().having( + (error) => error.message, + 'message', + contains('dimensions'), + ), + ), + ); expect(stderr.toString(), isEmpty); }); }); @@ -77,30 +99,37 @@ void main() async { io.File(p.join(tempDirectory.path, 'digest.json')); final StringSink stderr = StringBuffer(); await digestsFile.writeAsString(''' + { + "dimensions": {}, + "entries": [ { - "dimensions": {}, - "entries": [ - { - "filename": "test_name_1.png", - "width": 100, - "height": 100, - "maxDiffPixelsPercent": 0.01, - "maxColorDelta": 0 - } - ] + "filename": "test_name_1.png", + "width": 100, + "height": 100, + "maxDiffPixelsPercent": 0.01, + "maxColorDelta": 0 } + ] + } '''); - final FailedComparisonException error = - await _expectThrow(() async { - final Harvester harvester = await Harvester.create( - tempDirectory, - stderr, - addImageToSkiaGold: _alwaysThrowsAddImg); - await harvest(harvester); - }); - expect(error.testName, 'test_name_1.png'); - expect(stderr.toString(), contains('IntentionalError')); + final Harvester harvester = await Harvester.create( + tempDirectory, + stderr, + addImageToSkiaGold: _alwaysThrowsAddImg, + ); + expect( + () => harvest(harvester), + throwsA( + isA() + .having((e) => e.testName, 'testName', 'test_name_1.png') + .having( + (e) => e.toString(), + 'toString()', + contains('Failed comparison: test_name_1.png'), + ), + ), + ); }); }); @@ -186,30 +215,21 @@ void main() async { ]} '''); final Harvester harvester = await Harvester.create(tempDirectory, stderr); - final StateError error = await _expectThrow(() async { - await harvest(harvester); - }); - expect(error.message, contains('GOLDCTL')); + expect( + () => harvest(harvester), + throwsA( + isA().having( + (t) => t.message, + 'message', + contains('GOLDCTL'), + ), + ), + ); expect(stderr.toString(), isEmpty); }); }); } - -FutureOr _expectThrow( - FutureOr Function() callback) async { - try { - await callback(); - fail('Expected an exception of type $T'); - } on T catch (e) { - return e; - } catch (e) { - fail('Expected an exception of type $T, but got $e'); - } - // fail(...) unfortunately does not return Never, but it does always throw. - throw UnsupportedError('Unreachable'); -} - final class _IntentionalError extends Error {} Future _alwaysThrowsAddImg(