Skip to content

Commit 7b300f0

Browse files
Reverts "Retry on transient Skia failure." (#139407)
Reverts flutter/flutter#139182 Initiated by: Hixie This change reverts the following previous change: Original Description: This works around https://g-issues.skia.org/issues/40044713 using exponential backoff. This is completely untested because I have no idea how to test it. Also I only changed one of the code paths here. I figure if we get success here then we can start propagating the change to other places in this file that generate errors, maybe factoring out the retry and error reporting logic so it's not duplicated multiple times.
1 parent e047f27 commit 7b300f0

File tree

3 files changed

+34
-204
lines changed

3 files changed

+34
-204
lines changed

packages/flutter_goldens_client/lib/skia_client.dart

Lines changed: 31 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -314,16 +314,6 @@ class SkiaGoldClient {
314314
_tryjobInitialized = true;
315315
}
316316

317-
static void _addIndented(StringBuffer buffer, String text) {
318-
if (text.isEmpty) {
319-
buffer.writeln(' <empty>');
320-
} else {
321-
for (final String line in text.split('\n')) {
322-
buffer.writeln(' $line');
323-
}
324-
}
325-
}
326-
327317
/// Executes the `imgtest add` command in the goldctl tool for tryjobs.
328318
///
329319
/// The `imgtest` command collects and uploads test results to the Skia Gold
@@ -334,64 +324,41 @@ class SkiaGoldClient {
334324
/// The [testName] and [goldenFile] parameters reference the current
335325
/// comparison being evaluated by the [FlutterPreSubmitFileComparator].
336326
Future<void> tryjobAdd(String testName, File goldenFile) async {
337-
Duration delay = const Duration(seconds: 5);
338-
while (true) {
339-
final io.ProcessResult result = await process.run(<String>[
340-
_goldctl,
341-
'imgtest', 'add',
342-
'--work-dir', workDirectory.childDirectory('temp').path,
343-
'--test-name', cleanTestName(testName),
344-
'--png-file', goldenFile.path,
345-
..._getPixelMatchingArguments(),
346-
]);
347-
348-
final String resultStdout = result.stdout as String;
349-
final String resultStderr = result.stderr as String;
350-
if (result.exitCode == 0 ||
351-
resultStdout.contains('Untriaged') ||
352-
resultStdout.contains('negative image')) {
353-
return; // success
354-
}
327+
final List<String> imgtestCommand = <String>[
328+
_goldctl,
329+
'imgtest', 'add',
330+
'--work-dir', workDirectory
331+
.childDirectory('temp')
332+
.path,
333+
'--test-name', cleanTestName(testName),
334+
'--png-file', goldenFile.path,
335+
..._getPixelMatchingArguments(),
336+
];
355337

356-
if (resultStdout.contains('502')) {
357-
// probably a transient error, try again
358-
// Ideally we'd use something like package:test's printOnError, but best reliability
359-
// in getting logs on CI for now we're just using print.
360-
// See also: https://github.com/flutter/flutter/issues/91285
361-
print('Transient failure from Skia Gold, retrying in ${delay.inSeconds} seconds.'); // ignore: avoid_print
362-
print(''); // ignore: avoid_print
363-
print('stdout from gold:'); // ignore: avoid_print
364-
final StringBuffer buffer = StringBuffer();
365-
_addIndented(buffer, resultStdout);
366-
print(buffer); // ignore: avoid_print
367-
await Future<void>.delayed(delay);
368-
delay *= 2;
369-
continue; // retry
370-
}
338+
final io.ProcessResult result = await process.run(imgtestCommand);
371339

372-
final StringBuffer buffer = StringBuffer()
373-
..write('Golden test for "$testName" failed with exit code ${result.exitCode} ')
374-
..writeln('for a reason unrelated to pixel comparison.');
375-
if (resultStdout.isNotEmpty) {
376-
buffer
377-
..writeln()
378-
..writeln('stdout from gold:');
379-
_addIndented(buffer, resultStdout);
380-
}
381-
if (resultStderr.isNotEmpty) {
382-
buffer
383-
..writeln()
384-
..writeln('stderr from gold:');
385-
_addIndented(buffer, resultStderr);
386-
}
387-
final File resultFile = workDirectory.childFile('result-state.json');
340+
final String/*!*/ resultStdout = result.stdout.toString();
341+
if (result.exitCode != 0 &&
342+
!(resultStdout.contains('Untriaged') || resultStdout.contains('negative image'))) {
343+
String? resultContents;
344+
final File resultFile = workDirectory.childFile(fs.path.join(
345+
'result-state.json',
346+
));
388347
if (await resultFile.exists()) {
389-
buffer
390-
..writeln()
391-
..writeln('result-state.json contents:');
392-
_addIndented(buffer, resultFile.readAsStringSync());
348+
resultContents = await resultFile.readAsString();
393349
}
394-
throw SkiaException(buffer.toString()); // failure
350+
final StringBuffer buf = StringBuffer()
351+
..writeln('Unexpected Gold tryjobAdd failure.')
352+
..writeln('Tryjob execution for golden file test $testName failed for')
353+
..writeln('a reason unrelated to pixel comparison.')
354+
..writeln()
355+
..writeln('Debug information for Gold --------------------------------')
356+
..writeln('stdout: ${result.stdout}')
357+
..writeln('stderr: ${result.stderr}')
358+
..writeln()
359+
..writeln()
360+
..writeln('result-state.json: ${resultContents ?? 'No result file found.'}');
361+
throw SkiaException(buf.toString());
395362
}
396363
}
397364

packages/flutter_goldens_client/pubspec.yaml

Lines changed: 3 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -15,60 +15,8 @@ dependencies:
1515
path: 1.8.3 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
1616
typed_data: 1.3.2 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
1717

18-
dev_dependencies:
19-
flutter_test:
20-
sdk: flutter
21-
test: 1.24.9
22-
23-
_fe_analyzer_shared: 65.0.0 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
24-
analyzer: 6.3.0 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
25-
args: 2.4.2 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
26-
async: 2.11.0 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
27-
boolean_selector: 2.1.1 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
28-
characters: 1.3.0 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
29-
clock: 1.1.1 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
30-
convert: 3.1.1 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
31-
coverage: 1.7.1 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
32-
fake_async: 1.3.1 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
33-
frontend_server_client: 3.2.0 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
34-
glob: 2.1.2 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
35-
http_multi_server: 3.2.1 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
36-
http_parser: 4.0.2 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
37-
io: 1.0.4 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
38-
js: 0.6.7 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
39-
leak_tracker: 9.0.16 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
40-
leak_tracker_testing: 1.0.5 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
41-
logging: 1.2.0 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
42-
matcher: 0.12.16 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
43-
material_color_utilities: 0.8.0 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
44-
mime: 1.0.4 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
45-
node_preamble: 2.0.2 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
46-
package_config: 2.1.0 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
47-
pool: 1.5.1 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
48-
pub_semver: 2.1.4 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
49-
shelf: 1.4.1 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
50-
shelf_packages_handler: 3.0.2 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
51-
shelf_static: 1.1.2 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
52-
shelf_web_socket: 1.0.4 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
53-
source_map_stack_trace: 2.1.1 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
54-
source_maps: 0.10.12 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
55-
source_span: 1.10.0 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
56-
stack_trace: 1.11.1 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
57-
stream_channel: 2.1.2 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
58-
string_scanner: 1.2.0 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
59-
term_glyph: 1.2.1 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
60-
test_api: 0.6.1 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
61-
test_core: 0.5.9 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
62-
vector_math: 2.1.4 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
63-
vm_service: 13.0.0 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
64-
watcher: 1.1.0 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
65-
web: 0.4.0 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
66-
web_socket_channel: 2.4.0 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
67-
webkit_inspection_protocol: 1.2.1 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
68-
yaml: 3.1.2 # THIS LINE IS AUTOGENERATED - TO UPDATE USE "flutter update-packages --force-upgrade"
69-
7018
dartdoc:
71-
# Exclude this package from the hosted API docs.
72-
nodoc: true
19+
# Exclude this package from the hosted API docs.
20+
nodoc: true
7321

74-
# PUBSPEC CHECKSUM: f695
22+
# PUBSPEC CHECKSUM: 1c2e

packages/flutter_goldens_client/test/skia_client_test.dart

Lines changed: 0 additions & 85 deletions
This file was deleted.

0 commit comments

Comments
 (0)