From d61b342680c303e4a8479f242369f78201bf9c96 Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Thu, 29 Feb 2024 14:41:31 -0800 Subject: [PATCH 1/6] Add prefixing for LUCI logs dumps. --- .../scenario_app/bin/run_android_tests.dart | 78 +++++++++++++++++-- testing/scenario_app/bin/utils/options.dart | 8 ++ 2 files changed, 80 insertions(+), 6 deletions(-) diff --git a/testing/scenario_app/bin/run_android_tests.dart b/testing/scenario_app/bin/run_android_tests.dart index b85ad6436d79a..d3fcbcce24ff7 100644 --- a/testing/scenario_app/bin/run_android_tests.dart +++ b/testing/scenario_app/bin/run_android_tests.dart @@ -79,6 +79,7 @@ void main(List args) async { contentsGolden: options.outputContentsGolden, ndkStack: options.ndkStack, forceSurfaceProducerSurfaceTexture: options.forceSurfaceProducerSurfaceTexture, + prefixLogsPerRun: options.prefixLogsPerRun, ); onSigint.cancel(); exit(0); @@ -122,15 +123,22 @@ Future _run({ required String? contentsGolden, required String ndkStack, required bool forceSurfaceProducerSurfaceTexture, + required bool prefixLogsPerRun, }) async { const ProcessManager pm = LocalProcessManager(); final String scenarioAppPath = join(outDir.path, 'scenario_app'); - final String logcatPath = join(logsDir.path, 'logcat.txt'); - // TODO(matanlurey): Use screenshots/ sub-directory (https://github.com/flutter/flutter/issues/143604). - if (!logsDir.existsSync()) { - logsDir.createSync(recursive: true); - } + // Due to the CI environment, the logs directory persists between runs and + // even different builds. Because we're checking the output directory after + // each run, we need a clean logs directory to avoid false positives. + // + // Only after the runner is done, we can move the logs to the final location. + // + // See [_copyFiles] below and https://github.com/flutter/flutter/issues/144402. + final Directory finalLogsDir = logsDir..createSync(recursive: true); + logsDir = Directory.systemTemp.createTempSync('scenario_app_test_logs.'); + final String logcatPath = join(logsDir.path, 'logcat.txt'); + final String screenshotPath = logsDir.path; final String apkOutPath = join(scenarioAppPath, 'app', 'outputs', 'apk'); final File testApk = File(join(apkOutPath, 'androidTest', 'debug', 'app-debug-androidTest.apk')); @@ -388,6 +396,29 @@ Future _run({ await logcat.flush(); await logcat.close(); log('wrote logcat to $logcatPath'); + + // Copy the logs to the final location. + // Optionally prefix the logs with a run number and backend name. + // See https://github.com/flutter/flutter/issues/144402. + final StringBuffer prefix = StringBuffer(); + if (prefixLogsPerRun) { + final int rerunNumber = _getAndIncrementRerunNumber(finalLogsDir.path); + prefix.write('run_$rerunNumber.'); + if (enableImpeller) { + prefix.write('impeller_'); + } else { + prefix.write('skia_'); + } + if (impellerBackend != null) { + prefix.write(impellerBackend.name); + } + prefix.write('.'); + } + _copyFiles( + source: logsDir, + destination: finalLogsDir, + prefix: prefix.toString(), + ); }); if (enableImpeller) { @@ -448,7 +479,9 @@ Future _run({ await Future.wait(pendingComparisons); }); - if (contentsGolden != null) { + final bool allTestsRun = smokeTestFullPath == null; + final bool checkGoldens = contentsGolden != null; + if (allTestsRun && checkGoldens) { // Check the output here. await step('Check output files...', () async { // TODO(matanlurey): Resolve this in a better way. On CI this file always exists. @@ -476,3 +509,36 @@ void _withTemporaryCwd(String path, void Function() callback) { Directory.current = originalCwd; } } + +/// Reads the file named `reruns.txt` in the logs directory and returns the number of reruns. +/// +/// If the file does not exist, it is created with the number 1 and that number is returned. +int _getAndIncrementRerunNumber(String logsDir) { + final File rerunFile = File(join(logsDir, 'reruns.txt')); + if (!rerunFile.existsSync()) { + rerunFile.writeAsStringSync('1'); + return 1; + } + final int rerunNumber = int.parse(rerunFile.readAsStringSync()) + 1; + rerunFile.writeAsStringSync(rerunNumber.toString());; + return rerunNumber; +} + +/// Copies the contents of [source] to [destination], optionally adding a [prefix] to the destination path. +/// +/// This function is used to copy the screenshots from the device to the logs directory. +void _copyFiles({ + required Directory source, + required Directory destination, + String prefix = '', +}) { + for (final FileSystemEntity entity in source.listSync(recursive: true)) { + if (entity is File) { + final String relativePath = relative(entity.path, from: source.path); + final String destinationPath = join(destination.path, prefix, relativePath); + final File destinationFile = File(destinationPath); + destinationFile.createSync(recursive: true); + destinationFile.writeAsBytesSync(entity.readAsBytesSync()); + } + } +} diff --git a/testing/scenario_app/bin/utils/options.dart b/testing/scenario_app/bin/utils/options.dart index 43cb0e6bed5d3..92eed1e136ae7 100644 --- a/testing/scenario_app/bin/utils/options.dart +++ b/testing/scenario_app/bin/utils/options.dart @@ -159,6 +159,11 @@ extension type const Options._(ArgResults _args) { 'https://github.com/flutter/flutter/issues/143539 for details.', negatable: false ) + ..addFlag( + 'prefix-logs-per-run', + help: 'Whether to prefix logs with a per-run unique identifier.', + defaultsTo: environment.isCi, + ) ..addOption( 'impeller-backend', help: 'The graphics backend to use when --enable-impeller is true. ' @@ -314,4 +319,7 @@ extension type const Options._(ArgResults _args) { } return _args['force-surface-producer-surface-texture'] as bool; } + + /// Whether to prefix logs with a per-run unique identifier. + bool get prefixLogsPerRun => _args['prefix-logs-per-run'] as bool; } From 7d585ac7bb861842118694a2bb9be739e2138063 Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Thu, 29 Feb 2024 14:42:00 -0800 Subject: [PATCH 2/6] ++ --- testing/scenario_app/bin/run_android_tests.dart | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/testing/scenario_app/bin/run_android_tests.dart b/testing/scenario_app/bin/run_android_tests.dart index d3fcbcce24ff7..5b2e281aeaa2c 100644 --- a/testing/scenario_app/bin/run_android_tests.dart +++ b/testing/scenario_app/bin/run_android_tests.dart @@ -138,7 +138,7 @@ Future _run({ final Directory finalLogsDir = logsDir..createSync(recursive: true); logsDir = Directory.systemTemp.createTempSync('scenario_app_test_logs.'); final String logcatPath = join(logsDir.path, 'logcat.txt'); - + final String screenshotPath = logsDir.path; final String apkOutPath = join(scenarioAppPath, 'app', 'outputs', 'apk'); final File testApk = File(join(apkOutPath, 'androidTest', 'debug', 'app-debug-androidTest.apk')); @@ -511,7 +511,7 @@ void _withTemporaryCwd(String path, void Function() callback) { } /// Reads the file named `reruns.txt` in the logs directory and returns the number of reruns. -/// +/// /// If the file does not exist, it is created with the number 1 and that number is returned. int _getAndIncrementRerunNumber(String logsDir) { final File rerunFile = File(join(logsDir, 'reruns.txt')); @@ -525,7 +525,7 @@ int _getAndIncrementRerunNumber(String logsDir) { } /// Copies the contents of [source] to [destination], optionally adding a [prefix] to the destination path. -/// +/// /// This function is used to copy the screenshots from the device to the logs directory. void _copyFiles({ required Directory source, From f23b20f7f2879a372e54c4ab5f4669c0a62b6ed4 Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Thu, 29 Feb 2024 16:15:48 -0800 Subject: [PATCH 3/6] ++ --- testing/scenario_app/bin/run_android_tests.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testing/scenario_app/bin/run_android_tests.dart b/testing/scenario_app/bin/run_android_tests.dart index 5b2e281aeaa2c..0dc2138c5db7c 100644 --- a/testing/scenario_app/bin/run_android_tests.dart +++ b/testing/scenario_app/bin/run_android_tests.dart @@ -520,7 +520,7 @@ int _getAndIncrementRerunNumber(String logsDir) { return 1; } final int rerunNumber = int.parse(rerunFile.readAsStringSync()) + 1; - rerunFile.writeAsStringSync(rerunNumber.toString());; + rerunFile.writeAsStringSync(rerunNumber.toString()); return rerunNumber; } From 77841d0a6eff2a6a5816dd101472c03fa4555ec1 Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Thu, 29 Feb 2024 16:55:49 -0800 Subject: [PATCH 4/6] ++ --- testing/scenario_app/bin/run_android_tests.dart | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/testing/scenario_app/bin/run_android_tests.dart b/testing/scenario_app/bin/run_android_tests.dart index 0dc2138c5db7c..5e7ce570bc1e2 100644 --- a/testing/scenario_app/bin/run_android_tests.dart +++ b/testing/scenario_app/bin/run_android_tests.dart @@ -532,13 +532,9 @@ void _copyFiles({ required Directory destination, String prefix = '', }) { - for (final FileSystemEntity entity in source.listSync(recursive: true)) { + for (final FileSystemEntity entity in source.listSync()) { if (entity is File) { - final String relativePath = relative(entity.path, from: source.path); - final String destinationPath = join(destination.path, prefix, relativePath); - final File destinationFile = File(destinationPath); - destinationFile.createSync(recursive: true); - destinationFile.writeAsBytesSync(entity.readAsBytesSync()); + entity.copySync(join(destination.path, prefix + basename(entity.path))); } } } From 458c181e88f8c29303408701b64cae772f9f04c4 Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Mon, 4 Mar 2024 09:45:16 -0800 Subject: [PATCH 5/6] ++ --- testing/scenario_app/bin/run_android_tests.dart | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/testing/scenario_app/bin/run_android_tests.dart b/testing/scenario_app/bin/run_android_tests.dart index 5e7ce570bc1e2..d834ae3a11a1b 100644 --- a/testing/scenario_app/bin/run_android_tests.dart +++ b/testing/scenario_app/bin/run_android_tests.dart @@ -409,8 +409,11 @@ Future _run({ } else { prefix.write('skia_'); } - if (impellerBackend != null) { - prefix.write(impellerBackend.name); + if (enableImpeller) { + prefix.write(impellerBackend!.name); + } + if (forceSurfaceProducerSurfaceTexture) { + prefix.write('_force-st'); } prefix.write('.'); } From fda493419cd87d2bd9ac495c1c3db2fddd82cd82 Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Mon, 4 Mar 2024 11:10:52 -0800 Subject: [PATCH 6/6] ++ --- testing/scenario_app/bin/run_android_tests.dart | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/testing/scenario_app/bin/run_android_tests.dart b/testing/scenario_app/bin/run_android_tests.dart index d834ae3a11a1b..43622acee9c39 100644 --- a/testing/scenario_app/bin/run_android_tests.dart +++ b/testing/scenario_app/bin/run_android_tests.dart @@ -405,12 +405,12 @@ Future _run({ final int rerunNumber = _getAndIncrementRerunNumber(finalLogsDir.path); prefix.write('run_$rerunNumber.'); if (enableImpeller) { - prefix.write('impeller_'); + prefix.write('impeller'); } else { - prefix.write('skia_'); + prefix.write('skia'); } if (enableImpeller) { - prefix.write(impellerBackend!.name); + prefix.write('_${impellerBackend!.name}'); } if (forceSurfaceProducerSurfaceTexture) { prefix.write('_force-st');