From 848b7a64b8a4883885c2a886224d61ad6bce4e04 Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Tue, 29 Aug 2023 13:07:37 -0700 Subject: [PATCH 01/16] Add a simple tool to scrape buildbucket logs. --- tools/build_bucket_golden_scraper/README.md | 45 ++++ .../build_bucket_golden_scraper/bin/main.dart | 20 ++ .../lib/build_bucket_golden_scraper.dart | 201 ++++++++++++++++++ .../build_bucket_golden_scraper/pubspec.yaml | 47 ++++ 4 files changed, 313 insertions(+) create mode 100644 tools/build_bucket_golden_scraper/README.md create mode 100644 tools/build_bucket_golden_scraper/bin/main.dart create mode 100644 tools/build_bucket_golden_scraper/lib/build_bucket_golden_scraper.dart create mode 100644 tools/build_bucket_golden_scraper/pubspec.yaml diff --git a/tools/build_bucket_golden_scraper/README.md b/tools/build_bucket_golden_scraper/README.md new file mode 100644 index 0000000000000..643ce5a2c0886 --- /dev/null +++ b/tools/build_bucket_golden_scraper/README.md @@ -0,0 +1,45 @@ +# `build_bucket_golden_scraper` + +Given logging on Flutter's CI, scrapes the log for golden file changes. + +```shell +$ dart bin/main.dart + +Wrote 3 golden file changes: + testing/resources/performance_overlay_gold_60fps.png + testing/resources/performance_overlay_gold_90fps.png + testing/resources/performance_overlay_gold_120fps.png +``` + +It can also be run with `--dry-run` to just print what it _would_ do: + +```shell +$ dart bin/main.dart --dry-run + +Found 3 golden file changes: + testing/resources/performance_overlay_gold_60fps.png + testing/resources/performance_overlay_gold_90fps.png + testing/resources/performance_overlay_gold_120fps.png + +Run again without --dry-run to apply these changes. +``` + +You're recommended to still use `git diff` to verify the changes look good. + +## Motivation + +Due to , on non-Linux OSes +there is no way to get golden-file changes locally for a variety of engine +tests. + +This tool, given log output from a Flutter CI run, will scrape the log for: + +```txt +Golden file mismatch. Please check the difference between /b/s/w/ir/cache/builder/src/flutter/testing/resources/performance_overlay_gold_90fps.png and /b/s/w/ir/cache/builder/src/flutter/testing/resources/performance_overlay_gold_90fps_new.png, and replace the former with the latter if the difference looks good. +S +See also the base64 encoded /b/s/w/ir/cache/builder/src/flutter/testing/resources/performance_overlay_gold_90fps_new.png: +iVBORw0KGgoAAAANSUhEUgAAA+gAAAPoCAYAAABNo9TkAAAABHNCSVQICAgIfAhkiAAAIABJREFUeJzs3elzFWeeJ/rnHB3tSEILktgEBrPvYBbbUF4K24X3t (...omitted) +``` + +And convert the base64 encoded image into a PNG file, and overwrite the old +golden file with the new one. diff --git a/tools/build_bucket_golden_scraper/bin/main.dart b/tools/build_bucket_golden_scraper/bin/main.dart new file mode 100644 index 0000000000000..94ffbd65fd945 --- /dev/null +++ b/tools/build_bucket_golden_scraper/bin/main.dart @@ -0,0 +1,20 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'dart:io' as io; + +import 'package:build_bucket_golden_scraper/build_bucket_golden_scraper.dart'; + +void main(List arguments) async { + final int result; + try { + result = await BuildBucketGoldenScraper.fromCommandLine(arguments).run(); + } on FormatException catch (e) { + io.stderr.writeln(e.message); + io.exit(1); + } + if (result != 0) { + io.exit(result); + } +} diff --git a/tools/build_bucket_golden_scraper/lib/build_bucket_golden_scraper.dart b/tools/build_bucket_golden_scraper/lib/build_bucket_golden_scraper.dart new file mode 100644 index 0000000000000..54bc8272da3c3 --- /dev/null +++ b/tools/build_bucket_golden_scraper/lib/build_bucket_golden_scraper.dart @@ -0,0 +1,201 @@ +// 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'; +import 'dart:io' as io; + +import 'package:args/args.dart'; +import 'package:engine_repo_tools/engine_repo_tools.dart'; +import 'package:meta/meta.dart'; +import 'package:path/path.dart' as p; + +/// "Downloads" (i.e. decodes base64 encoded strings) goldens from buildbucket. +/// +/// See ../README.md for motivation and usage. +final class BuildBucketGoldenScraper { + /// Creates a scraper with the given configuration. + BuildBucketGoldenScraper({ + required this.pathOrUrl, + this.dryRun = false, + String? engineSrcPath, + StringSink? outSink, + }) : + engine = engineSrcPath != null ? + Engine.fromSrcPath(engineSrcPath) : + Engine.findWithin(p.dirname(p.fromUri(io.Platform.script))), + _outSink = outSink ?? io.stdout; + + /// Creates a scraper from the command line arguments. + /// + /// Throws [FormatException] if the arguments are invalid. + factory BuildBucketGoldenScraper.fromCommandLine( + List args, { + StringSink? outSink, + StringSink? errSink, + }) { + outSink ??= io.stdout; + errSink ??= io.stderr; + + final ArgResults argResults = _argParser.parse(args); + if (argResults['help'] as bool) { + _usage(args); + } + final String? pathOrUrl = argResults.rest.isEmpty ? null : argResults.rest.first; + if (pathOrUrl == null) { + _usage(args); + } + return BuildBucketGoldenScraper( + pathOrUrl: pathOrUrl, + dryRun: argResults['dry-run'] as bool, + outSink: outSink, + engineSrcPath: argResults['engine-src-path'] as String?, + ); + } + + static Never _usage(List args) { + final StringBuffer output = StringBuffer(); + output.writeln('Usage: build_bucket_golden_scraper [options] '); + output.writeln(); + output.writeln(_argParser.usage); + throw FormatException(output.toString(), args.join(' ')); + } + + static final ArgParser _argParser = ArgParser() + ..addFlag( + 'help', + abbr: 'h', + help: 'Print this help message.', + negatable: false, + ) + ..addFlag( + 'dry-run', + help: "If true, don't write any files to disk (other than temporary files).", + negatable: false, + ) + ..addOption( + 'engine-src-path', + help: 'The path to the engine source code.', + valueHelp: 'path/that/contains/src (defaults to the directory containing this script)', + ); + + /// A local path or a URL to a buildbucket log file. + final String pathOrUrl; + + /// If true, don't write any files to disk (other than temporary files). + final bool dryRun; + + /// The path to the engine source code. + final Engine engine; + + /// How to print output, typically [io.stdout]. + final StringSink _outSink; + + /// Runs the scraper. + Future run() async { + final io.File file; + + // If the path is a URL, download it and store it in a temporary file. + final Uri? maybeUri = Uri.tryParse(pathOrUrl); + if (maybeUri == null) { + throw FormatException('Invalid path or URL: $pathOrUrl'); + } + if (maybeUri.hasScheme) { + file = await _downloadFile(maybeUri); + } else { + file = io.File(pathOrUrl); + } + + // Read the file as a string. + final String contents = await file.readAsString(); + + // Check that it is a buildbucket log file. + if (!contents.contains(_buildBucketMagicString)) { + throw FormatException('Not a buildbucket log file: $pathOrUrl'); + } + + // Check for occurences of a base64 encoded string. + // + // The format looks something like this: + // [LINE N+0]: See also the base64 encoded /b/s/w/ir/cache/builder/src/flutter/testing/resources/performance_overlay_gold_120fps_new.png: + // [LINE N+1]: {{BASE_64_ENCODED_IMAGE}} + // + // We want to extract the file name (relative to the engine root) and then + // decode the base64 encoded string (and write it to disk if we are not in + // dry-run mode). + final List<_Golden> goldens = <_Golden>[]; + final List lines = contents.split('\n'); + for (int i = 0; i < lines.length; i++) { + final String line = lines[i]; + if (line.startsWith(_base64MagicString)) { + final String relativePath = line.split(_buildBucketMagicString).last.split(':').first; + final String base64EncodedString = lines[i + 1]; + final List bytes = base64Decode(base64EncodedString); + final io.File outFile = io.File(p.join(engine.srcDir.path, relativePath)); + goldens.add(_Golden(outFile, bytes)); + } + } + + if (goldens.isEmpty) { + _outSink.writeln('No goldens found.'); + return 0; + } + + // Sort and de-duplicate the goldens. + goldens.sort(); + final Set<_Golden> uniqueGoldens = goldens.toSet(); + + // Write the goldens to disk (or pretend to in dry-run mode). + _outSink.writeln('Found ${uniqueGoldens.length} golden file changes:'); + for (final _Golden golden in uniqueGoldens) { + final String truncatedPathAfterFlutterDir = golden.outFile.path.split('flutter${p.separator}').last; + _outSink.writeln(' $truncatedPathAfterFlutterDir'); + if (!dryRun) { + await golden.outFile.writeAsBytes(golden.bytes); + } + } + if (dryRun) { + _outSink.writeln('Run again without --dry-run to apply these changes.'); + } + + return 0; + } + + static const String _buildBucketMagicString = '/b/s/w/ir/cache/builder/src/'; + static const String _base64MagicString = 'See also the base64 encoded $_buildBucketMagicString'; + + static Future _downloadFile(Uri uri) async { + final io.Directory tempDir = io.Directory.systemTemp.createTempSync('build_bucket_golden_scraper'); + final io.File file = io.File(p.join(tempDir.path, uri.pathSegments.last)); + final io.HttpClient client = io.HttpClient(); + final io.HttpClientRequest request = await client.getUrl(uri); + final io.HttpClientResponse response = await request.close(); + await response.pipe(file.openWrite()); + client.close(); + return file; + } +} + +@immutable +final class _Golden implements Comparable<_Golden> { + const _Golden(this.outFile, this.bytes); + + /// Where to write the golden file. + final io.File outFile; + + /// The bytes of the golden file to write. + final List bytes; + + @override + int get hashCode => outFile.path.hashCode; + + @override + bool operator ==(Object other) { + return other is _Golden && other.outFile.path == outFile.path; + } + + @override + int compareTo(_Golden other) { + return outFile.path.compareTo(other.outFile.path); + } +} diff --git a/tools/build_bucket_golden_scraper/pubspec.yaml b/tools/build_bucket_golden_scraper/pubspec.yaml new file mode 100644 index 0000000000000..9d38d8327d97e --- /dev/null +++ b/tools/build_bucket_golden_scraper/pubspec.yaml @@ -0,0 +1,47 @@ +# 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: build_bucket_golden_scraper +publish_to: none +environment: + sdk: ^3.0.0 + +# Do not add any dependencies that require more than what is provided in +# //third_party/pkg, //third_party/dart/pkg, or +# //third_party/dart/third_party/pkg. In particular, package:test is not usable +# here. + +# If you do add packages here, make sure you can run `pub get --offline`, and +# check the .packages and .package_config to make sure all the paths are +# relative to this directory into //third_party/dart + +dependencies: + args: any + engine_repo_tools: any + meta: any + path: any + +dev_dependencies: + async_helper: any + expect: any + litetest: any + smith: any + +dependency_overrides: + async_helper: + path: ../../../third_party/dart/pkg/async_helper + args: + path: ../../../third_party/dart/third_party/pkg/args + engine_repo_tools: + path: ../pkg/engine_repo_tools + expect: + path: ../../../third_party/dart/pkg/expect + litetest: + path: ../../testing/litetest + meta: + path: ../../../third_party/dart/pkg/meta + path: + path: ../../../third_party/dart/third_party/pkg/path + smith: + path: ../../../third_party/dart/pkg/smith From 2340e8589cbf9b0f3208e6b10e65bf0955304bfa Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Tue, 29 Aug 2023 13:09:24 -0700 Subject: [PATCH 02/16] Fix naming. --- .../lib/build_bucket_golden_scraper.dart | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tools/build_bucket_golden_scraper/lib/build_bucket_golden_scraper.dart b/tools/build_bucket_golden_scraper/lib/build_bucket_golden_scraper.dart index 54bc8272da3c3..32a8846f20a1c 100644 --- a/tools/build_bucket_golden_scraper/lib/build_bucket_golden_scraper.dart +++ b/tools/build_bucket_golden_scraper/lib/build_bucket_golden_scraper.dart @@ -129,9 +129,13 @@ final class BuildBucketGoldenScraper { final String line = lines[i]; if (line.startsWith(_base64MagicString)) { final String relativePath = line.split(_buildBucketMagicString).last.split(':').first; + + // Remove the _new suffix from the file name. + final String pathWithouNew = relativePath.replaceAll('_new', ''); + final String base64EncodedString = lines[i + 1]; final List bytes = base64Decode(base64EncodedString); - final io.File outFile = io.File(p.join(engine.srcDir.path, relativePath)); + final io.File outFile = io.File(p.join(engine.srcDir.path, pathWithouNew)); goldens.add(_Golden(outFile, bytes)); } } From b9c2cc1ca0455c34f62bcc0f3215c48cdba8a4a1 Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Tue, 29 Aug 2023 13:19:09 -0700 Subject: [PATCH 03/16] Update README. --- tools/build_bucket_golden_scraper/README.md | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tools/build_bucket_golden_scraper/README.md b/tools/build_bucket_golden_scraper/README.md index 643ce5a2c0886..2309478da8408 100644 --- a/tools/build_bucket_golden_scraper/README.md +++ b/tools/build_bucket_golden_scraper/README.md @@ -26,6 +26,20 @@ Run again without --dry-run to apply these changes. You're recommended to still use `git diff` to verify the changes look good. +## Upgrading `git diff` + +By default, `git diff` is not very helpful for binary files. You can install +[`imagemagick`](https://imagemagick.org/) and configure your local git client +to make `git diff` show a PNG diff: + +```shell +# On MacOS. +$ brew install exiftool imagemagick +$ git clone https://github.com/ewanmellor/git-diff-image +$ cd git-diff-image +$ ./install.sh +``` + ## Motivation Due to , on non-Linux OSes From a9378d80aba8494e8feb71513226664d4b82c883 Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Tue, 29 Aug 2023 13:41:54 -0700 Subject: [PATCH 04/16] Tweak README with suggestions for git diff. --- tools/build_bucket_golden_scraper/README.md | 35 ++++++++++++++++++--- 1 file changed, 31 insertions(+), 4 deletions(-) diff --git a/tools/build_bucket_golden_scraper/README.md b/tools/build_bucket_golden_scraper/README.md index 2309478da8408..687b357f4f7e7 100644 --- a/tools/build_bucket_golden_scraper/README.md +++ b/tools/build_bucket_golden_scraper/README.md @@ -34,10 +34,37 @@ to make `git diff` show a PNG diff: ```shell # On MacOS. -$ brew install exiftool imagemagick -$ git clone https://github.com/ewanmellor/git-diff-image -$ cd git-diff-image -$ ./install.sh +$ brew install imagemagick + +# Create a comparison script. +$ cat > ~/bin/git-imgdiff <> ~/.gitattributes < Date: Tue, 29 Aug 2023 13:55:03 -0700 Subject: [PATCH 05/16] Add a test. --- .../build_bucket_golden_scraper_test.dart | 74 +++++++++++++++++++ 1 file changed, 74 insertions(+) create mode 100644 tools/build_bucket_golden_scraper/test/build_bucket_golden_scraper_test.dart diff --git a/tools/build_bucket_golden_scraper/test/build_bucket_golden_scraper_test.dart b/tools/build_bucket_golden_scraper/test/build_bucket_golden_scraper_test.dart new file mode 100644 index 0000000000000..d6c26acdbd5e8 --- /dev/null +++ b/tools/build_bucket_golden_scraper/test/build_bucket_golden_scraper_test.dart @@ -0,0 +1,74 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'dart:io' as io; + +import 'package:build_bucket_golden_scraper/build_bucket_golden_scraper.dart'; +import 'package:litetest/litetest.dart'; +import 'package:path/path.dart' as p; + +int main(List args) { + test('parses command-line arguments', () { + // Create a fake engine directory. + final io.Directory buildRoot = io.Directory.systemTemp.createTempSync('build_bucket_golden_scraper_test_engine'); + final io.Directory srcDir = io.Directory(p.join(buildRoot.path, 'src'))..createSync(); + io.Directory(p.join(srcDir.path, 'flutter')).createSync(); + + // Does not run, but just parses. + try { + final BuildBucketGoldenScraper scraper = BuildBucketGoldenScraper.fromCommandLine([ + '--dry-run', + '--engine-src-path', + srcDir.path, + 'https://ci.chromium.org/raw/buildbucket/v1/builders/flutter/flutter-linux/builder:linux_bare', + ]); + + expect(scraper.dryRun, isTrue); + expect(scraper.engine.srcDir.path, srcDir.path); + expect(scraper.pathOrUrl, 'https://ci.chromium.org/raw/buildbucket/v1/builders/flutter/flutter-linux/builder:linux_bare'); + } finally { + buildRoot.deleteSync(recursive: true); + } + }); + + test('finds diffs', () async { + // Create a fake engine directory. + final io.Directory buildRoot = io.Directory.systemTemp.createTempSync('build_bucket_golden_scraper_test_engine'); + final io.Directory srcDir = io.Directory(p.join(buildRoot.path, 'src'))..createSync(); + io.Directory(p.join(srcDir.path, 'flutter', 'docs')).createSync(recursive: true); + + // Create an empty logo in docs/flutter_logo.png. + final io.File logo = io.File(p.join(srcDir.path, 'flutter', 'docs', 'flutter_logo.png'))..createSync(); + + // Create a fake log file. + try { + final io.File logFile = io.File(p.join(buildRoot.path, 'log.txt'))..createSync(); + logFile.writeAsStringSync(''' +See also the base64 encoded /b/s/w/ir/cache/builder/src/flutter/docs/flutter_logo_new.png: +iVBORw0KGgoAAAANSUhEUgAAADcAAAA3CAYAAACo29JGAAAAAXNSR0IArs4c6QAAAAlwSFlzAAALEwAACxMBAJqcGAAAA6hpVFh0WE1MOmNvbS5hZG9iZS54bXAAAAAAADx4OnhtcG1ldGEgeG1sbnM6eD0iYWRvYmU6bnM6bWV0YS8iIHg6eG1wdGs9IlhNUCBDb3JlIDUuNC4wIj4KICAgPHJkZjpSREYgeG1sbnM6cmRmPSJodHRwOi8vd3d3LnczLm9yZy8xOTk5LzAyLzIyLXJkZi1zeW50YXgtbnMjIj4KICAgICAgPHJkZjpEZXNjcmlwdGlvbiByZGY6YWJvdXQ9IiIKICAgICAgICAgICAgeG1sbnM6eG1wPSJodHRwOi8vbnMuYWRvYmUuY29tL3hhcC8xLjAvIgogICAgICAgICAgICB4bWxuczp0aWZmPSJodHRwOi8vbnMuYWRvYmUuY29tL3RpZmYvMS4wLyIKICAgICAgICAgICAgeG1sbnM6ZXhpZj0iaHR0cDovL25zLmFkb2JlLmNvbS9leGlmLzEuMC8iPgogICAgICAgICA8eG1wOk1vZGlmeURhdGU+MjAxOS0wOC0yN1QyMDowODo0NzwveG1wOk1vZGlmeURhdGU+CiAgICAgICAgIDx4bXA6Q3JlYXRvclRvb2w+UGl4ZWxtYXRvciAzLjguNTwveG1wOkNyZWF0b3JUb29sPgogICAgICAgICA8dGlmZjpPcmllbnRhdGlvbj4xPC90aWZmOk9yaWVudGF0aW9uPgogICAgICAgICA8dGlmZjpDb21wcmVzc2lvbj4wPC90aWZmOkNvbXByZXNzaW9uPgogICAgICAgICA8dGlmZjpSZXNvbHV0aW9uVW5pdD4yPC90aWZmOlJlc29sdXRpb25Vbml0PgogICAgICAgICA8dGlmZjpZUmVzb2x1dGlvbj43MjwvdGlmZjpZUmVzb2x1dGlvbj4KICAgICAgICAgPHRpZmY6WFJlc29sdXRpb24+NzI8L3RpZmY6WFJlc29sdXRpb24+CiAgICAgICAgIDxleGlmOlBpeGVsWERpbWVuc2lvbj41NTwvZXhpZjpQaXhlbFhEaW1lbnNpb24+CiAgICAgICAgIDxleGlmOkNvbG9yU3BhY2U+MTwvZXhpZjpDb2xvclNwYWNlPgogICAgICAgICA8ZXhpZjpQaXhlbFlEaW1lbnNpb24+NTU8L2V4aWY6UGl4ZWxZRGltZW5zaW9uPgogICAgICA8L3JkZjpEZXNjcmlwdGlvbj4KICAgPC9yZGY6UkRGPgo8L3g6eG1wbWV0YT4KKG12ggAADJJJREFUaAXtmntwVNUZwM+5793NJpuEoHVgtFaYUaqUkrGAKMEXhTqV1m4sPiBBCTN1cKZW+087denYdupYOxW1SJ0RCKBkAQUsEV9EUNBCrNgBHeVRLCCSQF6793Xuuaffd5MbNpCYQBLiH5whe8/evffc73e+x/nOdyHkQrswAxdm4MIMDNAM0AEap3/DpISUepSQFKV+5S77Yd8jfxSEMPjzYWCQkUKXEPhAeYWgcPQFnvNlXc9njvvbFyfqf8BrctuQw6WEkPaA0GlKecUH9gI1X3uCmb7me74ABAQDeQO2jk8KdNh8YhTplLWyJ5eXar/KhQr7StgZimMKNNYJVu/OUyPK4yzjaZxxTiUqgW7wX8AINIRIAMZBmzLheoGuua18UXUPYMgjDQUUPhPB6qYQCTU2d6c7G8D+ykxu+B73qCzJcAmqjILy2o8SkPlESBLxAzCLPV/953W/DMYC7ePx9DY0mkOvgYmto9SrrHfulHT5aW77MeFxF8A0ARZ5qkGfSgTO+YDraQnQmMmWf7L441+QdDlPbREK+Kp36vpTvW6JT/08CD0Aq6ongUAVu5yZsq4u9hmJew5zwRQ1CBRdHwqKE74vgI/pAMay7urGdw9V1S8pZcmaPVpqavdgOAjO4HltVbuEuqSUsrk7velKRKoGIyxmlm2DxozAw3LYsAt/QhLE1Qp1nWXcVz7bevyuHQ+NtBZsEvqiGdT5OuHPq+ZCsMp6dqNkSMt9Tos9y+kE6xQUiQIsQTrBsl7tl3s/vQfBKrYcNHoDw7HOm8+hCYHG3Pt2sOskRX4RrHMYM20HNRZEwk6ysAPmSIRrJHTdyXpvf/F+U3ndA2OzFVuEsXQqtcOrvu54XuA6NObO/SBbSg1lDcTo4SxjQfCQ9S5ggR3CBwZIMEU9YWiOybZb+5tvr3tgeAY11lcwhB50nwtNsWJ75molEntNUsklbsZmAKAiS24Lcg44AcBMLzBUZrIPm3a3lG24r6QtuUdo6THUzb2+t/6gwoUCzftIjPa5eFPR6Ugn6zAQSkXBusDBl+A7gGn5hupk3b0nj1uT/3lboimcoN5gTv990OBCsLnbmi8net5baky+zG5zPEoFuAL605kN1jJXzTc0Zrr75AZ30gsz4g3jIbrWQ3Q98+rezwxKtAzBKv9ljhRa9DU1T77MyTgMkHoEg+zDUfMMjZvuIdLKyxCsDBbocwVD9AGHm77pcx19o2pX9ltEyJv0fHWUm2EOOJKKgaJbjQliK3m67lnuEde2blx+U/QImmLd1yzQvettgOEQrHbGKKdyqyhhXNmgF2jfdVpdW/hc7wkMaG01phue7R3zbPfWVdcnDqRqBC4b52SKudAD5nOhKd73WkuRn9DX64X6ZLuFmcTnUYzJmNt30ywlqkc8mzVyy5y24obEh2ezjnUzXpdT3T6xyxV9+BKCzX23Ie6r8ZeNQv0mu9nNQk4Yoz2YIgxrKREAc70mnrVuWzElf3tyu4ikJ1ErKYTcUFfXR9nKSEkDEelyyk8XtY8DnH7bqe9hmMYZJ1FnLYDNcFpYxuc8L1iMT12a27NlQze464E32j9eNTm+BcaJgimaGETqygiHe9u3crl35faDBAbMoXs3Dq7sNxyOkkptUQ5On5SOFGkzneYOMNh4QToPv3Z9BCzQrqIbsNP2HM6c26sn5W0OwMYTa8E+oi0aRZ1kzf4CNVosmQyW9UQgZ9ePZkKiaitletx3TO7HL4oVrJwQPdz1ogHKLQ9Mm7AyUqzNtJtY1kdT7AEMHI9Jqq55Lkjt2snq6/M3V20EjY2n1r2vH4sumnZx9q5tmXuN4uj9vkt9WOnbXTUnxAbdIpy3BD7GikpiBDedpwDs+QGHm7PTWRUp1MqtJjcLe7FYoKcgj+rQGEqDficE7LBlNdjWMO9nALYRfWzJRGJV1cNxGs3OervtDi1uLKYyjVJcEXOV3gEYHOC8B5ud2HBCzK/IOkk3ak8Hw+8wRD+bENsgJs6SJFnxuQe+AhUOmHACm+0OeXCzyWVFkSRVISybmVU9Ob5u+udCT19B7OQOYiyZRM17tmZ/pBQYL8BNUafRdgTUUALJOgfBUpiguCkHa7e1hBFv+4JXN31wcF7tg6O63df1exFfdq3xdydrzlMisk4VhQMI7KVRgg6pQBRJlqlsKJJr2nOqr4u/hEGo9grioo9hdJxVl7kFwKoBLO6ZYLMyhXUxyD8xBw3+YK5U2Kkrvk98o8SIc0esZa2tCxAMg1B3Kuo3XLKmRl4xMfa81WwtUKOyRmWVC+5zEvgdRD2o9SgxVXItu2rFpMhyFOSSVwlPpomKwWPOe9YNRsJYBUZYGIBJNEiqA7WH84PmiZYNuWek2NC9rHg1e/Lo/PQtRS24bPSUyfQbbszepECBV0yKPg2Av9byFBU0iGUP1wcjUuOqBFuXBdXXRv4RCPIO8YOqVzl1r9zQMqFRVV+SJGmYB7sFMMUzwDDYAiMaAjOKIam2xestXzTc//LNI07gc7F61p3W8Fyuy/Z0Ta/nU1Baq6uDahbkgve+b/0OgsJCN+MKNaZRqGo9sqxUfiJZI+T0XpDzUXgmCPT9V5vHy/G8l6GkNXK4cNySiKxhHRZJgkCCWgPpYDEREKi4XmgonIl37MaTd6dvGnYkBWCpXnLPfmsOyVNQBi8rIz76UvWEyO/dNvtP0WKN+o73mwAMTGcMgl3VAbY+c40cj6bBh0b6Wdv90qFag82J0iFNuDy2g0EBNgFgntjRdrSlAsHwOb2BoVwDAhcCNh3AicaZZ3/JNmZvzTrmYvw+BmY/BQVYAinSuFdOXCUX6jVUVr7N2ixXUiVNApqjpiCNtg+AqLocjSV0CCJiV8uJttmvTC/8L4L1NakeELNEgLBhJTmFX1LUR3OFfruBgXbHrmsdrRVH1lBVuZq1mA5EUaihBIVKiu81MMCOiFJSrEORkvtQWY5IcH63czI7a/WU+CfhrgOH70sbcDh8aPAO4CqipMsJIynQA4COW9t0qTI8vgYW8lLWnHUh6EBlGWJBR2KNJtT+4oaSEYbwS4qiEhPeZ1aDk1x7c97HuC7WQnTtC1R4zaDAhYMHWnyUiB/UnozzWOIdyJe+52VMLOfpEE4DsECvcANmyTIsH4x5Qo1E6cWK0zDumsjExxJ0/3QowNb2UoANn5l7HDCfyx007Kegk1pIqOOA1JI4gL4Ef+BVYIBhbhVMLyz7kHrA9oeohkHcTIZsW/um/Fj545fjWLU1S89JCYMKF/gdCPfRTwqb3ff3zPY9sUYtiqqECzcARcnR0QCMM0bgRSJhpkkPv/GWb311vCgy8rK1evniGWRppU2ST0bw8rNpgwuHkmBggSDz8SNjs/bxtvm+yTcoRVGdcB/8B9IO0KDPPCIBGHdscnjz2yRz5EtJ0WROuBen0fylenLxLST9kEUqXjDOBu6c1H02DwivDRZxWAquWXdsuDZs2DKqyT90T5pYONIlTQVARo5seoNkDv2PyLEo8UEySCM9qkchb5SOevaJu72XFtSRipRBlqb6VE4/b3AIGQKOW99wCU0UVlNFvtE3XUdwrh/buJm0fn6QyAV5hIM9cXjXiG9SoTGhRGCrRA7JZvNd1poHtsNAGryb67X6PPhmieJ1NKxzIOC/by856hxqrRSCvytFNP34+tfdtn0HCSnKJ0wF/5NBrCDxxmwZ8k1mQfmdXAoRd5l6x+LSAKzqufY8NBy8m+N51Vzn87dsUcjUqd7498R3jtRuXdW879C1QnYhyOBmFgwSF/TOiNN+Fyz2DCrXuNn9hNiNs9jqB3eTMhinbmq3b1XxrvOquU44ACPP7VLrr6P7j+3eOUdElI+ongdvVT0P0/8zwPBG2GAIB7JxQq4U2rBl2p3PXBmAlaW63cvhLUMDh0+eX8rGoO9sfPhTkTkxGxZ1eI8TQ815QAcM+IGomK7CHrw9m9aIk7HBF8f6Wv5yknzmClKX8sAH8T8InNGGxixzxZj+lE5qH3TUmX8bTwtKYNNKRgvXBBMNV3lcCoG1o0EP1g7fJlpeHkzEVi6su8nKeYfD33OPQw+H0pTB+lVXaUd++tQEHh+2Uija5bCqB3LmcKEy8R80EJvbRMSgDJY9sdd33Z+T9Pz/BD/lfHwz4FAgXKAhE1GSz06hmjEfakocVnhkgW0fJtU5UqPBouQ+VFJk4yLBvfVk9f1nlPZy7xj6PvrgubQeloVvjuZCKAzvoz87O7mWzO/2jdDZDRIKMNhH/B9GCxeenWypVFAFGGzRLox/YQb6OQP/B/nlPRSmL0CtAAAAAElFTkSuQmCC +'''); + + // Tell the runner to extract diffs. + final StringBuffer outSink = StringBuffer(); + final BuildBucketGoldenScraper scraper = BuildBucketGoldenScraper( + pathOrUrl: logFile.path, + engineSrcPath: srcDir.path, + outSink: outSink, + ); + + // Run. + final int result = await scraper.run(); + expect(result, 0); + + // Check the output. + expect(outSink.toString(), contains('Found 1 golden file changes:')); + expect(outSink.toString(), contains('docs/flutter_logo.png')); + expect(logo.readAsBytesSync().length, 4257); + } finally { + buildRoot.deleteSync(recursive: true); + } + }); + + return 0; +} From 67f8b0b73ff1f2710b7365eb8f937b1e902fa77b Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Tue, 29 Aug 2023 13:56:48 -0700 Subject: [PATCH 06/16] Update run_tests.py. --- testing/run_tests.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/testing/run_tests.py b/testing/run_tests.py index 9a4a893205153..54577584f6ea2 100755 --- a/testing/run_tests.py +++ b/testing/run_tests.py @@ -953,6 +953,22 @@ def gather_clang_tidy_tests(build_dir): ) +def gather_build_bucket_golden_scraper_tests(build_dir): + test_dir = os.path.join(BUILDROOT_DIR, 'flutter', 'tools', 'build_bucket_golden_scraper') + dart_tests = glob.glob('%s/test/*_test.dart' % test_dir) + for dart_test_file in dart_tests: + opts = [ + '--disable-dart-dev', + dart_test_file, + ] + yield EngineExecutableTask( + build_dir, + os.path.join('dart-sdk', 'bin', 'dart'), + None, + flags=opts, + cwd=test_dir + ) + def gather_engine_repo_tools_tests(build_dir): test_dir = os.path.join( BUILDROOT_DIR, 'flutter', 'tools', 'pkg', 'engine_repo_tools' @@ -1249,6 +1265,7 @@ def main(): tasks += list(gather_litetest_tests(build_dir)) tasks += list(gather_githooks_tests(build_dir)) tasks += list(gather_clang_tidy_tests(build_dir)) + tasks += list(gather_build_bucket_golden_scraper_tests(build_dir)) tasks += list(gather_engine_repo_tools_tests(build_dir)) tasks += list(gather_api_consistency_tests(build_dir)) tasks += list(gather_path_ops_tests(build_dir)) From 8ca3c0c516ec4fe7cd6aa43c5cd487b97a0f2291 Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Tue, 29 Aug 2023 13:57:39 -0700 Subject: [PATCH 07/16] Formatting. --- .../lib/build_bucket_golden_scraper.dart | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tools/build_bucket_golden_scraper/lib/build_bucket_golden_scraper.dart b/tools/build_bucket_golden_scraper/lib/build_bucket_golden_scraper.dart index 32a8846f20a1c..ac13506be68d5 100644 --- a/tools/build_bucket_golden_scraper/lib/build_bucket_golden_scraper.dart +++ b/tools/build_bucket_golden_scraper/lib/build_bucket_golden_scraper.dart @@ -11,7 +11,7 @@ import 'package:meta/meta.dart'; import 'package:path/path.dart' as p; /// "Downloads" (i.e. decodes base64 encoded strings) goldens from buildbucket. -/// +/// /// See ../README.md for motivation and usage. final class BuildBucketGoldenScraper { /// Creates a scraper with the given configuration. @@ -20,14 +20,14 @@ final class BuildBucketGoldenScraper { this.dryRun = false, String? engineSrcPath, StringSink? outSink, - }) : + }) : engine = engineSrcPath != null ? Engine.fromSrcPath(engineSrcPath) : Engine.findWithin(p.dirname(p.fromUri(io.Platform.script))), _outSink = outSink ?? io.stdout; /// Creates a scraper from the command line arguments. - /// + /// /// Throws [FormatException] if the arguments are invalid. factory BuildBucketGoldenScraper.fromCommandLine( List args, { @@ -197,7 +197,7 @@ final class _Golden implements Comparable<_Golden> { bool operator ==(Object other) { return other is _Golden && other.outFile.path == outFile.path; } - + @override int compareTo(_Golden other) { return outFile.path.compareTo(other.outFile.path); From a715e87f4efb55156e6919aaf0e899336f11b6a1 Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Tue, 29 Aug 2023 13:57:59 -0700 Subject: [PATCH 08/16] Formatting. --- .../lib/build_bucket_golden_scraper.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/build_bucket_golden_scraper/lib/build_bucket_golden_scraper.dart b/tools/build_bucket_golden_scraper/lib/build_bucket_golden_scraper.dart index ac13506be68d5..17736b995caed 100644 --- a/tools/build_bucket_golden_scraper/lib/build_bucket_golden_scraper.dart +++ b/tools/build_bucket_golden_scraper/lib/build_bucket_golden_scraper.dart @@ -21,7 +21,7 @@ final class BuildBucketGoldenScraper { String? engineSrcPath, StringSink? outSink, }) : - engine = engineSrcPath != null ? + engine = engineSrcPath != null ? Engine.fromSrcPath(engineSrcPath) : Engine.findWithin(p.dirname(p.fromUri(io.Platform.script))), _outSink = outSink ?? io.stdout; From 29c1553e98861ab650dd650a7bd83ba1f58a9259 Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Tue, 29 Aug 2023 13:58:18 -0700 Subject: [PATCH 09/16] Formatting. --- testing/run_tests.py | 1 + 1 file changed, 1 insertion(+) diff --git a/testing/run_tests.py b/testing/run_tests.py index 54577584f6ea2..98e008d837008 100755 --- a/testing/run_tests.py +++ b/testing/run_tests.py @@ -969,6 +969,7 @@ def gather_build_bucket_golden_scraper_tests(build_dir): cwd=test_dir ) + def gather_engine_repo_tools_tests(build_dir): test_dir = os.path.join( BUILDROOT_DIR, 'flutter', 'tools', 'pkg', 'engine_repo_tools' From e2bd5b0aaef09417480ab6e78b6b0faa09cd7a9b Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Tue, 29 Aug 2023 13:58:44 -0700 Subject: [PATCH 10/16] Formatting. --- testing/run_tests.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/testing/run_tests.py b/testing/run_tests.py index 98e008d837008..20c3ade050974 100755 --- a/testing/run_tests.py +++ b/testing/run_tests.py @@ -954,7 +954,9 @@ def gather_clang_tidy_tests(build_dir): def gather_build_bucket_golden_scraper_tests(build_dir): - test_dir = os.path.join(BUILDROOT_DIR, 'flutter', 'tools', 'build_bucket_golden_scraper') + test_dir = os.path.join( + BUILDROOT_DIR, 'flutter', 'tools', 'build_bucket_golden_scraper' + ) dart_tests = glob.glob('%s/test/*_test.dart' % test_dir) for dart_test_file in dart_tests: opts = [ From aedc253416f0cc1d24331f5bcbfbbde4011c0470 Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Tue, 29 Aug 2023 15:08:11 -0700 Subject: [PATCH 11/16] Avoid temporary file for downloads, fix CI. --- .../lib/build_bucket_golden_scraper.dart | 24 +++++++++---------- tools/pub_get_offline.py | 1 + 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/tools/build_bucket_golden_scraper/lib/build_bucket_golden_scraper.dart b/tools/build_bucket_golden_scraper/lib/build_bucket_golden_scraper.dart index 17736b995caed..c73d6defe5f63 100644 --- a/tools/build_bucket_golden_scraper/lib/build_bucket_golden_scraper.dart +++ b/tools/build_bucket_golden_scraper/lib/build_bucket_golden_scraper.dart @@ -93,22 +93,23 @@ final class BuildBucketGoldenScraper { /// Runs the scraper. Future run() async { - final io.File file; - // If the path is a URL, download it and store it in a temporary file. final Uri? maybeUri = Uri.tryParse(pathOrUrl); if (maybeUri == null) { throw FormatException('Invalid path or URL: $pathOrUrl'); } + + final String contents; if (maybeUri.hasScheme) { - file = await _downloadFile(maybeUri); + contents = await _downloadFile(maybeUri); } else { - file = io.File(pathOrUrl); + final io.File readFile = io.File(pathOrUrl); + if (!readFile.existsSync()) { + throw FormatException('File does not exist: $pathOrUrl'); + } + contents = readFile.readAsStringSync(); } - // Read the file as a string. - final String contents = await file.readAsString(); - // Check that it is a buildbucket log file. if (!contents.contains(_buildBucketMagicString)) { throw FormatException('Not a buildbucket log file: $pathOrUrl'); @@ -168,15 +169,14 @@ final class BuildBucketGoldenScraper { static const String _buildBucketMagicString = '/b/s/w/ir/cache/builder/src/'; static const String _base64MagicString = 'See also the base64 encoded $_buildBucketMagicString'; - static Future _downloadFile(Uri uri) async { - final io.Directory tempDir = io.Directory.systemTemp.createTempSync('build_bucket_golden_scraper'); - final io.File file = io.File(p.join(tempDir.path, uri.pathSegments.last)); + static Future _downloadFile(Uri uri) async { final io.HttpClient client = io.HttpClient(); final io.HttpClientRequest request = await client.getUrl(uri); final io.HttpClientResponse response = await request.close(); - await response.pipe(file.openWrite()); + final StringBuffer contents = StringBuffer(); + await response.transform(utf8.decoder).forEach(contents.write); client.close(); - return file; + return contents.toString(); } } diff --git a/tools/pub_get_offline.py b/tools/pub_get_offline.py index 8efd6efec285c..8ed5aef38cdd1 100644 --- a/tools/pub_get_offline.py +++ b/tools/pub_get_offline.py @@ -33,6 +33,7 @@ os.path.join(ENGINE_DIR, "testing", "symbols"), os.path.join(ENGINE_DIR, "tools", "api_check"), os.path.join(ENGINE_DIR, "tools", "android_lint"), + os.path.join(ENGINE_DIR, "tools", "build_bucket_golden_scraper"), os.path.join(ENGINE_DIR, "tools", "clang_tidy"), os.path.join(ENGINE_DIR, "tools", "const_finder"), os.path.join(ENGINE_DIR, "tools", "pkg", "engine_repo_tools"), From bb1830359860ed66ac5c5c716a33cb1be5afb602 Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Tue, 29 Aug 2023 15:11:14 -0700 Subject: [PATCH 12/16] Tweak output. --- .../lib/build_bucket_golden_scraper.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/build_bucket_golden_scraper/lib/build_bucket_golden_scraper.dart b/tools/build_bucket_golden_scraper/lib/build_bucket_golden_scraper.dart index c73d6defe5f63..a0f661f023465 100644 --- a/tools/build_bucket_golden_scraper/lib/build_bucket_golden_scraper.dart +++ b/tools/build_bucket_golden_scraper/lib/build_bucket_golden_scraper.dart @@ -151,7 +151,7 @@ final class BuildBucketGoldenScraper { final Set<_Golden> uniqueGoldens = goldens.toSet(); // Write the goldens to disk (or pretend to in dry-run mode). - _outSink.writeln('Found ${uniqueGoldens.length} golden file changes:'); + _outSink.writeln('${dryRun ? 'Found' : 'Wrote'} ${uniqueGoldens.length} golden file changes:'); for (final _Golden golden in uniqueGoldens) { final String truncatedPathAfterFlutterDir = golden.outFile.path.split('flutter${p.separator}').last; _outSink.writeln(' $truncatedPathAfterFlutterDir'); From 20a9ce3fe16509185f69631181dfd341f959c791 Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Tue, 29 Aug 2023 15:12:03 -0700 Subject: [PATCH 13/16] Tweak output. --- .../test/build_bucket_golden_scraper_test.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/build_bucket_golden_scraper/test/build_bucket_golden_scraper_test.dart b/tools/build_bucket_golden_scraper/test/build_bucket_golden_scraper_test.dart index d6c26acdbd5e8..ff95e266e1363 100644 --- a/tools/build_bucket_golden_scraper/test/build_bucket_golden_scraper_test.dart +++ b/tools/build_bucket_golden_scraper/test/build_bucket_golden_scraper_test.dart @@ -62,7 +62,7 @@ iVBORw0KGgoAAAANSUhEUgAAADcAAAA3CAYAAACo29JGAAAAAXNSR0IArs4c6QAAAAlwSFlzAAALEwAA expect(result, 0); // Check the output. - expect(outSink.toString(), contains('Found 1 golden file changes:')); + expect(outSink.toString(), contains('Wrote 1 golden file changes:')); expect(outSink.toString(), contains('docs/flutter_logo.png')); expect(logo.readAsBytesSync().length, 4257); } finally { From f9e7734d5a72f03298b73d39125c9e66715b534d Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Tue, 29 Aug 2023 17:58:26 -0700 Subject: [PATCH 14/16] Allow a name and see what Zach says. --- 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 20c3ade050974..4ab449c16a759 100755 --- a/testing/run_tests.py +++ b/testing/run_tests.py @@ -953,7 +953,7 @@ def gather_clang_tidy_tests(build_dir): ) -def gather_build_bucket_golden_scraper_tests(build_dir): +def gather_build_bucket_golden_scraper_tests(build_dir): # pylint: invalid-name test_dir = os.path.join( BUILDROOT_DIR, 'flutter', 'tools', 'build_bucket_golden_scraper' ) From 65336799fa338e70d0688b4a55a55bcfe3d99e16 Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Tue, 29 Aug 2023 18:01:00 -0700 Subject: [PATCH 15/16] Formatting. --- 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 4ab449c16a759..4a12d2106d23f 100755 --- a/testing/run_tests.py +++ b/testing/run_tests.py @@ -953,7 +953,7 @@ def gather_clang_tidy_tests(build_dir): ) -def gather_build_bucket_golden_scraper_tests(build_dir): # pylint: invalid-name +def gather_build_bucket_golden_scraper_tests(build_dir): # pylint: invalid-name test_dir = os.path.join( BUILDROOT_DIR, 'flutter', 'tools', 'build_bucket_golden_scraper' ) From b84d415648abf2de3008d89ced5014b1cc30720d Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Wed, 30 Aug 2023 15:00:31 -0700 Subject: [PATCH 16/16] Remove pylint ignore. --- 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 4a12d2106d23f..20c3ade050974 100755 --- a/testing/run_tests.py +++ b/testing/run_tests.py @@ -953,7 +953,7 @@ def gather_clang_tidy_tests(build_dir): ) -def gather_build_bucket_golden_scraper_tests(build_dir): # pylint: invalid-name +def gather_build_bucket_golden_scraper_tests(build_dir): test_dir = os.path.join( BUILDROOT_DIR, 'flutter', 'tools', 'build_bucket_golden_scraper' )