diff --git a/engine/src/flutter/testing/run_tests.py b/engine/src/flutter/testing/run_tests.py index a70e950d409..4658f007c00 100755 --- a/engine/src/flutter/testing/run_tests.py +++ b/engine/src/flutter/testing/run_tests.py @@ -12,7 +12,6 @@ from pathlib import Path import argparse import errno -from functools import reduce import glob import logging import logging.handlers @@ -1013,27 +1012,6 @@ class DirectoryChange(): os.chdir(self.old_cwd) -def generate_dir_listing(dir_path: str) -> str: - listing = os.listdir(dir_path) - listing.sort() - return reduce(lambda a, b: a + '\n' + b, listing) - - -def str_replace_range(instr: str, start: int, end: int, replacement: str) -> str: - return instr[:start] + replacement + instr[end:] - - -def redirect_patch(patch: str) -> str: - 'Makes a diff point its output file to its input file.' - input_path = re.search(r'^--- a(.*)', patch, re.MULTILINE) - output_path = re.search(r'^\+\+\+ b(.*)', patch, re.MULTILINE) - return str_replace_range( - patch, - output_path.span(1)[0], - output_path.span(1)[1], input_path.group(1) - ) - - def run_impeller_golden_tests(build_dir: str): """ Executes the impeller golden image tests from in the `variant` build. @@ -1047,25 +1025,20 @@ def run_impeller_golden_tests(build_dir: str): harvester_path: Path = Path(SCRIPT_DIR).parent.joinpath('tools' ).joinpath('golden_tests_harvester') with tempfile.TemporaryDirectory(prefix='impeller_golden') as temp_dir: - run_cmd([tests_path, '--working_dir=%s' % temp_dir], cwd=build_dir) - with tempfile.NamedTemporaryFile(mode='w', - prefix='impeller_golden_tests_output') as dir_listing_file: - dir_listing = generate_dir_listing(temp_dir) - dir_listing_file.write(dir_listing) - golden_path = os.path.join('testing', 'impeller_golden_tests_output.txt') - diff_result = subprocess.run( - f'git diff -p {golden_path} {dir_listing_file.name}', - check=False, - shell=True, - stdout=subprocess.PIPE, - cwd=os.path.join(BUILDROOT_DIR, 'flutter') - ) - if diff_result.returncode != 0: - print_divider('<') - print(f'Unexpected diff in {golden_path}, use `git apply` with the following patch.') - print('') - print(redirect_patch(diff_result.stdout.decode())) - raise RuntimeError('impeller_golden_tests diff failure') + run_cmd([tests_path, f'--working_dir={temp_dir}'], cwd=build_dir) + golden_path = os.path.join('testing', 'impeller_golden_tests_output.txt') + script_path = os.path.join('tools', 'dir_contents_diff', 'bin', 'dir_contents_diff.dart') + diff_result = subprocess.run( + f'dart run {script_path} {golden_path} {temp_dir}', + check=False, + shell=True, + stdout=subprocess.PIPE, + cwd=os.path.join(BUILDROOT_DIR, 'flutter') + ) + if diff_result.returncode != 0: + print_divider('<') + print(diff_result.stdout.decode()) + raise RuntimeError('impeller_golden_tests diff failure') with DirectoryChange(harvester_path): run_cmd(['dart', 'pub', 'get']) diff --git a/engine/src/flutter/testing/scenario_app/README.md b/engine/src/flutter/testing/scenario_app/README.md index 470eb320fb1..8d866ce01b3 100644 --- a/engine/src/flutter/testing/scenario_app/README.md +++ b/engine/src/flutter/testing/scenario_app/README.md @@ -41,3 +41,10 @@ viewport. Then set the scenario from the Android or iOS app by calling `set_scenario` on platform channel `driver`. + +## Output validation + +When using `//flutter/testing/scenario_app/run_android_tests.sh` the generated +output will be checked against a golden file at +`//flutter/testing/scenario_app_android_output.txt` to make sure all output was +generated. A patch will be printed to stdout if they don't match. diff --git a/engine/src/flutter/testing/scenario_app/bin/android_integration_tests.dart b/engine/src/flutter/testing/scenario_app/bin/android_integration_tests.dart index 68e4f500943..94d0cfc64b6 100644 --- a/engine/src/flutter/testing/scenario_app/bin/android_integration_tests.dart +++ b/engine/src/flutter/testing/scenario_app/bin/android_integration_tests.dart @@ -8,6 +8,7 @@ import 'dart:io'; import 'dart:typed_data'; import 'package:args/args.dart'; +import 'package:dir_contents_diff/dir_contents_diff.dart' show dirContentsDiff; import 'package:engine_repo_tools/engine_repo_tools.dart'; import 'package:path/path.dart'; import 'package:process/process.dart'; @@ -17,6 +18,17 @@ import 'utils/logs.dart'; import 'utils/process_manager_extension.dart'; import 'utils/screenshot_transformer.dart'; +void _withTemporaryCwd(String path, void Function() callback) { + final String originalCwd = Directory.current.path; + Directory.current = Directory(path).parent.path; + + try { + callback(); + } finally { + Directory.current = originalCwd; + } +} + // If you update the arguments, update the documentation in the README.md file. void main(List args) async { final Engine? engine = Engine.tryFindWithin(); @@ -61,6 +73,9 @@ void main(List args) async { 'enable-impeller', help: 'Enable Impeller for the Android app.', ) + ..addOption('output-contents-golden', + help: + 'Path to a file that will be used to check the contents of the output to make sure everything was created.') ..addOption( 'impeller-backend', help: 'The Impeller backend to use for the Android app.', @@ -88,6 +103,7 @@ void main(List args) async { final bool useSkiaGold = results['use-skia-gold'] as bool; final String? smokeTest = results['smoke-test'] as String?; final bool enableImpeller = results['enable-impeller'] as bool; + final String? contentsGolden = results['output-contents-golden'] as String?; final _ImpellerBackend? impellerBackend = _ImpellerBackend.tryParse(results['impeller-backend'] as String?); if (enableImpeller && impellerBackend == null) { panic(['invalid graphics-backend', results['impeller-backend'] as String? ?? '']); @@ -99,6 +115,7 @@ void main(List args) async { useSkiaGold: useSkiaGold, enableImpeller: enableImpeller, impellerBackend: impellerBackend, + contentsGolden: contentsGolden, ); exit(0); }, @@ -135,6 +152,7 @@ Future _run({ required bool useSkiaGold, required bool enableImpeller, required _ImpellerBackend? impellerBackend, + required String? contentsGolden, }) async { const ProcessManager pm = LocalProcessManager(); @@ -207,7 +225,7 @@ Future _run({ final IOSink logcat = File(logcatPath).openWrite(); try { - await step('Creating screenshot directory...', () async { + await step('Creating screenshot directory `$screenshotPath`...', () async { Directory(screenshotPath).createSync(recursive: true); }); @@ -353,6 +371,19 @@ Future _run({ await Future.wait(pendingComparisons); }); + if (contentsGolden != null) { + // Check the output here. + await step('Check output files...', () async { + // TODO(gaaclarke): We should move this into dir_contents_diff. + _withTemporaryCwd(contentsGolden, () { + final int exitCode = dirContentsDiff(basename(contentsGolden), screenshotPath); + if (exitCode != 0) { + panic(['Output contents incorrect.']); + } + }); + }); + } + await step('Flush logcat...', () async { await logcat.flush(); await logcat.close(); diff --git a/engine/src/flutter/testing/scenario_app/pubspec.yaml b/engine/src/flutter/testing/scenario_app/pubspec.yaml index 48df1bb67f2..3da23e05a4b 100644 --- a/engine/src/flutter/testing/scenario_app/pubspec.yaml +++ b/engine/src/flutter/testing/scenario_app/pubspec.yaml @@ -16,6 +16,7 @@ environment: # relative to this directory into //third_party/dart, or //third_party/pkg dependencies: args: any + dir_contents_diff: any engine_repo_tools: any path: any process: any @@ -30,6 +31,8 @@ dependency_overrides: path: ../../../third_party/dart/third_party/pkg/collection crypto: path: ../../../third_party/dart/third_party/pkg/crypto + dir_contents_diff: + path: ../../tools/dir_contents_diff engine_repo_tools: path: ../../tools/pkg/engine_repo_tools file: diff --git a/engine/src/flutter/testing/scenario_app/run_android_tests.sh b/engine/src/flutter/testing/scenario_app/run_android_tests.sh index 4a808f72c0c..6287bff06e2 100755 --- a/engine/src/flutter/testing/scenario_app/run_android_tests.sh +++ b/engine/src/flutter/testing/scenario_app/run_android_tests.sh @@ -9,6 +9,12 @@ set -e +# Check number of args. +if [ $# -lt 1 ]; then + echo "Usage: $0 [flags*]" + exit 1 +fi + # Needed because if it is set, cd may print the path it changed to. unset CDPATH @@ -38,6 +44,7 @@ SRC_DIR="$( pwd -P )" OUT_DIR="$SRC_DIR/out/$BUILD_VARIANT" +CONTENTS_GOLDEN="$SRC_DIR/flutter/testing/scenario_app_android_output.txt" # Dump the logcat and symbolize stack traces before exiting. function dumpLogcat { @@ -68,4 +75,5 @@ cd $SCRIPT_DIR "$SRC_DIR"/third_party/dart/tools/sdks/dart-sdk/bin/dart run \ "$SCRIPT_DIR"/bin/android_integration_tests.dart \ --out-dir="$OUT_DIR" \ + --output-contents-golden="$CONTENTS_GOLDEN" \ "$@" diff --git a/engine/src/flutter/testing/scenario_app_android_output.txt b/engine/src/flutter/testing/scenario_app_android_output.txt new file mode 100644 index 00000000000..315e37c9231 --- /dev/null +++ b/engine/src/flutter/testing/scenario_app_android_output.txt @@ -0,0 +1,60 @@ +ExternalTextureTests_testCanvasSurface.png +ExternalTextureTests_testCroppedMediaSurface_bottomLeft.png +ExternalTextureTests_testCroppedMediaSurface_topRight.png +ExternalTextureTests_testCroppedRotatedMediaSurface_bottomLeft_90.png +ExternalTextureTests_testMediaSurface.png +ExternalTextureTests_testRotatedMediaSurface_180.png +ExternalTextureTests_testRotatedMediaSurface_270.png +ExternalTextureTests_testRotatedMediaSurface_90.png +PlatformTextureUiTests_testPlatformView.png +PlatformTextureUiTests_testPlatformViewClippath.png +PlatformTextureUiTests_testPlatformViewCliprect.png +PlatformTextureUiTests_testPlatformViewCliprrect.png +PlatformTextureUiTests_testPlatformViewMultiple.png +PlatformTextureUiTests_testPlatformViewMultipleBackgroundForeground.png +PlatformTextureUiTests_testPlatformViewMultipleWithoutOverlays.png +PlatformTextureUiTests_testPlatformViewOpacity.png +PlatformTextureUiTests_testPlatformViewRotate.png +PlatformTextureUiTests_testPlatformViewTransform.png +PlatformTextureUiTests_testPlatformViewTwoIntersectingOverlays.png +PlatformTextureUiTests_testPlatformViewWithoutOverlayIntersection.png +PlatformViewUiTests_testPlatformView.png +PlatformViewUiTests_testPlatformViewClippath.png +PlatformViewUiTests_testPlatformViewCliprect.png +PlatformViewUiTests_testPlatformViewCliprrect.png +PlatformViewUiTests_testPlatformViewMultiple.png +PlatformViewUiTests_testPlatformViewMultipleBackgroundForeground.png +PlatformViewUiTests_testPlatformViewMultipleWithoutOverlays.png +PlatformViewUiTests_testPlatformViewOpacity.png +PlatformViewUiTests_testPlatformViewRotate.png +PlatformViewUiTests_testPlatformViewTransform.png +PlatformViewUiTests_testPlatformViewTwoIntersectingOverlays.png +PlatformViewUiTests_testPlatformViewWithoutOverlayIntersection.png +PlatformViewWithSurfaceViewBadContextUiTest_testPlatformView.png +PlatformViewWithSurfaceViewHybridFallbackUiTest_testPlatformView.png +PlatformViewWithSurfaceViewHybridUiTest_testPlatformView.png +PlatformViewWithSurfaceViewUiTest_testPlatformView.png +PlatformViewWithSurfaceViewUiTest_testPlatformViewClippath.png +PlatformViewWithSurfaceViewUiTest_testPlatformViewCliprect.png +PlatformViewWithSurfaceViewUiTest_testPlatformViewCliprrect.png +PlatformViewWithSurfaceViewUiTest_testPlatformViewMultiple.png +PlatformViewWithSurfaceViewUiTest_testPlatformViewMultipleBackgroundForeground.png +PlatformViewWithSurfaceViewUiTest_testPlatformViewMultipleWithoutOverlays.png +PlatformViewWithSurfaceViewUiTest_testPlatformViewOpacity.png +PlatformViewWithSurfaceViewUiTest_testPlatformViewRotate.png +PlatformViewWithSurfaceViewUiTest_testPlatformViewTransform.png +PlatformViewWithSurfaceViewUiTest_testPlatformViewTwoIntersectingOverlays.png +PlatformViewWithSurfaceViewUiTest_testPlatformViewWithoutOverlayIntersection.png +PlatformViewWithTextureViewUiTest_testPlatformView.png +PlatformViewWithTextureViewUiTest_testPlatformViewClippath.png +PlatformViewWithTextureViewUiTest_testPlatformViewCliprect.png +PlatformViewWithTextureViewUiTest_testPlatformViewCliprrect.png +PlatformViewWithTextureViewUiTest_testPlatformViewMultiple.png +PlatformViewWithTextureViewUiTest_testPlatformViewMultipleBackgroundForeground.png +PlatformViewWithTextureViewUiTest_testPlatformViewMultipleWithoutOverlays.png +PlatformViewWithTextureViewUiTest_testPlatformViewOpacity.png +PlatformViewWithTextureViewUiTest_testPlatformViewRotate.png +PlatformViewWithTextureViewUiTest_testPlatformViewTransform.png +PlatformViewWithTextureViewUiTest_testPlatformViewTwoIntersectingOverlays.png +PlatformViewWithTextureViewUiTest_testPlatformViewWithoutOverlayIntersection.png +SpawnEngineTests_testSpawnedEngine.png \ No newline at end of file diff --git a/engine/src/flutter/tools/dir_contents_diff/README.md b/engine/src/flutter/tools/dir_contents_diff/README.md new file mode 100644 index 00000000000..2eddf23a43f --- /dev/null +++ b/engine/src/flutter/tools/dir_contents_diff/README.md @@ -0,0 +1,12 @@ +# dir_contents_diff + +This tool will compare the contents of a directory to a file that lists the +contents of the directory, printing out a patch to apply if they differ. + +The exit code is 0 if there is no difference. + +## Usage + +```sh +dart run ./bin/dir_contents_diff.dart +``` diff --git a/engine/src/flutter/tools/dir_contents_diff/bin/dir_contents_diff.dart b/engine/src/flutter/tools/dir_contents_diff/bin/dir_contents_diff.dart new file mode 100644 index 00000000000..22273291b3a --- /dev/null +++ b/engine/src/flutter/tools/dir_contents_diff/bin/dir_contents_diff.dart @@ -0,0 +1,11 @@ +// 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' show exitCode; + +import 'package:dir_contents_diff/dir_contents_diff.dart'; + +void main(List args) { + exitCode = run(args); +} diff --git a/engine/src/flutter/tools/dir_contents_diff/lib/dir_contents_diff.dart b/engine/src/flutter/tools/dir_contents_diff/lib/dir_contents_diff.dart new file mode 100644 index 00000000000..49b485cd391 --- /dev/null +++ b/engine/src/flutter/tools/dir_contents_diff/lib/dir_contents_diff.dart @@ -0,0 +1,96 @@ +// 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:convert' show utf8; +import 'dart:io'; + +String _basename(String path) { + return path.split(Platform.pathSeparator).last; +} + +String _generateDirListing(String dirPath) { + final Directory dir = Directory(dirPath); + final List entities = dir.listSync(); + entities.sort( + (FileSystemEntity a, FileSystemEntity b) => a.path.compareTo(b.path)); + return entities + .map((FileSystemEntity entity) => _basename(entity.path)) + .join('\n'); +} + +String _strReplaceRange( + String inputStr, int start, int end, String replacement) { + return inputStr.substring(0, start) + replacement + inputStr.substring(end); +} + +String _redirectPatch(String patch) { + final RegExp inputPathExp = RegExp(r'^--- a(.*)', multiLine: true); + final RegExp outputPathExp = RegExp(r'^\+\+\+ b(.*)', multiLine: true); + + final Match? inputPathMatch = inputPathExp.firstMatch(patch); + final Match? outputPathMatch = outputPathExp.firstMatch(patch); + + assert(inputPathMatch != null); + assert(outputPathMatch != null); + + if (inputPathMatch != null && outputPathMatch != null) { + return _strReplaceRange( + patch, + outputPathMatch.start + 5, // +5 to account for '+++ b' + outputPathMatch.end, + inputPathMatch.group(1)!, + ); + } + throw Exception('Unable to find input and output paths'); +} + +File _makeTempFile(String prefix) { + final Directory systemTempDir = Directory.systemTemp; + final String filename = '$prefix-${DateTime.now().millisecondsSinceEpoch}'; + final String path = '${systemTempDir.path}${Platform.pathSeparator}$filename'; + final File result = File(path); + result.createSync(); + return result; +} + +/// Run the diff of the contents of a directory at [dirPath] and the contents of +/// a file at [goldenPath]. Returns 0 if there is no diff. Be aware that the +/// CWD should be inside of the git repository for the patch to be correct. +int dirContentsDiff(String goldenPath, String dirPath) { + if (!File(goldenPath).existsSync()) { + throw Exception('unable to find `$goldenPath`'); + } + if (!Directory(dirPath).existsSync()) { + throw Exception('unable to find `$dirPath`'); + } + int result = 0; + final File tempFile = _makeTempFile('dir_contents_diff'); + try { + final String dirListing = _generateDirListing(dirPath); + tempFile.writeAsStringSync(dirListing); + final ProcessResult diffResult = Process.runSync( + 'git', ['diff', '-p', goldenPath, tempFile.path], + runInShell: true, stdoutEncoding: utf8); + if (diffResult.exitCode != 0) { + print( + 'Unexpected diff in $goldenPath, use `git apply` with the following patch.\n'); + print(_redirectPatch(diffResult.stdout as String)); + result = 1; + } + } finally { + tempFile.deleteSync(); + } + + return result; +} + +/// The main entrypoint for the program, returns `exitCode`. +int run(List args) { + if (args.length != 2) { + throw Exception('usage: '); + } + final String goldenPath = args[0]; + final String dirPath = args[1]; + return dirContentsDiff(goldenPath, dirPath); +} diff --git a/engine/src/flutter/tools/dir_contents_diff/pubspec.yaml b/engine/src/flutter/tools/dir_contents_diff/pubspec.yaml new file mode 100644 index 00000000000..852177e328a --- /dev/null +++ b/engine/src/flutter/tools/dir_contents_diff/pubspec.yaml @@ -0,0 +1,3 @@ +name: dir_contents_diff +environment: + sdk: '>=3.2.0-0 <4.0.0' diff --git a/engine/src/flutter/tools/dir_contents_diff/test/dir_contents_diff_test.dart b/engine/src/flutter/tools/dir_contents_diff/test/dir_contents_diff_test.dart new file mode 100644 index 00000000000..e69de29bb2d diff --git a/engine/src/flutter/tools/pub_get_offline.py b/engine/src/flutter/tools/pub_get_offline.py index e436dcad216..fe167ae1766 100644 --- a/engine/src/flutter/tools/pub_get_offline.py +++ b/engine/src/flutter/tools/pub_get_offline.py @@ -36,6 +36,7 @@ ALL_PACKAGES = [ os.path.join(ENGINE_DIR, 'tools', 'clang_tidy'), os.path.join(ENGINE_DIR, 'tools', 'compare_goldens'), os.path.join(ENGINE_DIR, 'tools', 'const_finder'), + os.path.join(ENGINE_DIR, 'tools', 'dir_contents_diff'), os.path.join(ENGINE_DIR, 'tools', 'engine_tool'), os.path.join(ENGINE_DIR, 'tools', 'gen_web_locale_keymap'), os.path.join(ENGINE_DIR, 'tools', 'githooks'),