From b3f596d627f79d304c65bf179aa49dae31c0aacb Mon Sep 17 00:00:00 2001 From: Mouad Debbar Date: Fri, 14 Apr 2023 13:02:20 -0400 Subject: [PATCH 1/3] [web] Don't run goldctl init more than once --- .../lib/skia_gold_client.dart | 25 +++++++------------ 1 file changed, 9 insertions(+), 16 deletions(-) diff --git a/testing/skia_gold_client/lib/skia_gold_client.dart b/testing/skia_gold_client/lib/skia_gold_client.dart index b7d23abdb37c9..982fb591bb734 100644 --- a/testing/skia_gold_client/lib/skia_gold_client.dart +++ b/testing/skia_gold_client/lib/skia_gold_client.dart @@ -55,10 +55,6 @@ class SkiaGoldClient { String get _keysPath => path.join(workDirectory.path, 'keys.json'); String get _failuresPath => path.join(workDirectory.path, 'failures.json'); - /// Indicates whether the `goldctl` tool has been initialized for the current - /// test context. - bool _isInitialized = false; - /// Indicates whether the client has already been authorized to communicate /// with the Skia Gold backend. bool get _isAuthorized { @@ -121,10 +117,6 @@ 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 { - if (_isInitialized) { - return; - } - final File keys = File(_keysPath); final File failures = File(_failuresPath); @@ -165,7 +157,6 @@ class SkiaGoldClient { ..writeln('stderr: ${result.stderr}'); throw Exception(buf.toString()); } - _isInitialized = true; } /// Executes the `imgtest add` command in the `goldctl` tool. @@ -226,7 +217,7 @@ class SkiaGoldClient { int pixelDeltaThreshold, double maxDifferentPixelsRate, ) async { - await _imgtestInit(); + await _callOnce(_imgtestInit); final List imgtestCommand = [ _goldctl, @@ -253,10 +244,6 @@ class SkiaGoldClient { /// The `imgtest` command collects and uploads test results to the Skia Gold /// backend, the `init` argument initializes the current tryjob. Future _tryjobInit() async { - if (_isInitialized) { - return; - } - final File keys = File(_keysPath); final File failures = File(_failuresPath); @@ -300,7 +287,6 @@ class SkiaGoldClient { ..writeln('stderr: ${result.stderr}'); throw Exception(buf.toString()); } - _isInitialized = true; } /// Executes the `imgtest add` command in the `goldctl` tool for tryjobs. @@ -319,7 +305,7 @@ class SkiaGoldClient { int pixelDeltaThreshold, double differentPixelsRate, ) async { - await _tryjobInit(); + await _callOnce(_tryjobInit); final List tryjobCommand = [ _goldctl, @@ -495,5 +481,12 @@ class SkiaGoldClient { } } +Future? _oneResult; +Future _callOnce(Future Function() callback) async { + // If a call has already been made, return the result of that call. + _oneResult ??= callback(); + return _oneResult; +} + /// Used to make HttpRequests during testing. class SkiaGoldHttpOverrides extends HttpOverrides { } From e45eef88899014e4bf4851502a5d9b3ba6848e0f Mon Sep 17 00:00:00 2001 From: Mouad Debbar Date: Fri, 14 Apr 2023 14:11:56 -0400 Subject: [PATCH 2/3] make it once per client --- .../skia_gold_client/lib/skia_gold_client.dart | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/testing/skia_gold_client/lib/skia_gold_client.dart b/testing/skia_gold_client/lib/skia_gold_client.dart index 982fb591bb734..4506c47d57bf8 100644 --- a/testing/skia_gold_client/lib/skia_gold_client.dart +++ b/testing/skia_gold_client/lib/skia_gold_client.dart @@ -55,6 +55,13 @@ class SkiaGoldClient { String get _keysPath => path.join(workDirectory.path, 'keys.json'); String get _failuresPath => path.join(workDirectory.path, 'failures.json'); + Future? _initResult; + Future _initOnce(Future Function() callback) async { + // If a call has already been made, return the result of that call. + _initResult ??= callback(); + return _initResult; + } + /// Indicates whether the client has already been authorized to communicate /// with the Skia Gold backend. bool get _isAuthorized { @@ -217,7 +224,7 @@ class SkiaGoldClient { int pixelDeltaThreshold, double maxDifferentPixelsRate, ) async { - await _callOnce(_imgtestInit); + await _initOnce(_imgtestInit); final List imgtestCommand = [ _goldctl, @@ -305,7 +312,7 @@ class SkiaGoldClient { int pixelDeltaThreshold, double differentPixelsRate, ) async { - await _callOnce(_tryjobInit); + await _initOnce(_tryjobInit); final List tryjobCommand = [ _goldctl, @@ -481,12 +488,5 @@ class SkiaGoldClient { } } -Future? _oneResult; -Future _callOnce(Future Function() callback) async { - // If a call has already been made, return the result of that call. - _oneResult ??= callback(); - return _oneResult; -} - /// Used to make HttpRequests during testing. class SkiaGoldHttpOverrides extends HttpOverrides { } From 74b22ee18dd57eee3a841c765af4eb44e2617f72 Mon Sep 17 00:00:00 2001 From: Mouad Debbar Date: Fri, 14 Apr 2023 15:30:18 -0400 Subject: [PATCH 3/3] remove unnecessary async --- testing/skia_gold_client/lib/skia_gold_client.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/testing/skia_gold_client/lib/skia_gold_client.dart b/testing/skia_gold_client/lib/skia_gold_client.dart index 4506c47d57bf8..b258de044b554 100644 --- a/testing/skia_gold_client/lib/skia_gold_client.dart +++ b/testing/skia_gold_client/lib/skia_gold_client.dart @@ -56,10 +56,10 @@ class SkiaGoldClient { String get _failuresPath => path.join(workDirectory.path, 'failures.json'); Future? _initResult; - Future _initOnce(Future Function() callback) async { + Future _initOnce(Future Function() callback) { // If a call has already been made, return the result of that call. _initResult ??= callback(); - return _initResult; + return _initResult!; } /// Indicates whether the client has already been authorized to communicate