diff --git a/packages/flutter_goldens_client/lib/skia_client.dart b/packages/flutter_goldens_client/lib/skia_client.dart index e89a3068a9c..8e1748270d0 100644 --- a/packages/flutter_goldens_client/lib/skia_client.dart +++ b/packages/flutter_goldens_client/lib/skia_client.dart @@ -314,6 +314,16 @@ class SkiaGoldClient { _tryjobInitialized = true; } + static void _addIndented(StringBuffer buffer, String text) { + if (text.isEmpty) { + buffer.writeln(' '); + } else { + for (final String line in text.split('\n')) { + buffer.writeln(' $line'); + } + } + } + /// Executes the `imgtest add` command in the goldctl tool for tryjobs. /// /// The `imgtest` command collects and uploads test results to the Skia Gold @@ -324,41 +334,64 @@ class SkiaGoldClient { /// The [testName] and [goldenFile] parameters reference the current /// comparison being evaluated by the [FlutterPreSubmitFileComparator]. Future tryjobAdd(String testName, File goldenFile) async { - final List imgtestCommand = [ - _goldctl, - 'imgtest', 'add', - '--work-dir', workDirectory - .childDirectory('temp') - .path, - '--test-name', cleanTestName(testName), - '--png-file', goldenFile.path, - ..._getPixelMatchingArguments(), - ]; + Duration delay = const Duration(seconds: 5); + while (true) { + final io.ProcessResult result = await process.run([ + _goldctl, + 'imgtest', 'add', + '--work-dir', workDirectory.childDirectory('temp').path, + '--test-name', cleanTestName(testName), + '--png-file', goldenFile.path, + ..._getPixelMatchingArguments(), + ]); - final io.ProcessResult result = await process.run(imgtestCommand); - - final String/*!*/ resultStdout = result.stdout.toString(); - if (result.exitCode != 0 && - !(resultStdout.contains('Untriaged') || resultStdout.contains('negative image'))) { - String? resultContents; - final File resultFile = workDirectory.childFile(fs.path.join( - 'result-state.json', - )); - if (await resultFile.exists()) { - resultContents = await resultFile.readAsString(); + final String resultStdout = result.stdout as String; + final String resultStderr = result.stderr as String; + if (result.exitCode == 0 || + resultStdout.contains('Untriaged') || + resultStdout.contains('negative image')) { + return; // success } - final StringBuffer buf = StringBuffer() - ..writeln('Unexpected Gold tryjobAdd failure.') - ..writeln('Tryjob execution for golden file test $testName failed for') - ..writeln('a reason unrelated to pixel comparison.') - ..writeln() - ..writeln('Debug information for Gold --------------------------------') - ..writeln('stdout: ${result.stdout}') - ..writeln('stderr: ${result.stderr}') - ..writeln() - ..writeln() - ..writeln('result-state.json: ${resultContents ?? 'No result file found.'}'); - throw SkiaException(buf.toString()); + + if (resultStdout.contains('502')) { + // probably a transient error, try again + // Ideally we'd use something like package:test's printOnError, but best reliability + // in getting logs on CI for now we're just using print. + // See also: https://github.com/flutter/flutter/issues/91285 + print('Transient failure from Skia Gold, retrying in ${delay.inSeconds} seconds.'); // ignore: avoid_print + print(''); // ignore: avoid_print + print('stdout from gold:'); // ignore: avoid_print + final StringBuffer buffer = StringBuffer(); + _addIndented(buffer, resultStdout); + print(buffer); // ignore: avoid_print + await Future.delayed(delay); + delay *= 2; + continue; // retry + } + + final StringBuffer buffer = StringBuffer() + ..write('Golden test for "$testName" failed with exit code ${result.exitCode} ') + ..writeln('for a reason unrelated to pixel comparison.'); + if (resultStdout.isNotEmpty) { + buffer + ..writeln() + ..writeln('stdout from gold:'); + _addIndented(buffer, resultStdout); + } + if (resultStderr.isNotEmpty) { + buffer + ..writeln() + ..writeln('stderr from gold:'); + _addIndented(buffer, resultStderr); + } + final File resultFile = workDirectory.childFile('result-state.json'); + if (await resultFile.exists()) { + buffer + ..writeln() + ..writeln('result-state.json contents:'); + _addIndented(buffer, resultFile.readAsStringSync()); + } + throw SkiaException(buffer.toString()); // failure } } diff --git a/packages/flutter_goldens_client/pubspec.yaml b/packages/flutter_goldens_client/pubspec.yaml index 8aa3f3c7f6b..366b2ede2e3 100644 --- a/packages/flutter_goldens_client/pubspec.yaml +++ b/packages/flutter_goldens_client/pubspec.yaml @@ -15,8 +15,60 @@ dependencies: path: 1.8.3 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade" typed_data: 1.3.2 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade" -dartdoc: - # Exclude this package from the hosted API docs. - nodoc: true +dev_dependencies: + flutter_test: + sdk: flutter + test: 1.24.9 -# PUBSPEC CHECKSUM: 1c2e \ No newline at end of file + _fe_analyzer_shared: 65.0.0 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade" + analyzer: 6.3.0 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade" + args: 2.4.2 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade" + async: 2.11.0 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade" + boolean_selector: 2.1.1 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade" + characters: 1.3.0 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade" + clock: 1.1.1 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade" + convert: 3.1.1 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade" + coverage: 1.7.1 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade" + fake_async: 1.3.1 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade" + frontend_server_client: 3.2.0 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade" + glob: 2.1.2 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade" + http_multi_server: 3.2.1 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade" + http_parser: 4.0.2 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade" + io: 1.0.4 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade" + js: 0.6.7 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade" + leak_tracker: 9.0.16 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade" + leak_tracker_testing: 1.0.5 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade" + logging: 1.2.0 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade" + matcher: 0.12.16 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade" + material_color_utilities: 0.8.0 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade" + mime: 1.0.4 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade" + node_preamble: 2.0.2 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade" + package_config: 2.1.0 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade" + pool: 1.5.1 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade" + pub_semver: 2.1.4 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade" + shelf: 1.4.1 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade" + shelf_packages_handler: 3.0.2 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade" + shelf_static: 1.1.2 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade" + shelf_web_socket: 1.0.4 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade" + source_map_stack_trace: 2.1.1 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade" + source_maps: 0.10.12 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade" + source_span: 1.10.0 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade" + stack_trace: 1.11.1 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade" + stream_channel: 2.1.2 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade" + string_scanner: 1.2.0 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade" + term_glyph: 1.2.1 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade" + test_api: 0.6.1 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade" + test_core: 0.5.9 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade" + vector_math: 2.1.4 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade" + vm_service: 13.0.0 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade" + watcher: 1.1.0 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade" + web: 0.4.0 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade" + web_socket_channel: 2.4.0 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade" + webkit_inspection_protocol: 1.2.1 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade" + yaml: 3.1.2 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade" + +dartdoc: + # Exclude this package from the hosted API docs. + nodoc: true + +# PUBSPEC CHECKSUM: f695 diff --git a/packages/flutter_goldens_client/test/skia_client_test.dart b/packages/flutter_goldens_client/test/skia_client_test.dart new file mode 100644 index 00000000000..3e993b1cd6e --- /dev/null +++ b/packages/flutter_goldens_client/test/skia_client_test.dart @@ -0,0 +1,85 @@ +// Copyright 2014 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:async'; +import 'dart:convert'; +import 'dart:io' show HttpClient, ProcessResult; + +import 'package:file/file.dart'; +import 'package:file/memory.dart'; +import 'package:flutter_goldens_client/skia_client.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:platform/platform.dart'; +import 'package:process/process.dart'; + +void main() { + test('502 retry', () async { + final List log = []; + await runZoned( + zoneSpecification: ZoneSpecification( + print: (Zone self, ZoneDelegate parent, Zone zone, String line) { + log.add(line); + }, + createTimer: (Zone self, ZoneDelegate parent, Zone zone, Duration duration, void Function() f) { + log.add('created timer for $duration'); + return parent.createTimer(zone, Duration.zero, f); + }, + ), + () async { + final FileSystem fs; + final SkiaGoldClient skiaClient = SkiaGoldClient( + fs: fs = MemoryFileSystem(), + process: FakeProcessManager(log), + platform: FakePlatform( + environment: const { + 'GOLDCTL': 'goldctl', + }, + ), + httpClient: FakeHttpClient(), + fs.directory('/'), + ); + print('start'); // ignore: avoid_print + await skiaClient.tryjobAdd('test', fs.file('golden')); + print('end'); // ignore: avoid_print + expect(log, [ + 'start', + 'goldctl imgtest add --work-dir /temp --test-name t --png-file golden', + 'Transient failure from Skia Gold, retrying in 5 seconds.', + '', + 'stdout from gold:', + ' 502\n', + 'created timer for 0:00:05.000000', + 'goldctl imgtest add --work-dir /temp --test-name t --png-file golden', + 'end' + ]); + }, + ); + }); +} + +class FakeProcessManager extends Fake implements ProcessManager { + FakeProcessManager(this.log); + + final List log; + int _index = 0; + + @override + Future run(List command, { + Map? environment, + bool includeParentEnvironment = true, + bool runInShell = false, + Encoding? stderrEncoding, + Encoding? stdoutEncoding, + String? workingDirectory, + }) async { + log.add(command.join(' ')); + _index += 1; + switch (_index) { + case 1: return ProcessResult(0, 1, '502', ''); + case 2: return ProcessResult(0, 0, '200', ''); + default: throw StateError('unexpected call to run'); + } + } +} +class FakeHttpClient extends Fake implements HttpClient { }