From 15ce4dfd4476fc08075f3e0b836efad8cfc1a3a2 Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Wed, 21 Feb 2024 10:48:17 -0800 Subject: [PATCH 1/6] Add a throw statement for imgtestAdd non 0 exit codes. --- testing/skia_gold_client/lib/skia_gold_client.dart | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/testing/skia_gold_client/lib/skia_gold_client.dart b/testing/skia_gold_client/lib/skia_gold_client.dart index c4b25631179e2..38545eacbc589 100644 --- a/testing/skia_gold_client/lib/skia_gold_client.dart +++ b/testing/skia_gold_client/lib/skia_gold_client.dart @@ -257,11 +257,15 @@ class SkiaGoldClient { final ProcessResult result = await _runCommand(imgtestCommand); if (result.exitCode != 0) { - // We do not want to throw for non-zero exit codes here, as an intentional - // change or new golden file test expect non-zero exit codes. Logging here - // is meant to inform when an unexpected result occurs. - print('goldctl imgtest add stdout: ${result.stdout}'); - print('goldctl imgtest add stderr: ${result.stderr}'); + final StringBuffer buf = StringBuffer() + ..writeln('Skia Gold imgtest add failed.') + ..writeln('An error occurred when adding a golden file test with ') + ..writeln('goldctl.') + ..writeln() + ..writeln('Debug information for Gold:') + ..writeln('stdout: ${result.stdout}') + ..writeln('stderr: ${result.stderr}'); + throw Exception(buf.toString()); } else if (verbose) { print('stdout:\n${result.stdout}'); print('stderr:\n${result.stderr}'); From 1a2c59723725559cd73fe654aebfe0fec4e73784 Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Wed, 21 Feb 2024 11:16:27 -0800 Subject: [PATCH 2/6] ++ --- .../skia_gold_client/lib/skia_gold_client.dart | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/testing/skia_gold_client/lib/skia_gold_client.dart b/testing/skia_gold_client/lib/skia_gold_client.dart index 38545eacbc589..5f654a4bc37c8 100644 --- a/testing/skia_gold_client/lib/skia_gold_client.dart +++ b/testing/skia_gold_client/lib/skia_gold_client.dart @@ -257,12 +257,17 @@ class SkiaGoldClient { final ProcessResult result = await _runCommand(imgtestCommand); if (result.exitCode != 0) { - final StringBuffer buf = StringBuffer() - ..writeln('Skia Gold imgtest add failed.') - ..writeln('An error occurred when adding a golden file test with ') - ..writeln('goldctl.') +final StringBuffer buf = StringBuffer() + ..writeln('Skia Gold received an unapproved image in post-submit ') + ..writeln('testing. Golden file images in flutter/engine are triaged ') + ..writeln('in pre-submit during code review for the given PR.') ..writeln() - ..writeln('Debug information for Gold:') + ..writeln('Visit https://flutter-engine-gold.skia.org/ to view and approve ') + ..writeln('the image(s), or revert the associated change. For more ') + ..writeln('information, visit the wiki: ') + ..writeln('https://github.com/flutter/flutter/wiki/Writing-a-golden-file-test-for-package:flutter') + ..writeln() + ..writeln('Debug information for Gold --------------------------------') ..writeln('stdout: ${result.stdout}') ..writeln('stderr: ${result.stderr}'); throw Exception(buf.toString()); From d4e4d98eb3fea913f8e1a819596d982c35f07665 Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Wed, 21 Feb 2024 11:59:54 -0800 Subject: [PATCH 3/6] Reduce down the public API, prepare to add tests. --- .../lib/skia_gold_client.dart | 136 +++++++++--------- testing/skia_gold_client/pubspec.yaml | 9 ++ .../test/skia_gold_client_test.dart | 0 .../bin/golden_tests_harvester.dart | 23 +-- 4 files changed, 74 insertions(+), 94 deletions(-) create mode 100644 testing/skia_gold_client/test/skia_gold_client_test.dart diff --git a/testing/skia_gold_client/lib/skia_gold_client.dart b/testing/skia_gold_client/lib/skia_gold_client.dart index 5f654a4bc37c8..531cb4cea286b 100644 --- a/testing/skia_gold_client/lib/skia_gold_client.dart +++ b/testing/skia_gold_client/lib/skia_gold_client.dart @@ -3,7 +3,7 @@ // found in the LICENSE file. import 'dart:convert'; -import 'dart:io'; +import 'dart:io' as io; import 'package:crypto/crypto.dart'; import 'package:engine_repo_tools/engine_repo_tools.dart'; @@ -19,25 +19,34 @@ const String _instance = 'flutter-engine'; /// Whether the Skia Gold client is available and can be used in this /// environment. -bool get isSkiaGoldClientAvailable => Platform.environment.containsKey(_kGoldctlKey); +bool get isSkiaGoldClientAvailable => io.Platform.environment.containsKey(_kGoldctlKey); /// Returns true if the current environment is a LUCI builder. -bool get isLuciEnv => Platform.environment.containsKey(_kLuciEnvName); +bool get isLuciEnv => io.Platform.environment.containsKey(_kLuciEnvName); /// Whether the current task is run during a presubmit check. -bool get _isPresubmit => isLuciEnv && isSkiaGoldClientAvailable && Platform.environment.containsKey(_kPresubmitEnvName); +bool get _isPresubmit => isLuciEnv && isSkiaGoldClientAvailable && io.Platform.environment.containsKey(_kPresubmitEnvName); /// Whether the current task is run during a postsubmit check. -bool get _isPostsubmit => isLuciEnv && isSkiaGoldClientAvailable && !Platform.environment.containsKey(_kPresubmitEnvName); +bool get _isPostsubmit => isLuciEnv && isSkiaGoldClientAvailable && !io.Platform.environment.containsKey(_kPresubmitEnvName); /// A client for uploading image tests and making baseline requests to the /// Flutter Gold Dashboard. -class SkiaGoldClient { +interface class SkiaGoldClient { /// Creates a [SkiaGoldClient] with the given [workDirectory]. /// /// [dimensions] allows to add attributes about the environment /// used to generate the screenshots. - SkiaGoldClient(this.workDirectory, { this.dimensions, this.verbose = false}); + SkiaGoldClient( + this.workDirectory, { + this.dimensions, + this.verbose = false, + io.HttpClient? httpClient, + ProcessManager? processManager, + StringSink? stderr, + }) : httpClient = httpClient ?? io.HttpClient(), + process = processManager ?? const LocalProcessManager(), + _stderr = stderr ?? io.stderr; /// Whether to print verbose output from goldctl. /// @@ -45,18 +54,21 @@ class SkiaGoldClient { /// ordinarily be set to true. final bool verbose; + /// Where output is written for diagnostics. + final StringSink _stderr; + /// Allows to add attributes about the environment used to generate the screenshots. final Map? dimensions; /// A controller for launching sub-processes. - final ProcessManager process = const LocalProcessManager(); + final ProcessManager process; /// A client for making Http requests to the Flutter Gold dashboard. - final HttpClient httpClient = HttpClient(); + final io.HttpClient httpClient; /// The local [Directory] for the current test context. In this directory, the /// client will create image and JSON files for the `goldctl` tool to use. - final Directory workDirectory; + final io.Directory workDirectory; String get _tempPath => path.join(workDirectory.path, 'temp'); String get _keysPath => path.join(workDirectory.path, 'keys.json'); @@ -72,7 +84,7 @@ class SkiaGoldClient { /// Indicates whether the client has already been authorized to communicate /// with the Skia Gold backend. bool get _isAuthorized { - final File authFile = File(path.join(_tempPath, 'auth_opt.json')); + final io.File authFile = io.File(path.join(_tempPath, 'auth_opt.json')); if(authFile.existsSync()) { final String contents = authFile.readAsStringSync(); @@ -88,7 +100,7 @@ class SkiaGoldClient { isSkiaGoldClientAvailable, 'Trying to use `goldctl` in an environment where it is not available', ); - return Platform.environment[_kGoldctlKey]!; + return io.Platform.environment[_kGoldctlKey]!; } /// Prepares the local work space for golden file testing and calls the @@ -107,7 +119,7 @@ class SkiaGoldClient { '--luci', ]; - final ProcessResult result = await _runCommand(authCommand); + final io.ProcessResult result = await _runCommand(authCommand); if (result.exitCode != 0) { final StringBuffer buf = StringBuffer() @@ -120,12 +132,12 @@ class SkiaGoldClient { ..writeln('stderr: ${result.stderr}'); throw Exception(buf.toString()); } else if (verbose) { - print('stdout:\n${result.stdout}'); - print('stderr:\n${result.stderr}'); + _stderr.writeln('stdout:\n${result.stdout}'); + _stderr.writeln('stderr:\n${result.stderr}'); } } - Future _runCommand(List command) { + Future _runCommand(List command) { print(command.join(' ')); return process.run(command); } @@ -135,8 +147,8 @@ class SkiaGoldClient { /// The `imgtest` command collects and uploads test results to the Skia Gold /// backend, the `init` argument initializes the current test. Future _imgtestInit() async { - final File keys = File(_keysPath); - final File failures = File(_failuresPath); + final io.File keys = io.File(_keysPath); + final io.File failures = io.File(_failuresPath); await keys.writeAsString(_getKeysJSON()); await failures.create(); @@ -163,7 +175,7 @@ class SkiaGoldClient { throw Exception(buf.toString()); } - final ProcessResult result = await _runCommand(imgtestInitCommand); + final io.ProcessResult result = await _runCommand(imgtestInitCommand); if (result.exitCode != 0) { final StringBuffer buf = StringBuffer() @@ -176,8 +188,8 @@ class SkiaGoldClient { ..writeln('stderr: ${result.stderr}'); throw Exception(buf.toString()); } else if (verbose) { - print('stdout:\n${result.stdout}'); - print('stderr:\n${result.stderr}'); + _stderr.writeln('stdout:\n${result.stdout}'); + _stderr.writeln('stderr:\n${result.stderr}'); } } @@ -209,7 +221,7 @@ class SkiaGoldClient { /// allowed to be different. Future addImg( String testName, - File goldenFile, { + io.File goldenFile, { double differentPixelsRate = 0.01, int pixelColorDelta = 0, required int screenshotSize, @@ -235,7 +247,7 @@ class SkiaGoldClient { /// comparison being evaluated. Future _imgtestAdd( String testName, - File goldenFile, + io.File goldenFile, int screenshotSize, int pixelDeltaThreshold, double maxDifferentPixelsRate, @@ -247,17 +259,17 @@ class SkiaGoldClient { 'imgtest', 'add', if (verbose) '--verbose', '--work-dir', _tempPath, - '--test-name', cleanTestName(testName), + '--test-name', _cleanTestName(testName), '--png-file', goldenFile.path, // Otherwise post submit will not fail. '--passfail', ..._getMatchingArguments(testName, screenshotSize, pixelDeltaThreshold, maxDifferentPixelsRate), ]; - final ProcessResult result = await _runCommand(imgtestCommand); + final io.ProcessResult result = await _runCommand(imgtestCommand); if (result.exitCode != 0) { -final StringBuffer buf = StringBuffer() + final StringBuffer buf = StringBuffer() ..writeln('Skia Gold received an unapproved image in post-submit ') ..writeln('testing. Golden file images in flutter/engine are triaged ') ..writeln('in pre-submit during code review for the given PR.') @@ -272,8 +284,8 @@ final StringBuffer buf = StringBuffer() ..writeln('stderr: ${result.stderr}'); throw Exception(buf.toString()); } else if (verbose) { - print('stdout:\n${result.stdout}'); - print('stderr:\n${result.stderr}'); + _stderr.writeln('stdout:\n${result.stdout}'); + _stderr.writeln('stderr:\n${result.stderr}'); } } @@ -282,8 +294,8 @@ final StringBuffer buf = StringBuffer() /// The `imgtest` command collects and uploads test results to the Skia Gold /// backend, the `init` argument initializes the current tryjob. Future _tryjobInit() async { - final File keys = File(_keysPath); - final File failures = File(_failuresPath); + final io.File keys = io.File(_keysPath); + final io.File failures = io.File(_failuresPath); await keys.writeAsString(_getKeysJSON()); await failures.create(); @@ -301,7 +313,7 @@ final StringBuffer buf = StringBuffer() '--passfail', '--crs', 'github', '--patchset_id', commitHash, - ...getCIArguments(), + ..._getCIArguments(), ]; if (tryjobInitCommand.contains(null)) { @@ -313,7 +325,7 @@ final StringBuffer buf = StringBuffer() throw Exception(buf.toString()); } - final ProcessResult result = await _runCommand(tryjobInitCommand); + final io.ProcessResult result = await _runCommand(tryjobInitCommand); if (result.exitCode != 0) { final StringBuffer buf = StringBuffer() @@ -326,8 +338,8 @@ final StringBuffer buf = StringBuffer() ..writeln('stderr: ${result.stderr}'); throw Exception(buf.toString()); } else if (verbose) { - print('stdout:\n${result.stdout}'); - print('stderr:\n${result.stderr}'); + _stderr.writeln('stdout:\n${result.stdout}'); + _stderr.writeln('stderr:\n${result.stderr}'); } } @@ -342,7 +354,7 @@ final StringBuffer buf = StringBuffer() /// comparison being evaluated. Future _tryjobAdd( String testName, - File goldenFile, + io.File goldenFile, int screenshotSize, int pixelDeltaThreshold, double differentPixelsRate, @@ -354,12 +366,12 @@ final StringBuffer buf = StringBuffer() 'imgtest', 'add', if (verbose) '--verbose', '--work-dir', _tempPath, - '--test-name', cleanTestName(testName), + '--test-name', _cleanTestName(testName), '--png-file', goldenFile.path, ..._getMatchingArguments(testName, screenshotSize, pixelDeltaThreshold, differentPixelsRate), ]; - final ProcessResult result = await _runCommand(tryjobCommand); + final io.ProcessResult result = await _runCommand(tryjobCommand); final String resultStdout = result.stdout.toString(); if (result.exitCode != 0 && @@ -375,8 +387,8 @@ final StringBuffer buf = StringBuffer() ..writeln(); throw Exception(buf.toString()); } else if (verbose) { - print('stdout:\n${result.stdout}'); - print('stderr:\n${result.stderr}'); + _stderr.writeln('stdout:\n${result.stdout}'); + _stderr.writeln('stderr:\n${result.stderr}'); } } @@ -408,15 +420,15 @@ final StringBuffer buf = StringBuffer() /// at head. Future getExpectationForTest(String testName) async { late String? expectation; - final String traceID = getTraceID(testName); - await HttpOverrides.runWithHttpOverrides>(() async { + final String traceID = _getTraceID(testName); + await io.HttpOverrides.runWithHttpOverrides>(() async { final Uri requestForExpectations = Uri.parse( '$_skiaGoldHost/json/v2/latestpositivedigest/$traceID' ); late String rawResponse; try { - final HttpClientRequest request = await httpClient.getUrl(requestForExpectations); - final HttpClientResponse response = await request.close(); + final io.HttpClientRequest request = await httpClient.getUrl(requestForExpectations); + final io.HttpClientResponse response = await request.close(); rawResponse = await utf8.decodeStream(response); final dynamic jsonResponse = json.decode(rawResponse); if (jsonResponse is! Map) { @@ -424,7 +436,7 @@ final StringBuffer buf = StringBuffer() } expectation = jsonResponse['digest'] as String?; } on FormatException catch (error) { - print( + _stderr.writeln( 'Formatting error detected requesting expectations from Flutter Gold.\n' 'error: $error\n' 'url: $requestForExpectations\n' @@ -438,30 +450,10 @@ final StringBuffer buf = StringBuffer() return expectation; } - /// Returns a list of bytes representing the golden image retrieved from the - /// Skia Gold dashboard. - /// - /// The provided image hash represents an expectation from Skia Gold. - Future>getImageBytes(String imageHash) async { - final List imageBytes = []; - await HttpOverrides.runWithHttpOverrides>(() async { - final Uri requestForImage = Uri.parse( - '$_skiaGoldHost/img/images/$imageHash.png', - ); - - final HttpClientRequest request = await httpClient.getUrl(requestForImage); - final HttpClientResponse response = await request.close(); - await response.forEach((List bytes) => imageBytes.addAll(bytes)); - }, - SkiaGoldHttpOverrides(), - ); - return imageBytes; - } - /// Returns the current commit hash of the engine repository. Future _getCurrentCommit() async { final String engineCheckout = Engine.findWithin().flutterDir.path; - final ProcessResult revParse = await process.run( + final io.ProcessResult revParse = await process.run( ['git', 'rev-parse', 'HEAD'], workingDirectory: engineCheckout, ); @@ -479,7 +471,7 @@ final StringBuffer buf = StringBuffer() Map _getKeys() { final Map initialKeys = { 'CI': 'luci', - 'Platform': Platform.operatingSystem, + 'Platform': io.Platform.operatingSystem, }; if (dimensions != null) { initialKeys.addAll(dimensions!); @@ -494,15 +486,15 @@ final StringBuffer buf = StringBuffer() /// Removes the file extension from the [fileName] to represent the test name /// properly. - String cleanTestName(String fileName) { + static String _cleanTestName(String fileName) { return fileName.split(path.extension(fileName))[0]; } /// Returns a list of arguments for initializing a tryjob based on the testing /// environment. - List getCIArguments() { - final String jobId = Platform.environment['LOGDOG_STREAM_PREFIX']!.split('/').last; - final List refs = Platform.environment['GOLD_TRYJOB']!.split('/'); + List _getCIArguments() { + final String jobId = io.Platform.environment['LOGDOG_STREAM_PREFIX']!.split('/').last; + final List refs = io.Platform.environment['GOLD_TRYJOB']!.split('/'); final String pullRequest = refs[refs.length - 2]; return [ @@ -515,7 +507,7 @@ final StringBuffer buf = StringBuffer() /// Returns a trace id based on the current testing environment to lookup /// the latest positive digest on Skia Gold with a hex-encoded md5 hash of /// the image keys. - String getTraceID(String testName) { + String _getTraceID(String testName) { final Map keys = { ..._getKeys(), 'name': testName, @@ -528,4 +520,4 @@ final StringBuffer buf = StringBuffer() } /// Used to make HttpRequests during testing. -class SkiaGoldHttpOverrides extends HttpOverrides { } +class SkiaGoldHttpOverrides extends io.HttpOverrides { } diff --git a/testing/skia_gold_client/pubspec.yaml b/testing/skia_gold_client/pubspec.yaml index a728eb7ba69b8..e187b7c69c6b1 100644 --- a/testing/skia_gold_client/pubspec.yaml +++ b/testing/skia_gold_client/pubspec.yaml @@ -20,6 +20,9 @@ dependencies: engine_repo_tools: any process: any +dev_dependencies: + litetest: any + dependency_overrides: collection: path: ../../../third_party/dart/third_party/pkg/collection @@ -27,8 +30,12 @@ dependency_overrides: path: ../../../third_party/dart/third_party/pkg/crypto engine_repo_tools: path: ../../tools/pkg/engine_repo_tools + expect: + path: ../../../third_party/dart/pkg/expect file: path: ../../../third_party/dart/third_party/pkg/file/packages/file + litetest: + path: ../../testing/litetest meta: path: ../../../third_party/dart/pkg/meta path: @@ -37,5 +44,7 @@ dependency_overrides: path: ../../third_party/pkg/platform process: path: ../../third_party/pkg/process + smith: + path: ../../../third_party/dart/pkg/smith typed_data: path: ../../../third_party/dart/third_party/pkg/typed_data diff --git a/testing/skia_gold_client/test/skia_gold_client_test.dart b/testing/skia_gold_client/test/skia_gold_client_test.dart new file mode 100644 index 0000000000000..e69de29bb2d1d diff --git a/tools/golden_tests_harvester/bin/golden_tests_harvester.dart b/tools/golden_tests_harvester/bin/golden_tests_harvester.dart index 62a5f4525d856..78ba895fea2f6 100644 --- a/tools/golden_tests_harvester/bin/golden_tests_harvester.dart +++ b/tools/golden_tests_harvester/bin/golden_tests_harvester.dart @@ -40,31 +40,11 @@ class FakeSkiaGoldClient implements SkiaGoldClient { Logger.instance.log('auth dimensions:${dimensions ?? 'null'}'); } - @override - String cleanTestName(String fileName) { - throw UnimplementedError(); - } - - @override - List getCIArguments() { - throw UnimplementedError(); - } - @override Future getExpectationForTest(String testName) { throw UnimplementedError(); } - @override - Future> getImageBytes(String imageHash) { - throw UnimplementedError(); - } - - @override - String getTraceID(String testName) { - throw UnimplementedError(); - } - @override HttpClient get httpClient => throw UnimplementedError(); @@ -76,8 +56,7 @@ class FakeSkiaGoldClient implements SkiaGoldClient { } void _printUsage() { - Logger.instance - .log('dart run ./bin/golden_tests_harvester.dart '); + Logger.instance.log('dart run ./bin/golden_tests_harvester.dart '); } Future main(List arguments) async { From 2d9d2e5a67248ad140bd753c3892e7a083b98631 Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Wed, 21 Feb 2024 15:29:50 -0800 Subject: [PATCH 4/6] Tests. --- .../lib/skia_gold_client.dart | 97 ++- testing/skia_gold_client/pubspec.yaml | 6 + .../test/skia_gold_client_test.dart | 619 ++++++++++++++++++ .../bin/golden_tests_harvester.dart | 8 +- 4 files changed, 702 insertions(+), 28 deletions(-) diff --git a/testing/skia_gold_client/lib/skia_gold_client.dart b/testing/skia_gold_client/lib/skia_gold_client.dart index 531cb4cea286b..60cc5f33c9c64 100644 --- a/testing/skia_gold_client/lib/skia_gold_client.dart +++ b/testing/skia_gold_client/lib/skia_gold_client.dart @@ -7,6 +7,7 @@ import 'dart:io' as io; import 'package:crypto/crypto.dart'; import 'package:engine_repo_tools/engine_repo_tools.dart'; +import 'package:meta/meta.dart'; import 'package:path/path.dart' as path; import 'package:process/process.dart'; @@ -19,17 +20,11 @@ const String _instance = 'flutter-engine'; /// Whether the Skia Gold client is available and can be used in this /// environment. -bool get isSkiaGoldClientAvailable => io.Platform.environment.containsKey(_kGoldctlKey); +bool get isSkiaGoldClientAvailable => SkiaGoldClient.isAvailable(); /// Returns true if the current environment is a LUCI builder. bool get isLuciEnv => io.Platform.environment.containsKey(_kLuciEnvName); -/// Whether the current task is run during a presubmit check. -bool get _isPresubmit => isLuciEnv && isSkiaGoldClientAvailable && io.Platform.environment.containsKey(_kPresubmitEnvName); - -/// Whether the current task is run during a postsubmit check. -bool get _isPostsubmit => isLuciEnv && isSkiaGoldClientAvailable && !io.Platform.environment.containsKey(_kPresubmitEnvName); - /// A client for uploading image tests and making baseline requests to the /// Flutter Gold Dashboard. interface class SkiaGoldClient { @@ -44,9 +39,42 @@ interface class SkiaGoldClient { io.HttpClient? httpClient, ProcessManager? processManager, StringSink? stderr, + Map? environment, }) : httpClient = httpClient ?? io.HttpClient(), process = processManager ?? const LocalProcessManager(), - _stderr = stderr ?? io.stderr; + _stderr = stderr ?? io.stderr, + _environment = environment ?? io.Platform.environment; + + /// Whether the client is available and can be used in this environment. + static bool isAvailable({ + Map? environment, + }) { + final String? result = (environment ?? io.Platform.environment)[_kGoldctlKey]; + return result != null && result.isNotEmpty; + } + + /// Returns true if the current environment is a LUCI builder. + static bool isLuciEnv({ + Map? environment, + }) { + return (environment ?? io.Platform.environment).containsKey(_kLuciEnvName); + } + + /// Whether the current environment is a presubmit job. + bool get _isPresubmit { + return + isLuciEnv(environment: _environment) && + isAvailable(environment: _environment) && + _environment.containsKey(_kPresubmitEnvName); + } + + /// Whether the current environment is a postsubmit job. + bool get _isPostsubmit { + return + isLuciEnv(environment: _environment) && + isAvailable(environment: _environment) && + !_environment.containsKey(_kPresubmitEnvName); + } /// Whether to print verbose output from goldctl. /// @@ -54,6 +82,9 @@ interface class SkiaGoldClient { /// ordinarily be set to true. final bool verbose; + /// Environment variables for the currently running process. + final Map _environment; + /// Where output is written for diagnostics. final StringSink _stderr; @@ -86,7 +117,7 @@ interface class SkiaGoldClient { bool get _isAuthorized { final io.File authFile = io.File(path.join(_tempPath, 'auth_opt.json')); - if(authFile.existsSync()) { + if (authFile.existsSync()) { final String contents = authFile.readAsStringSync(); final Map decoded = json.decode(contents) as Map; return !(decoded['GSUtil'] as bool); @@ -97,10 +128,14 @@ interface class SkiaGoldClient { /// The path to the local [Directory] where the `goldctl` tool is hosted. String get _goldctl { assert( - isSkiaGoldClientAvailable, + isAvailable(environment: _environment), 'Trying to use `goldctl` in an environment where it is not available', ); - return io.Platform.environment[_kGoldctlKey]!; + final String? result = _environment[_kGoldctlKey]; + if (result == null || result.isEmpty) { + throw StateError('The environment variable $_kGoldctlKey is not set.'); + } + return result; } /// Prepares the local work space for golden file testing and calls the @@ -138,7 +173,6 @@ interface class SkiaGoldClient { } Future _runCommand(List command) { - print(command.join(' ')); return process.run(command); } @@ -227,7 +261,6 @@ interface class SkiaGoldClient { required int screenshotSize, }) async { assert(_isPresubmit || _isPostsubmit); - if (_isPresubmit) { await _tryjobAdd(testName, goldenFile, screenshotSize, pixelColorDelta, differentPixelsRate); } @@ -256,11 +289,16 @@ interface class SkiaGoldClient { final List imgtestCommand = [ _goldctl, - 'imgtest', 'add', - if (verbose) '--verbose', - '--work-dir', _tempPath, - '--test-name', _cleanTestName(testName), - '--png-file', goldenFile.path, + 'imgtest', + 'add', + if (verbose) + '--verbose', + '--work-dir', + _tempPath, + '--test-name', + _cleanTestName(testName), + '--png-file', + goldenFile.path, // Otherwise post submit will not fail. '--passfail', ..._getMatchingArguments(testName, screenshotSize, pixelDeltaThreshold, maxDifferentPixelsRate), @@ -363,11 +401,15 @@ interface class SkiaGoldClient { final List tryjobCommand = [ _goldctl, - 'imgtest', 'add', + 'imgtest', + 'add', if (verbose) '--verbose', - '--work-dir', _tempPath, - '--test-name', _cleanTestName(testName), - '--png-file', goldenFile.path, + '--work-dir', + _tempPath, + '--test-name', + _cleanTestName(testName), + '--png-file', + goldenFile.path, ..._getMatchingArguments(testName, screenshotSize, pixelDeltaThreshold, differentPixelsRate), ]; @@ -420,7 +462,7 @@ interface class SkiaGoldClient { /// at head. Future getExpectationForTest(String testName) async { late String? expectation; - final String traceID = _getTraceID(testName); + final String traceID = getTraceID(testName); await io.HttpOverrides.runWithHttpOverrides>(() async { final Uri requestForExpectations = Uri.parse( '$_skiaGoldHost/json/v2/latestpositivedigest/$traceID' @@ -487,14 +529,14 @@ interface class SkiaGoldClient { /// Removes the file extension from the [fileName] to represent the test name /// properly. static String _cleanTestName(String fileName) { - return fileName.split(path.extension(fileName))[0]; + return path.basenameWithoutExtension(fileName); } /// Returns a list of arguments for initializing a tryjob based on the testing /// environment. List _getCIArguments() { - final String jobId = io.Platform.environment['LOGDOG_STREAM_PREFIX']!.split('/').last; - final List refs = io.Platform.environment['GOLD_TRYJOB']!.split('/'); + final String jobId = _environment['LOGDOG_STREAM_PREFIX']!.split('/').last; + final List refs = _environment['GOLD_TRYJOB']!.split('/'); final String pullRequest = refs[refs.length - 2]; return [ @@ -507,7 +549,8 @@ interface class SkiaGoldClient { /// Returns a trace id based on the current testing environment to lookup /// the latest positive digest on Skia Gold with a hex-encoded md5 hash of /// the image keys. - String _getTraceID(String testName) { + @visibleForTesting + String getTraceID(String testName) { final Map keys = { ..._getKeys(), 'name': testName, diff --git a/testing/skia_gold_client/pubspec.yaml b/testing/skia_gold_client/pubspec.yaml index e187b7c69c6b1..e1464c04ffcf6 100644 --- a/testing/skia_gold_client/pubspec.yaml +++ b/testing/skia_gold_client/pubspec.yaml @@ -16,14 +16,18 @@ environment: # relative to this directory into //third_party/dart, or //third_party/pkg dependencies: crypto: any + meta: any path: any engine_repo_tools: any process: any dev_dependencies: litetest: any + process_fakes: any dependency_overrides: + async_helper: + path: ../../../third_party/dart/pkg/async_helper collection: path: ../../../third_party/dart/third_party/pkg/collection crypto: @@ -44,6 +48,8 @@ dependency_overrides: path: ../../third_party/pkg/platform process: path: ../../third_party/pkg/process + process_fakes: + path: ../../tools/pkg/process_fakes smith: path: ../../../third_party/dart/pkg/smith typed_data: diff --git a/testing/skia_gold_client/test/skia_gold_client_test.dart b/testing/skia_gold_client/test/skia_gold_client_test.dart index e69de29bb2d1d..1d094d0a48e94 100644 --- a/testing/skia_gold_client/test/skia_gold_client_test.dart +++ b/testing/skia_gold_client/test/skia_gold_client_test.dart @@ -0,0 +1,619 @@ +import 'dart:async'; +import 'dart:convert'; +import 'dart:io' as io; +import 'dart:typed_data'; + +import 'package:litetest/litetest.dart'; +import 'package:path/path.dart' as p; +import 'package:process_fakes/process_fakes.dart'; +import 'package:skia_gold_client/skia_gold_client.dart'; + +void main() { + /// A mock commit hash that is used to simulate a successful git call. + const String mockCommitHash = '1234567890abcdef'; + + /// Simulating what a presubmit environment would look like. + const Map presubmitEnv = { + 'GOLDCTL': 'python tools/goldctl.py', + 'GOLD_TRYJOB': 'flutter/engine/1234567890', + 'LOGDOG_STREAM_PREFIX': 'buildbucket/cr-buildbucket.appspot.com/1234567890/+/logdog', + 'LUCI_CONTEXT': '{}', + }; + + /// Simulating what a postsubmit environment would look like. + const Map postsubmitEnv = { + 'GOLDCTL': 'python tools/goldctl.py', + 'LOGDOG_STREAM_PREFIX': 'buildbucket/cr-buildbucket.appspot.com/1234567890/+/logdog', + 'LUCI_CONTEXT': '{}' + }; + + /// Simulating what a local environment would look like. + const Map localEnv = {}; + + /// Creates a [SkiaGoldClient] with the given [dimensions] and [verbose] flag. + /// + /// Optionally, the [onRun] function can be provided to handle the execution + /// of the command-line tool. If not provided, it throws an + /// [UnsupportedError] by default. + /// + /// Side-effects of the client can be observed through the test fixture. + SkiaGoldClient createClient( + _TestFixture fixture, { + required Map environment, + Map? dimensions, + bool verbose = false, + io.ProcessResult Function(List command) onRun = _runUnhandled, + }) { + return SkiaGoldClient( + fixture.workDirectory, + dimensions: dimensions, + httpClient: fixture.httpClient, + processManager: FakeProcessManager( + onRun: onRun, + ), + verbose: verbose, + stderr: fixture.outputSink, + environment: environment, + ); + } + + /// Creates a `temp/auth_opt.json` file in the working directory. + /// + /// This simulates what the goldctl tool does when it runs. + void createAuthOptDotJson(String workDirectory) { + final io.File authOptDotJson = io.File(p.join(workDirectory, 'temp', 'auth_opt.json')); + authOptDotJson.createSync(recursive: true); + authOptDotJson.writeAsStringSync('{"GSUtil": false}'); + } + + test('fails if GOLDCTL is not set', () async { + final _TestFixture fixture = _TestFixture(); + try { + final SkiaGoldClient client = createClient( + fixture, + environment: localEnv, + ); + try { + await client.auth(); + fail('auth should fail if GOLDCTL is not set'); + } catch (error) { + expect('$error', contains('GOLDCTL is not set')); + } + } finally { + fixture.dispose(); + } + }); + + test('auth executes successfully', () async { + final _TestFixture fixture = _TestFixture(); + try { + final SkiaGoldClient client = createClient( + fixture, + environment: presubmitEnv, + onRun: (List command) { + expect(command, [ + 'python tools/goldctl.py', + 'auth', + '--work-dir', + p.join(fixture.workDirectory.path, 'temp'), + '--luci', + ]); + createAuthOptDotJson(fixture.workDirectory.path); + return io.ProcessResult(0, 0, '', ''); + }, + ); + await client.auth(); + } finally { + fixture.dispose(); + } + }); + + test('auth is only invoked once per instance', () async { + final _TestFixture fixture = _TestFixture(); + try { + int callsToGoldctl = 0; + final SkiaGoldClient client = createClient( + fixture, + environment: presubmitEnv, + onRun: (List command) { + callsToGoldctl++; + expect(command, [ + 'python tools/goldctl.py', + 'auth', + '--work-dir', + p.join(fixture.workDirectory.path, 'temp'), + '--luci', + ]); + createAuthOptDotJson(fixture.workDirectory.path); + return io.ProcessResult(0, 0, '', ''); + }, + ); + + await client.auth(); + await client.auth(); + expect(callsToGoldctl, 1); + } finally { + fixture.dispose(); + } + }); + + test('auth executes successfully with verbose logging', () async { + final _TestFixture fixture = _TestFixture(); + try { + final SkiaGoldClient client = createClient( + fixture, + environment: presubmitEnv, + verbose: true, + onRun: (List command) { + expect(command, [ + 'python tools/goldctl.py', + 'auth', + '--verbose', + '--work-dir', + p.join(fixture.workDirectory.path, 'temp'), + '--luci', + ]); + return io.ProcessResult(0, 0, 'stdout', 'stderr'); + }, + ); + + await client.auth(); + expect(fixture.outputSink.toString(), contains('stdout:\nstdout')); + expect(fixture.outputSink.toString(), contains('stderr:\nstderr')); + } finally { + fixture.dispose(); + } + }); + + test('auth fails', () async { + final _TestFixture fixture = _TestFixture(); + try { + final SkiaGoldClient client = createClient( + fixture, + environment: presubmitEnv, + onRun: (List command) { + return io.ProcessResult(1, 0, '', 'error-text'); + }, + ); + + try { + await client.auth(); + } catch (error) { + expect('$error', contains('Skia Gold authorization failed.')); + expect('$error', contains('error-text')); + } + } finally { + fixture.dispose(); + } + }); + + test('addImg [pre-submit] executes successfully', () async { + final _TestFixture fixture = _TestFixture(); + try { + final SkiaGoldClient client = createClient( + fixture, + environment: presubmitEnv, + onRun: (List command) { + if (command case ['git', ...]) { + return io.ProcessResult(0, 0, mockCommitHash, ''); + } + if (command case ['python tools/goldctl.py', 'imgtest', 'init', ...]) { + return io.ProcessResult(0, 0, '', ''); + } + expect(command, [ + 'python tools/goldctl.py', + 'imgtest', + 'add', + '--work-dir', + p.join(fixture.workDirectory.path, 'temp'), + '--test-name', + 'test-name', + '--png-file', + p.join(fixture.workDirectory.path, 'temp', 'golden.png'), + '--add-test-optional-key', + 'image_matching_algorithm:fuzzy', + '--add-test-optional-key', + 'fuzzy_max_different_pixels:10', + '--add-test-optional-key', + 'fuzzy_pixel_delta_threshold:0', + ]); + return io.ProcessResult(0, 0, '', ''); + }, + ); + + await client.addImg( + 'test-name.foo', + io.File(p.join(fixture.workDirectory.path, 'temp', 'golden.png')), + screenshotSize: 1000, + ); + } finally { + fixture.dispose(); + } + }); + + test('addImg [pre-submit] executes successfully with verbose logging', () async { + final _TestFixture fixture = _TestFixture(); + try { + final SkiaGoldClient client = createClient( + fixture, + environment: presubmitEnv, + verbose: true, + onRun: (List command) { + if (command case ['git', ...]) { + return io.ProcessResult(0, 0, mockCommitHash, ''); + } + if (command case ['python tools/goldctl.py', 'imgtest', 'init', ...]) { + return io.ProcessResult(0, 0, '', ''); + } + expect(command, [ + 'python tools/goldctl.py', + 'imgtest', + 'add', + '--verbose', + '--work-dir', + p.join(fixture.workDirectory.path, 'temp'), + '--test-name', + 'test-name', + '--png-file', + p.join(fixture.workDirectory.path, 'temp', 'golden.png'), + '--add-test-optional-key', + 'image_matching_algorithm:fuzzy', + '--add-test-optional-key', + 'fuzzy_max_different_pixels:10', + '--add-test-optional-key', + 'fuzzy_pixel_delta_threshold:0', + ]); + return io.ProcessResult(0, 0, 'stdout', 'stderr'); + }, + ); + + await client.addImg( + 'test-name.foo', + io.File(p.join(fixture.workDirectory.path, 'temp', 'golden.png')), + screenshotSize: 1000, + ); + + expect(fixture.outputSink.toString(), contains('stdout:\nstdout')); + expect(fixture.outputSink.toString(), contains('stderr:\nstderr')); + } finally { + fixture.dispose(); + } + }); + + // A success case (exit code 0) with a message of "Untriaged" is OK. + test('addImg [pre-submit] succeeds but has an untriaged image', () async { + final _TestFixture fixture = _TestFixture(); + try { + final SkiaGoldClient client = createClient( + fixture, + environment: presubmitEnv, + onRun: (List command) { + if (command case ['git', ...]) { + return io.ProcessResult(0, 0, mockCommitHash, ''); + } + if (command case ['python tools/goldctl.py', 'imgtest', 'init', ...]) { + return io.ProcessResult(0, 0, '', ''); + } + expect(command, [ + 'python tools/goldctl.py', + 'imgtest', + 'add', + '--work-dir', + p.join(fixture.workDirectory.path, 'temp'), + '--test-name', + 'test-name', + '--png-file', + p.join(fixture.workDirectory.path, 'temp', 'golden.png'), + '--add-test-optional-key', + 'image_matching_algorithm:fuzzy', + '--add-test-optional-key', + 'fuzzy_max_different_pixels:10', + '--add-test-optional-key', + 'fuzzy_pixel_delta_threshold:0', + ]); + // Intentionally returning a non-zero exit code. + return io.ProcessResult(0, 1, 'Untriaged', ''); + }, + ); + + await client.addImg( + 'test-name.foo', + io.File(p.join(fixture.workDirectory.path, 'temp', 'golden.png')), + screenshotSize: 1000, + ); + } finally { + fixture.dispose(); + } + }); + + test('addImg [pre-submit] fails due to an unexpected error', () async { + final _TestFixture fixture = _TestFixture(); + try { + final SkiaGoldClient client = createClient( + fixture, + environment: presubmitEnv, + onRun: (List command) { + if (command case ['git', ...]) { + return io.ProcessResult(0, 0, mockCommitHash, ''); + } + if (command case ['python tools/goldctl.py', 'imgtest', 'init', ...]) { + return io.ProcessResult(0, 0, '', ''); + } + return io.ProcessResult(1, 0, '', 'error-text'); + }, + ); + + try { + await client.addImg( + 'test-name.foo', + io.File(p.join(fixture.workDirectory.path, 'temp', 'golden.png')), + screenshotSize: 1000, + ); + } catch (error) { + expect('$error', contains('Skia Gold image test failed.')); + expect('$error', contains('error-text')); + } + } finally { + fixture.dispose(); + } + }); + + test('addImg [post-submit] executes successfully', () async { + final _TestFixture fixture = _TestFixture(); + try { + final SkiaGoldClient client = createClient( + fixture, + environment: postsubmitEnv, + onRun: (List command) { + if (command case ['git', ...]) { + return io.ProcessResult(0, 0, mockCommitHash, ''); + } + if (command case ['python tools/goldctl.py', 'imgtest', 'init', ...]) { + return io.ProcessResult(0, 0, '', ''); + } + expect(command, [ + 'python tools/goldctl.py', + 'imgtest', + 'add', + '--work-dir', + p.join(fixture.workDirectory.path, 'temp'), + '--test-name', + 'test-name', + '--png-file', + p.join(fixture.workDirectory.path, 'temp', 'golden.png'), + '--passfail', + '--add-test-optional-key', + 'image_matching_algorithm:fuzzy', + '--add-test-optional-key', + 'fuzzy_max_different_pixels:10', + '--add-test-optional-key', + 'fuzzy_pixel_delta_threshold:0', + ]); + return io.ProcessResult(0, 0, '', ''); + }, + ); + + await client.addImg( + 'test-name.foo', + io.File(p.join(fixture.workDirectory.path, 'temp', 'golden.png')), + screenshotSize: 1000, + ); + } finally { + fixture.dispose(); + } + }); + + test('addImg [post-submit] executes successfully with verbose logging', () async { + final _TestFixture fixture = _TestFixture(); + try { + final SkiaGoldClient client = createClient( + fixture, + environment: postsubmitEnv, + verbose: true, + onRun: (List command) { + if (command case ['git', ...]) { + return io.ProcessResult(0, 0, mockCommitHash, ''); + } + if (command case ['python tools/goldctl.py', 'imgtest', 'init', ...]) { + return io.ProcessResult(0, 0, '', ''); + } + expect(command, [ + 'python tools/goldctl.py', + 'imgtest', + 'add', + '--verbose', + '--work-dir', + p.join(fixture.workDirectory.path, 'temp'), + '--test-name', + 'test-name', + '--png-file', + p.join(fixture.workDirectory.path, 'temp', 'golden.png'), + '--passfail', + '--add-test-optional-key', + 'image_matching_algorithm:fuzzy', + '--add-test-optional-key', + 'fuzzy_max_different_pixels:10', + '--add-test-optional-key', + 'fuzzy_pixel_delta_threshold:0', + ]); + return io.ProcessResult(0, 0, 'stdout', 'stderr'); + }, + ); + + await client.addImg( + 'test-name.foo', + io.File(p.join(fixture.workDirectory.path, 'temp', 'golden.png')), + screenshotSize: 1000, + ); + + expect(fixture.outputSink.toString(), contains('stdout:\nstdout')); + expect(fixture.outputSink.toString(), contains('stderr:\nstderr')); + } finally { + fixture.dispose(); + } + }); + + test('addImg [post-submit] fails due to an unapproved image', () async { + final _TestFixture fixture = _TestFixture(); + try { + final SkiaGoldClient client = createClient( + fixture, + environment: postsubmitEnv, + onRun: (List command) { + if (command case ['git', ...]) { + return io.ProcessResult(0, 0, mockCommitHash, ''); + } + if (command case ['python tools/goldctl.py', 'imgtest', 'init', ...]) { + return io.ProcessResult(0, 0, '', ''); + } + return io.ProcessResult(1, 0, '', 'error-text'); + }, + ); + + try { + await client.addImg( + 'test-name.foo', + io.File(p.join(fixture.workDirectory.path, 'temp', 'golden.png')), + screenshotSize: 1000, + ); + } catch (error) { + expect('$error', contains('Skia Gold image test failed.')); + expect('$error', contains('error-text')); + } + } finally { + fixture.dispose(); + } + }); + + test('getExpectationsForTest returns the latest positive digest', () async { + final _TestFixture fixture = _TestFixture(); + try { + final SkiaGoldClient client = createClient( + fixture, + environment: presubmitEnv, + onRun: (List command) { + expect(command, [ + 'python tools/goldctl.py', + 'imgtest', + 'get', + '--work-dir', + p.join(fixture.workDirectory.path, 'temp'), + '--test-name', + 'test-name', + ]); + return io.ProcessResult(0, 0, '{"digest":"digest"}', ''); + }, + ); + + final String hash = client.getTraceID('test-name'); + fixture.httpClient.setJsonResponse( + Uri.parse('https://flutter-engine-gold.skia.org/json/v2/latestpositivedigest/$hash'), + { + 'digest': 'digest', + }, + ); + + final String? digest = await client.getExpectationForTest('test-name'); + expect(digest, 'digest'); + } finally { + fixture.dispose(); + } + }); +} + +final class _TestFixture { + _TestFixture(); + + final io.Directory workDirectory = io.Directory.systemTemp.createTempSync('skia_gold_client_test'); + final _FakeHttpClient httpClient = _FakeHttpClient(); + final StringSink outputSink = StringBuffer(); + + void dispose() { + workDirectory.deleteSync(recursive: true); + } +} + +io.ProcessResult _runUnhandled(List command) { + throw UnimplementedError('Unhandled run: ${command.join(' ')}'); +} + +/// An in-memory fake of [io.HttpClient] that allows [getUrl] to be mocked. +/// +/// This class is used to simulate a response from the server. +/// +/// Any other methods called on this class will throw a [NoSuchMethodError]. +final class _FakeHttpClient implements io.HttpClient { + final Map _expectedResponses = {}; + + /// Sets an expected response for the given [request] to [jsonEncodableValue]. + /// + /// This method is used to simulate a response from the server. + void setJsonResponse(Uri request, Object? jsonEncodableValue) { + _expectedResponses[request] = jsonEncodableValue; + } + + @override + Future getUrl(Uri url) async { + final Object? response = _expectedResponses[url]; + if (response == null) { + throw StateError('No request expected for $url'); + } + return _FakeHttpClientRequest.withJsonResponse(response); + } + + @override + Object? noSuchMethod(Invocation invocation) { + return super.noSuchMethod(invocation); + } +} + +final class _FakeHttpClientRequest implements io.HttpClientRequest { + factory _FakeHttpClientRequest.withJsonResponse(Object? jsonResponse) { + final Uint8List bytes = utf8.encoder.convert(jsonEncode(jsonResponse)); + return _FakeHttpClientRequest._(_FakeHttpClientResponse(bytes)); + } + + _FakeHttpClientRequest._(this._response); + + final io.HttpClientResponse _response; + + @override + Future close() async { + return _response; + } + + @override + Object? noSuchMethod(Invocation invocation) { + return super.noSuchMethod(invocation); + } +} + +final class _FakeHttpClientResponse extends Stream> + implements io.HttpClientResponse { + _FakeHttpClientResponse(this._bytes); + + final Uint8List _bytes; + + @override + StreamSubscription> listen( + void Function(List event)? onData, { + Function? onError, + void Function()? onDone, + bool? cancelOnError, + }) { + return Stream>.fromIterable(>[_bytes]).listen( + onData, + onError: onError, + onDone: onDone, + cancelOnError: cancelOnError, + ); + } + + @override + int get statusCode => 200; + + @override + Object? noSuchMethod(Invocation invocation) { + return super.noSuchMethod(invocation); + } +} diff --git a/tools/golden_tests_harvester/bin/golden_tests_harvester.dart b/tools/golden_tests_harvester/bin/golden_tests_harvester.dart index 78ba895fea2f6..021d97505cb9a 100644 --- a/tools/golden_tests_harvester/bin/golden_tests_harvester.dart +++ b/tools/golden_tests_harvester/bin/golden_tests_harvester.dart @@ -45,6 +45,11 @@ class FakeSkiaGoldClient implements SkiaGoldClient { throw UnimplementedError(); } + @override + String getTraceID(String testName) { + throw UnimplementedError(); + } + @override HttpClient get httpClient => throw UnimplementedError(); @@ -56,7 +61,8 @@ class FakeSkiaGoldClient implements SkiaGoldClient { } void _printUsage() { - Logger.instance.log('dart run ./bin/golden_tests_harvester.dart '); + Logger.instance + .log('dart run ./bin/golden_tests_harvester.dart '); } Future main(List arguments) async { From 9f5cf2d5bbf4c2ddaba2e3444535674e202e4329 Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Wed, 21 Feb 2024 15:30:09 -0800 Subject: [PATCH 5/6] Format. --- testing/skia_gold_client/lib/skia_gold_client.dart | 6 +++--- testing/skia_gold_client/pubspec.yaml | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/testing/skia_gold_client/lib/skia_gold_client.dart b/testing/skia_gold_client/lib/skia_gold_client.dart index 60cc5f33c9c64..5dde4d8608003 100644 --- a/testing/skia_gold_client/lib/skia_gold_client.dart +++ b/testing/skia_gold_client/lib/skia_gold_client.dart @@ -34,7 +34,7 @@ interface class SkiaGoldClient { /// used to generate the screenshots. SkiaGoldClient( this.workDirectory, { - this.dimensions, + this.dimensions, this.verbose = false, io.HttpClient? httpClient, ProcessManager? processManager, @@ -62,7 +62,7 @@ interface class SkiaGoldClient { /// Whether the current environment is a presubmit job. bool get _isPresubmit { - return + return isLuciEnv(environment: _environment) && isAvailable(environment: _environment) && _environment.containsKey(_kPresubmitEnvName); @@ -70,7 +70,7 @@ interface class SkiaGoldClient { /// Whether the current environment is a postsubmit job. bool get _isPostsubmit { - return + return isLuciEnv(environment: _environment) && isAvailable(environment: _environment) && !_environment.containsKey(_kPresubmitEnvName); diff --git a/testing/skia_gold_client/pubspec.yaml b/testing/skia_gold_client/pubspec.yaml index e1464c04ffcf6..a7ba15de53abb 100644 --- a/testing/skia_gold_client/pubspec.yaml +++ b/testing/skia_gold_client/pubspec.yaml @@ -21,7 +21,7 @@ dependencies: engine_repo_tools: any process: any -dev_dependencies: +dev_dependencies: litetest: any process_fakes: any From d0a1a92921a58f3715d3aafac7ffa194ef30f40b Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Wed, 21 Feb 2024 15:32:54 -0800 Subject: [PATCH 6/6] Run tests on CI. --- testing/run_tests.py | 1 + 1 file changed, 1 insertion(+) diff --git a/testing/run_tests.py b/testing/run_tests.py index 5107ec49d18e2..966a02778e34d 100755 --- a/testing/run_tests.py +++ b/testing/run_tests.py @@ -908,6 +908,7 @@ def build_dart_host_test_list(build_dir): ], ), (os.path.join('flutter', 'testing', 'litetest'), []), + (os.path.join('flutter', 'testing', 'skia_gold_client'), []), ( os.path.join('flutter', 'tools', 'api_check'), [os.path.join(BUILDROOT_DIR, 'flutter')],