Skip to content

Commit

Permalink
Reverts "Retry on transient Skia failure." (#139407)
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@auto-submit
auto-submit[bot] authored Dec 2, 2023
1 parent e047f27 commit 7b300f0
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 204 deletions.
95 changes: 31 additions & 64 deletions packages/flutter_goldens_client/lib/skia_client.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -314,16 +314,6 @@ class SkiaGoldClient {
_tryjobInitialized = true;
}

static void _addIndented(StringBuffer buffer, String text) {
if (text.isEmpty) {
buffer.writeln(' <empty>');
} 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
Expand All @@ -334,64 +324,41 @@ class SkiaGoldClient {
/// The [testName] and [goldenFile] parameters reference the current
/// comparison being evaluated by the [FlutterPreSubmitFileComparator].
Future<void> tryjobAdd(String testName, File goldenFile) async {
Duration delay = const Duration(seconds: 5);
while (true) {
final io.ProcessResult result = await process.run(<String>[
_goldctl,
'imgtest', 'add',
'--work-dir', workDirectory.childDirectory('temp').path,
'--test-name', cleanTestName(testName),
'--png-file', goldenFile.path,
..._getPixelMatchingArguments(),
]);

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 List<String> imgtestCommand = <String>[
_goldctl,
'imgtest', 'add',
'--work-dir', workDirectory
.childDirectory('temp')
.path,
'--test-name', cleanTestName(testName),
'--png-file', goldenFile.path,
..._getPixelMatchingArguments(),
];

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<void>.delayed(delay);
delay *= 2;
continue; // retry
}
final io.ProcessResult result = await process.run(imgtestCommand);

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');
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()) {
buffer
..writeln()
..writeln('result-state.json contents:');
_addIndented(buffer, resultFile.readAsStringSync());
resultContents = await resultFile.readAsString();
}
throw SkiaException(buffer.toString()); // failure
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());
}
}

Expand Down
58 changes: 3 additions & 55 deletions packages/flutter_goldens_client/pubspec.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,60 +15,8 @@ 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"

dev_dependencies:
flutter_test:
sdk: flutter
test: 1.24.9

_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
# Exclude this package from the hosted API docs.
nodoc: true

# PUBSPEC CHECKSUM: f695
# PUBSPEC CHECKSUM: 1c2e
85 changes: 0 additions & 85 deletions packages/flutter_goldens_client/test/skia_client_test.dart
View file

This file was deleted.

0 comments on commit 7b300f0

Please sign in to comment.