From 84cef7bf8aa80a9a35490d3208b5a53994f7b16a Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Fri, 23 Feb 2024 16:17:45 -0800 Subject: [PATCH 1/7] Actually list all sources for scenario_app target. --- testing/scenario_app/android/BUILD.gn | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/testing/scenario_app/android/BUILD.gn b/testing/scenario_app/android/BUILD.gn index 111b7be100ce0..9aabbdc72adaa 100644 --- a/testing/scenario_app/android/BUILD.gn +++ b/testing/scenario_app/android/BUILD.gn @@ -9,6 +9,7 @@ _android_sources = [ "app/src/androidTest/java/dev/flutter/TestRunner.java", "app/src/androidTest/java/dev/flutter/scenarios/EngineLaunchE2ETest.java", "app/src/androidTest/java/dev/flutter/scenarios/ExampleInstrumentedTest.java", + "app/src/androidTest/java/dev/flutter/scenariosui/DrawSolidBlueScreenTest.java", "app/src/androidTest/java/dev/flutter/scenariosui/ExternalTextureTests.java", "app/src/androidTest/java/dev/flutter/scenariosui/GetBitmapTests.java", "app/src/androidTest/java/dev/flutter/scenariosui/MemoryLeakTests.java", @@ -23,6 +24,8 @@ _android_sources = [ "app/src/androidTest/java/dev/flutter/scenariosui/SpawnEngineTests.java", "app/src/androidTest/java/dev/flutter/scenariosui/SpawnMultiEngineTest.java", "app/src/main/AndroidManifest.xml", + "app/src/main/assets/sample.mp4", + "app/src/main/java/dev/flutter/scenarios/ExternalTextureFlutterActivity.java", "app/src/main/java/dev/flutter/scenarios/GetBitmapActivity.java", "app/src/main/java/dev/flutter/scenarios/PlatformViewsActivity.java", "app/src/main/java/dev/flutter/scenarios/SpawnMultiEngineActivity.java", @@ -33,6 +36,10 @@ _android_sources = [ "app/src/main/java/dev/flutter/scenarios/TestableFlutterActivity.java", "app/src/main/java/dev/flutter/scenarios/TextPlatformViewFactory.java", "app/src/main/java/dev/flutter/scenarios/TexturePlatformViewFactory.java", + "app/src/main/res/values/colors.xml", + "app/src/main/res/values/styles.xml", + "app/src/main/res/xml/data_extraction_rules.xml", + "app/src/main/res/xml/extraction_config_11_and_below.xml", "build.gradle", ] From 794caf7c5e7148d33499ec806c5a44a9f5acc1bb Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Fri, 23 Feb 2024 18:43:50 -0800 Subject: [PATCH 2/7] Start work on better filtering. --- .../scenario_app/bin/run_android_tests.dart | 186 ++++++++++-------- .../bin/utils/adb_logcat_filtering.dart | 91 ++++++++- testing/scenario_app/bin/utils/logs.dart | 9 + testing/scenario_app/tool/logcat_reader.dart | 43 ++++ 4 files changed, 233 insertions(+), 96 deletions(-) create mode 100644 testing/scenario_app/tool/logcat_reader.dart diff --git a/testing/scenario_app/bin/run_android_tests.dart b/testing/scenario_app/bin/run_android_tests.dart index f0ad8276b0465..d58f7561792cc 100644 --- a/testing/scenario_app/bin/run_android_tests.dart +++ b/testing/scenario_app/bin/run_android_tests.dart @@ -47,48 +47,52 @@ void main(List args) async { ..addOption( 'adb', help: 'Path to the adb tool', - defaultsTo: engine != null ? join( - engine.srcDir.path, - 'third_party', - 'android_tools', - 'sdk', - 'platform-tools', - 'adb', - ) : null, + defaultsTo: engine != null + ? join( + engine.srcDir.path, + 'third_party', + 'android_tools', + 'sdk', + 'platform-tools', + 'adb', + ) + : null, ) ..addOption( 'ndk-stack', help: 'Path to the ndk-stack tool', - defaultsTo: engine != null ? join( - engine.srcDir.path, - 'third_party', - 'android_tools', - 'ndk', - 'prebuilt', - () { - if (Platform.isLinux) { - return 'linux-x86_64'; - } else if (Platform.isMacOS) { - return 'darwin-x86_64'; - } else if (Platform.isWindows) { - return 'windows-x86_64'; - } else { - throw UnsupportedError('Unsupported platform: ${Platform.operatingSystem}'); - } - }(), - 'bin', - 'ndk-stack', - ) : null, + defaultsTo: engine != null + ? join( + engine.srcDir.path, + 'third_party', + 'android_tools', + 'ndk', + 'prebuilt', + () { + if (Platform.isLinux) { + return 'linux-x86_64'; + } else if (Platform.isMacOS) { + return 'darwin-x86_64'; + } else if (Platform.isWindows) { + return 'windows-x86_64'; + } else { + throw UnsupportedError('Unsupported platform: ${Platform.operatingSystem}'); + } + }(), + 'bin', + 'ndk-stack', + ) + : null, ) ..addOption( 'out-dir', help: 'Out directory', - defaultsTo: - engine?. - outputs(). - where((Output o) => basename(o.path.path).startsWith('android_')). - firstOrNull?. - path.path, + defaultsTo: engine + ?.outputs() + .where((Output o) => basename(o.path.path).startsWith('android_')) + .firstOrNull + ?.path + .path, ) ..addOption( 'smoke-test', @@ -105,15 +109,18 @@ void main(List args) async { ) ..addOption( 'output-contents-golden', - help: 'Path to a file that contains the expected filenames of golden files.', - defaultsTo: engine != null ? join( - engine.srcDir.path, - 'flutter', - 'testing', - 'scenario_app', - 'android', - 'expected_golden_output.txt', - ) : null, + help: + 'Path to a file that contains the expected filenames of golden files.', + defaultsTo: engine != null + ? join( + engine.srcDir.path, + 'flutter', + 'testing', + 'scenario_app', + 'android', + 'expected_golden_output.txt', + ) + : null, ) ..addOption( 'impeller-backend', @@ -124,8 +131,8 @@ void main(List args) async { ..addOption( 'logs-dir', help: 'The directory to store the logs and screenshots. Defaults to ' - 'the value of the FLUTTER_LOGS_DIR environment variable, if set, ' - 'otherwise it defaults to a path within out-dir.', + 'the value of the FLUTTER_LOGS_DIR environment variable, if set, ' + 'otherwise it defaults to a path within out-dir.', defaultsTo: Platform.environment['FLUTTER_LOGS_DIR'], ); @@ -153,7 +160,10 @@ void main(List args) async { 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? ?? '']); + panic([ + 'invalid graphics-backend', + results['impeller-backend'] as String? ?? '' + ]); } final Directory logsDir = Directory(results['logs-dir'] as String? ?? join(outDir.path, 'scenario_app', 'logs')); final String? ndkStack = results['ndk-stack'] as String?; @@ -215,7 +225,10 @@ Future _run({ const ProcessManager pm = LocalProcessManager(); if (!outDir.existsSync()) { - panic(['out-dir does not exist: $outDir', 'make sure to build the selected engine variant']); + panic([ + 'out-dir does not exist: $outDir', + 'make sure to build the selected engine variant' + ]); } if (!adb.existsSync()) { @@ -236,11 +249,17 @@ Future _run({ log('writing logs and screenshots to ${logsDir.path}'); if (!testApk.existsSync()) { - panic(['test apk does not exist: ${testApk.path}', 'make sure to build the selected engine variant']); + panic([ + 'test apk does not exist: ${testApk.path}', + 'make sure to build the selected engine variant' + ]); } if (!appApk.existsSync()) { - panic(['app apk does not exist: ${appApk.path}', 'make sure to build the selected engine variant']); + panic([ + 'app apk does not exist: ${appApk.path}', + 'make sure to build the selected engine variant' + ]); } // Start a TCP socket in the host, and forward it to the device that runs the tests. @@ -248,7 +267,7 @@ Future _run({ // for the screenshots. // On LUCI, the host uploads the screenshots to Skia Gold. SkiaGoldClient? skiaGoldClient; - late ServerSocket server; + late ServerSocket server; final List> pendingComparisons = >[]; await step('Starting server...', () async { server = await ServerSocket.bind(InternetAddress.anyIPv4, _tcpPort); @@ -259,7 +278,8 @@ Future _run({ if (verbose) { stdout.writeln('client connected ${client.remoteAddress.address}:${client.remotePort}'); } - client.transform(const ScreenshotBlobTransformer()).listen((Screenshot screenshot) { + client.transform(const ScreenshotBlobTransformer()).listen( + (Screenshot screenshot) { final String fileName = screenshot.filename; final Uint8List fileContent = screenshot.fileContent; if (verbose) { @@ -277,18 +297,15 @@ Future _run({ } if (isSkiaGoldClientAvailable) { final Future comparison = skiaGoldClient! - .addImg(fileName, goldenFile, - screenshotSize: screenshot.pixelCount) - .catchError((dynamic err) { - panic(['skia gold comparison failed: $err']); - }); + .addImg(fileName, goldenFile, screenshotSize: screenshot.pixelCount) + .catchError((dynamic err) { + panic(['skia gold comparison failed: $err']); + }); pendingComparisons.add(comparison); } - }, - onError: (dynamic err) { + }, onError: (dynamic err) { panic(['error while receiving bytes: $err']); - }, - cancelOnError: true); + }, cancelOnError: true); }); }); @@ -311,27 +328,26 @@ Future _run({ final (Future logcatExitCode, Stream logcatOutput) = getProcessStreams(logcatProcess); logcatProcessExitCode = logcatExitCode; + String? filterToProcessId; + logcatOutput.listen((String line) { // Always write to the full log. logcat.writeln(line); // Conditionally parse and write to stderr. final AdbLogLine? adbLogLine = AdbLogLine.tryParse(line); - switch (adbLogLine?.process) { - case null: - break; - case 'ActivityManager': - // These are mostly noise, i.e. "D ActivityManager: freezing 24632 com.blah". - if (adbLogLine!.severity == 'D') { - break; - } - // TODO(matanlurey): Figure out why this isn't 'flutter.scenario' or similar. - // Also, why is there two different names? - case 'utter.scenario': - case 'utter.scenarios': - case 'flutter': - case 'FlutterJNI': - log('[adb] $line'); + if (verbose || adbLogLine == null) { + log(line); + return; + } + + filterToProcessId ??= adbLogLine.tryParseProcess(); + if (filterToProcessId != null) { + // adbLogLine.printToStdout(verbose: verbose, filterToProcessId: filterToProcessId); + } + }, onError: (Object? err) { + if (verbose) { + logWarning('logcat stream error: $err'); } }); }); @@ -364,10 +380,7 @@ Future _run({ log('using dimensions: ${json.encode(dimensions)}'); skiaGoldClient = SkiaGoldClient( outDir, - dimensions: { - 'AndroidAPILevel': connectedDeviceAPILevel, - 'GraphicsBackend': enableImpeller ? 'impeller-${impellerBackend!.name}' : 'skia', - }, + dimensions: dimensions, ); }); @@ -412,11 +425,9 @@ Future _run({ 'am', 'instrument', '-w', - if (smokeTestFullPath != null) - '-e class $smokeTestFullPath', + if (smokeTestFullPath != null) '-e class $smokeTestFullPath', 'dev.flutter.scenarios.test/dev.flutter.TestRunner', - if (enableImpeller) - '-e enable-impeller', + if (enableImpeller) '-e enable-impeller', if (impellerBackend != null) '-e impeller-backend ${impellerBackend.name}', ]); @@ -465,7 +476,8 @@ Future _run({ final int exitCode = await pm.runAndForward([ adb.path, 'reverse', - '--remove', 'tcp:3000', + '--remove', + 'tcp:3000', ]); if (exitCode != 0) { panic(['could not unforward port']); @@ -473,14 +485,16 @@ Future _run({ }); await step('Uninstalling app APK...', () async { - final int exitCode = await pm.runAndForward([adb.path, 'uninstall', 'dev.flutter.scenarios']); + final int exitCode = await pm.runAndForward( + [adb.path, 'uninstall', 'dev.flutter.scenarios']); if (exitCode != 0) { panic(['could not uninstall app apk']); } }); await step('Uninstalling test APK...', () async { - final int exitCode = await pm.runAndForward([adb.path, 'uninstall', 'dev.flutter.scenarios.test']); + final int exitCode = await pm.runAndForward( + [adb.path, 'uninstall', 'dev.flutter.scenarios.test']); if (exitCode != 0) { panic(['could not uninstall app apk']); } diff --git a/testing/scenario_app/bin/utils/adb_logcat_filtering.dart b/testing/scenario_app/bin/utils/adb_logcat_filtering.dart index ca67ab17df46d..15750d54d3bd6 100644 --- a/testing/scenario_app/bin/utils/adb_logcat_filtering.dart +++ b/testing/scenario_app/bin/utils/adb_logcat_filtering.dart @@ -1,3 +1,7 @@ +// 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. + /// Some notes about filtering `adb logcat` output, especially as a result of /// running `adb shell` to instrument the app and test scripts, as it's /// non-trivial and error-prone. @@ -26,6 +30,8 @@ /// See also: . library; +import 'logs.dart'; + /// Represents a line of `adb logcat` output parsed into a structured form. /// /// For example the line: @@ -40,14 +46,15 @@ library; /// with lazy parsing. extension type const AdbLogLine._(Match _match) { // RegEx that parses into the following groups: - // 1. Everything up to the severity (I, W, E, etc.). - // In other words, any whitespace, numbers, hyphens, colons, and periods. - // 2. The severity (a single uppercase letter). - // 3. The name of the process (up to the colon). - // 4. The message (after the colon). + // 1. The time of the log message, such as `02-22 13:54:39.839`. + // 2. The process ID. + // 3. The thread ID. + // 4. The character representing the severity of the log message, such as `I`. + // 5. The tag, such as `ActivityManager`. + // 6. The actual log message. // // This regex is simple versus being more precise. Feel free to improve it. - static final RegExp _pattern = RegExp(r'([^A-Z]*)([A-Z])\s([^:]*)\:\s(.*)'); + static final RegExp _pattern = RegExp(r'(\d+-\d+\s[\d|:]+\.\d+)\s+(\d+)\s+(\d+)\s(\w)\s(\S+):\s*(.*)'); /// Parses the given [adbLogCatLine] into a structured form. /// @@ -57,15 +64,79 @@ extension type const AdbLogLine._(Match _match) { return match == null ? null : AdbLogLine._(match); } + /// Tries to parse the process that was started, if the log line is about it. + String? tryParseProcess() { + if (name == 'ActivityManager' && message.startsWith('Start proc')) { + // Start proc 6840:d + final RegExpMatch? match = RegExp(r'Start proc (\d+):').firstMatch(message); + return match?.group(1); + } + return null; + } + + /// Returns `true` if the log line is verbose. + bool isVerbose({String? filterToProcessId}) => !_isRelevant(filterToProcessId: filterToProcessId); + bool _isRelevant({String? filterToProcessId}) { + // Fatal errors are always useful. + if (severity == 'F') { + return true; + } + + // If a process ID is specified, only include logs from that process. + if (process == filterToProcessId) { + return true; + } + + // These are "known" tags useful for debugging. + if (const { + 'utter.scenario', + 'utter.scenarios', + 'TestRunner', + }.contains(name)) { + return true; + } + + // And... whatever, include anything with the word "flutter". + return name.toLowerCase().contains('flutter') || message.toLowerCase().contains('flutter'); + } + + /// Logs the line to the console. + void printToStdout({required bool verbose, String? filterToProcessId}) { + if (!verbose || isVerbose(filterToProcessId: filterToProcessId)) { + return; + } + if (severity == 'E' || severity == 'F') { + logWarning(line); + } else if (name == 'TestRunner') { + logImportant(line); + } else { + print('Got here verbose=$verbose, filterToProcessId=$filterToProcessId, this.name=$name'); + log(line); + } + } + /// The full line of `adb logcat` output. String get line => _match.group(0)!; + /// The time of the log message, such as `02-22 13:54:39.839`. + String get time => _match.group(1)!; + + /// The process ID. + String get process => _match.group(2)!; + + /// The thread ID. + String get thread => _match.group(3)!; + /// The character representing the severity of the log message, such as `I`. - String get severity => _match.group(2)!; + String get severity => _match.group(4)!; - /// The process name, such as `ActivityManager`. - String get process => _match.group(3)!; + /// The tag, such as `ActivityManager`. + String get name => _match.group(5)!; /// The actual log message. - String get message => _match.group(4)!; + String get message => _match.group(6)!; + + String toDebugString() { + return 'AdbLogLine(time: $time, process: $process, thread: $thread, severity: $severity, name: $name, message: $message)'; + } } diff --git a/testing/scenario_app/bin/utils/logs.dart b/testing/scenario_app/bin/utils/logs.dart index b84e8127f29fe..5610f1325c4d5 100644 --- a/testing/scenario_app/bin/utils/logs.dart +++ b/testing/scenario_app/bin/utils/logs.dart @@ -7,6 +7,7 @@ import 'dart:io'; bool _supportsAnsi = stdout.supportsAnsiEscapes; String _green = _supportsAnsi ? '\u001b[1;32m' : ''; String _red = _supportsAnsi ? '\u001b[31m' : ''; +String _yellow = _supportsAnsi ? '\u001b[33m' : ''; String _gray = _supportsAnsi ? '\u001b[90m' : ''; String _reset = _supportsAnsi? '\u001B[0m' : ''; @@ -26,6 +27,14 @@ void log(String msg) { stdout.writeln('$_gray$msg$_reset'); } +void logImportant(String msg) { + stdout.writeln(msg); +} + +void logWarning(String msg) { + stderr.writeln('$_yellow$msg$_reset'); +} + final class Panic extends Error {} Never panic(List messages) { diff --git a/testing/scenario_app/tool/logcat_reader.dart b/testing/scenario_app/tool/logcat_reader.dart new file mode 100644 index 0000000000000..fd2a09a9a867f --- /dev/null +++ b/testing/scenario_app/tool/logcat_reader.dart @@ -0,0 +1,43 @@ +// 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; + +// It's bad to import a file from `bin` into `tool`. +// However this tool is not very important, so delete it if necessary. +import '../bin/utils/adb_logcat_filtering.dart'; + +/// A tiny tool to read `adb logcat` output and perform some analysis. +/// +/// This tool is not meant to be a full-fledged logcat reader. It's just a +/// simple tool that uses the [AdbLogLine] extension type to parse results of +/// `adb logcat` and explain what log tag names are most common. +void main(List args) { + if (args case [final String path]) { + final List parsed = io.File(path) + .readAsLinesSync() + .map(AdbLogLine.tryParse) + .whereType() + // Filter out all debug logs. + .where((AdbLogLine line) => line.severity != 'D') + .toList(); + + final Map tagCounts = {}; + for (final AdbLogLine line in parsed) { + tagCounts[line.name] = (tagCounts[line.name] ?? 0) + 1; + } + + // Print in order of most common to least common. + final List> sorted = tagCounts.entries.toList() + ..sort((MapEntry a, MapEntry b) => b.value.compareTo(a.value)); + for (final MapEntry entry in sorted) { + print("'${entry.key}', // ${entry.value}"); + } + + return; + } + + print('Usage: logcat_reader.dart '); + io.exitCode = 1; +} From 22f83c3f83cf02d186071143f27b8a43df83fbca Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Fri, 23 Feb 2024 19:01:44 -0800 Subject: [PATCH 3/7] Finish filtering. --- .../scenario_app/bin/run_android_tests.dart | 2 +- .../bin/utils/adb_logcat_filtering.dart | 36 +++++++++++++------ 2 files changed, 27 insertions(+), 11 deletions(-) diff --git a/testing/scenario_app/bin/run_android_tests.dart b/testing/scenario_app/bin/run_android_tests.dart index d58f7561792cc..b01971b77d761 100644 --- a/testing/scenario_app/bin/run_android_tests.dart +++ b/testing/scenario_app/bin/run_android_tests.dart @@ -343,7 +343,7 @@ Future _run({ filterToProcessId ??= adbLogLine.tryParseProcess(); if (filterToProcessId != null) { - // adbLogLine.printToStdout(verbose: verbose, filterToProcessId: filterToProcessId); + adbLogLine.printFormatted(hideVerbose: !verbose, filterToProcessId: filterToProcessId); } }, onError: (Object? err) { if (verbose) { diff --git a/testing/scenario_app/bin/utils/adb_logcat_filtering.dart b/testing/scenario_app/bin/utils/adb_logcat_filtering.dart index 15750d54d3bd6..d00769f4765e1 100644 --- a/testing/scenario_app/bin/utils/adb_logcat_filtering.dart +++ b/testing/scenario_app/bin/utils/adb_logcat_filtering.dart @@ -54,7 +54,7 @@ extension type const AdbLogLine._(Match _match) { // 6. The actual log message. // // This regex is simple versus being more precise. Feel free to improve it. - static final RegExp _pattern = RegExp(r'(\d+-\d+\s[\d|:]+\.\d+)\s+(\d+)\s+(\d+)\s(\w)\s(\S+):\s*(.*)'); + static final RegExp _pattern = RegExp(r'(\d+-\d+\s[\d|:]+\.\d+)\s+(\d+)\s+(\d+)\s(\w)\s(\S+)\s*:\s*(.*)'); /// Parses the given [adbLogCatLine] into a structured form. /// @@ -82,9 +82,24 @@ extension type const AdbLogLine._(Match _match) { return true; } + // Debug logs are rarely useful. + if (severity == 'D') { + return false; + } + // If a process ID is specified, only include logs from that process. - if (process == filterToProcessId) { - return true; + if (filterToProcessId != null && process != filterToProcessId) { + return false; + } + + // These are "known" noise tags. + if (const { + 'MonitoringInstr', + 'ResourceExtractor', + 'THREAD_STATE', + 'ziparchive', + }.contains(name)) { + return false; } // These are "known" tags useful for debugging. @@ -101,17 +116,18 @@ extension type const AdbLogLine._(Match _match) { } /// Logs the line to the console. - void printToStdout({required bool verbose, String? filterToProcessId}) { - if (!verbose || isVerbose(filterToProcessId: filterToProcessId)) { + void printFormatted({required bool hideVerbose, String? filterToProcessId}) { + // If the line is verbose, only print it if the verbose flag is set. + if (hideVerbose && isVerbose(filterToProcessId: filterToProcessId)) { return; } - if (severity == 'E' || severity == 'F') { - logWarning(line); + final String formatted = '$time [$severity] $name: $message'; + if (severity == 'W' || severity == 'E' || severity == 'F') { + logWarning(formatted); } else if (name == 'TestRunner') { - logImportant(line); + logImportant(formatted); } else { - print('Got here verbose=$verbose, filterToProcessId=$filterToProcessId, this.name=$name'); - log(line); + log(formatted); } } From 68a5de93e0bb8e68b2c76b7d7798651d3ab887be Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Fri, 23 Feb 2024 19:02:01 -0800 Subject: [PATCH 4/7] ++ --- testing/scenario_app/bin/utils/adb_logcat_filtering.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testing/scenario_app/bin/utils/adb_logcat_filtering.dart b/testing/scenario_app/bin/utils/adb_logcat_filtering.dart index d00769f4765e1..b6d7fdf18b7d1 100644 --- a/testing/scenario_app/bin/utils/adb_logcat_filtering.dart +++ b/testing/scenario_app/bin/utils/adb_logcat_filtering.dart @@ -76,7 +76,7 @@ extension type const AdbLogLine._(Match _match) { /// Returns `true` if the log line is verbose. bool isVerbose({String? filterToProcessId}) => !_isRelevant(filterToProcessId: filterToProcessId); - bool _isRelevant({String? filterToProcessId}) { + bool _isRelevant({String? filterToProcessId}) { // Fatal errors are always useful. if (severity == 'F') { return true; From 2b16508707573e721d24ff0e1d3c08e70d9db486 Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Mon, 26 Feb 2024 09:30:45 -0800 Subject: [PATCH 5/7] ++ --- testing/scenario_app/bin/run_android_tests.dart | 5 ++--- testing/scenario_app/bin/utils/adb_logcat_filtering.dart | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/testing/scenario_app/bin/run_android_tests.dart b/testing/scenario_app/bin/run_android_tests.dart index b01971b77d761..93b06318740b1 100644 --- a/testing/scenario_app/bin/run_android_tests.dart +++ b/testing/scenario_app/bin/run_android_tests.dart @@ -109,8 +109,7 @@ void main(List args) async { ) ..addOption( 'output-contents-golden', - help: - 'Path to a file that contains the expected filenames of golden files.', + help: 'Path to a file that contains the expected filenames of golden files.', defaultsTo: engine != null ? join( engine.srcDir.path, @@ -343,7 +342,7 @@ Future _run({ filterToProcessId ??= adbLogLine.tryParseProcess(); if (filterToProcessId != null) { - adbLogLine.printFormatted(hideVerbose: !verbose, filterToProcessId: filterToProcessId); + adbLogLine.printFormatted(hideVerbose: !verbose, filterToProcessId: filterToProcessId!); } }, onError: (Object? err) { if (verbose) { diff --git a/testing/scenario_app/bin/utils/adb_logcat_filtering.dart b/testing/scenario_app/bin/utils/adb_logcat_filtering.dart index b6d7fdf18b7d1..b8f3ff2ea181d 100644 --- a/testing/scenario_app/bin/utils/adb_logcat_filtering.dart +++ b/testing/scenario_app/bin/utils/adb_logcat_filtering.dart @@ -116,7 +116,7 @@ extension type const AdbLogLine._(Match _match) { } /// Logs the line to the console. - void printFormatted({required bool hideVerbose, String? filterToProcessId}) { + void printFormatted({required bool hideVerbose, required String filterToProcessId}) { // If the line is verbose, only print it if the verbose flag is set. if (hideVerbose && isVerbose(filterToProcessId: filterToProcessId)) { return; From 34a5f3e0eb4ba108a441c2a585996e7b176b77a7 Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Mon, 26 Feb 2024 10:22:12 -0800 Subject: [PATCH 6/7] ++ --- .../scenario_app/bin/run_android_tests.dart | 20 +++++++++++++++---- .../bin/utils/adb_logcat_filtering.dart | 20 ++++++++----------- 2 files changed, 24 insertions(+), 16 deletions(-) diff --git a/testing/scenario_app/bin/run_android_tests.dart b/testing/scenario_app/bin/run_android_tests.dart index 93b06318740b1..38bc898bfe7e5 100644 --- a/testing/scenario_app/bin/run_android_tests.dart +++ b/testing/scenario_app/bin/run_android_tests.dart @@ -327,7 +327,7 @@ Future _run({ final (Future logcatExitCode, Stream logcatOutput) = getProcessStreams(logcatProcess); logcatProcessExitCode = logcatExitCode; - String? filterToProcessId; + String? filterProcessId; logcatOutput.listen((String line) { // Always write to the full log. @@ -340,10 +340,22 @@ Future _run({ return; } - filterToProcessId ??= adbLogLine.tryParseProcess(); - if (filterToProcessId != null) { - adbLogLine.printFormatted(hideVerbose: !verbose, filterToProcessId: filterToProcessId!); + // If we haven't already found a process ID, try to find one. + // The process ID will help us filter out logs from other processes. + filterProcessId ??= adbLogLine.tryParseProcess(); + + // If this is a "verbose" log, possibly skip it. + final bool isVerbose = adbLogLine.isVerbose(filterProcessId: filterProcessId); + if (isVerbose || filterProcessId == null) { + // We've requested verbose output, so print everything. + if (verbose) { + adbLogLine.printFormatted(); + } + return; } + + // It's a non-verbose log, so print it. + adbLogLine.printFormatted(); }, onError: (Object? err) { if (verbose) { logWarning('logcat stream error: $err'); diff --git a/testing/scenario_app/bin/utils/adb_logcat_filtering.dart b/testing/scenario_app/bin/utils/adb_logcat_filtering.dart index b8f3ff2ea181d..97852630c0a6a 100644 --- a/testing/scenario_app/bin/utils/adb_logcat_filtering.dart +++ b/testing/scenario_app/bin/utils/adb_logcat_filtering.dart @@ -75,8 +75,8 @@ extension type const AdbLogLine._(Match _match) { } /// Returns `true` if the log line is verbose. - bool isVerbose({String? filterToProcessId}) => !_isRelevant(filterToProcessId: filterToProcessId); - bool _isRelevant({String? filterToProcessId}) { + bool isVerbose({String? filterProcessId}) => !_isRelevant(filterProcessId: filterProcessId); + bool _isRelevant({String? filterProcessId}) { // Fatal errors are always useful. if (severity == 'F') { return true; @@ -87,11 +87,6 @@ extension type const AdbLogLine._(Match _match) { return false; } - // If a process ID is specified, only include logs from that process. - if (filterToProcessId != null && process != filterToProcessId) { - return false; - } - // These are "known" noise tags. if (const { 'MonitoringInstr', @@ -111,16 +106,17 @@ extension type const AdbLogLine._(Match _match) { return true; } + // If a process ID is specified, exclude logs _not_ from that process. + if (filterProcessId != null && process != filterProcessId) { + return false; + } + // And... whatever, include anything with the word "flutter". return name.toLowerCase().contains('flutter') || message.toLowerCase().contains('flutter'); } /// Logs the line to the console. - void printFormatted({required bool hideVerbose, required String filterToProcessId}) { - // If the line is verbose, only print it if the verbose flag is set. - if (hideVerbose && isVerbose(filterToProcessId: filterToProcessId)) { - return; - } + void printFormatted() { final String formatted = '$time [$severity] $name: $message'; if (severity == 'W' || severity == 'E' || severity == 'F') { logWarning(formatted); From 14f7bd96520c0a353bab50fa874cf33849f3f487 Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Mon, 26 Feb 2024 10:24:56 -0800 Subject: [PATCH 7/7] ++ --- testing/scenario_app/bin/run_android_tests.dart | 4 ++-- testing/scenario_app/bin/utils/logs.dart | 10 +++++++--- testing/scenario_app/tool/logcat_reader.dart | 2 +- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/testing/scenario_app/bin/run_android_tests.dart b/testing/scenario_app/bin/run_android_tests.dart index 38bc898bfe7e5..436cb421424ee 100644 --- a/testing/scenario_app/bin/run_android_tests.dart +++ b/testing/scenario_app/bin/run_android_tests.dart @@ -343,7 +343,7 @@ Future _run({ // If we haven't already found a process ID, try to find one. // The process ID will help us filter out logs from other processes. filterProcessId ??= adbLogLine.tryParseProcess(); - + // If this is a "verbose" log, possibly skip it. final bool isVerbose = adbLogLine.isVerbose(filterProcessId: filterProcessId); if (isVerbose || filterProcessId == null) { @@ -351,7 +351,7 @@ Future _run({ if (verbose) { adbLogLine.printFormatted(); } - return; + return; } // It's a non-verbose log, so print it. diff --git a/testing/scenario_app/bin/utils/logs.dart b/testing/scenario_app/bin/utils/logs.dart index 5610f1325c4d5..4d883d340e261 100644 --- a/testing/scenario_app/bin/utils/logs.dart +++ b/testing/scenario_app/bin/utils/logs.dart @@ -23,8 +23,12 @@ Future step(String msg, Future Function() fn) async { } } +void _logWithColor(String color, String msg) { + stdout.writeln('$color$msg$_reset'); +} + void log(String msg) { - stdout.writeln('$_gray$msg$_reset'); + _logWithColor(_gray, msg); } void logImportant(String msg) { @@ -32,14 +36,14 @@ void logImportant(String msg) { } void logWarning(String msg) { - stderr.writeln('$_yellow$msg$_reset'); + _logWithColor(_yellow, msg); } final class Panic extends Error {} Never panic(List messages) { for (final String message in messages) { - stderr.writeln('$_red$message$_reset'); + _logWithColor(_red, message); } throw Panic(); } diff --git a/testing/scenario_app/tool/logcat_reader.dart b/testing/scenario_app/tool/logcat_reader.dart index fd2a09a9a867f..909204a1af342 100644 --- a/testing/scenario_app/tool/logcat_reader.dart +++ b/testing/scenario_app/tool/logcat_reader.dart @@ -8,7 +8,7 @@ import 'dart:io' as io; // However this tool is not very important, so delete it if necessary. import '../bin/utils/adb_logcat_filtering.dart'; -/// A tiny tool to read `adb logcat` output and perform some analysis. +/// A tiny tool to read saved `adb logcat` output and perform some analysis. /// /// This tool is not meant to be a full-fledged logcat reader. It's just a /// simple tool that uses the [AdbLogLine] extension type to parse results of