Skip to content

Commit

Permalink
Add more logs to diagnose Gold flake (#108930)
Browse files Browse the repository at this point in the history
  • Loading branch information
@Piinks @pull
Piinks authored and pull[bot] committed Jan 30, 2024
1 parent 9a296df commit 6a24f2d
Show file tree
Hide file tree
Showing 2 changed files with 138 additions and 16 deletions.
88 changes: 88 additions & 0 deletions packages/flutter_goldens/test/flutter_goldens_test.dart
View file
Expand Up @@ -489,6 +489,94 @@ void main() {
);
});

test('throws for error state from imgtestAdd', () {
final File goldenFile = fs.file('/workDirectory/temp/golden_file_test.png')
..createSync(recursive: true);
platform = FakePlatform(
environment: <String, String>{
'FLUTTER_ROOT': _kFlutterRoot,
'GOLDCTL' : 'goldctl',
},
operatingSystem: 'macos'
);

skiaClient = SkiaGoldClient(
workDirectory,
fs: fs,
process: process,
platform: platform,
httpClient: fakeHttpClient,
);

const RunInvocation goldctlInvocation = RunInvocation(
<String>[
'goldctl',
'imgtest', 'add',
'--work-dir', '/workDirectory/temp',
'--test-name', 'golden_file_test',
'--png-file', '/workDirectory/temp/golden_file_test.png',
'--passfail',
],
null,
);
process.processResults[goldctlInvocation] = ProcessResult(123, 1, 'Expected failure', 'Expected failure');
process.fallbackProcessResult = ProcessResult(123, 1, 'Fallback failure', 'Fallback failure');

expect(
skiaClient.imgtestAdd('golden_file_test', goldenFile),
throwsA(
isA<SkiaException>().having((SkiaException error) => error.message,
'message',
contains('result-state.json'),
),
),
);
});

test('throws for error state from tryjobAdd', () {
final File goldenFile = fs.file('/workDirectory/temp/golden_file_test.png')
..createSync(recursive: true);
platform = FakePlatform(
environment: <String, String>{
'FLUTTER_ROOT': _kFlutterRoot,
'GOLDCTL' : 'goldctl',
},
operatingSystem: 'macos'
);

skiaClient = SkiaGoldClient(
workDirectory,
fs: fs,
process: process,
platform: platform,
httpClient: fakeHttpClient,
);

const RunInvocation goldctlInvocation = RunInvocation(
<String>[
'goldctl',
'imgtest', 'add',
'--work-dir', '/workDirectory/temp',
'--test-name', 'golden_file_test',
'--png-file', '/workDirectory/temp/golden_file_test.png',
'--passfail',
],
null,
);
process.processResults[goldctlInvocation] = ProcessResult(123, 1, 'Expected failure', 'Expected failure');
process.fallbackProcessResult = ProcessResult(123, 1, 'Fallback failure', 'Fallback failure');

expect(
skiaClient.tryjobAdd('golden_file_test', goldenFile),
throwsA(
isA<SkiaException>().having((SkiaException error) => error.message,
'message',
contains('result-state.json'),
),
),
);
});

group('Request Handling', () {
const String expectation = '55109a4bed52acc780530f7a9aeff6c0';

Expand Down
66 changes: 50 additions & 16 deletions packages/flutter_goldens_client/lib/skia_client.dart
View file
Expand Up @@ -21,6 +21,21 @@ const String _kGoldctlKey = 'GOLDCTL';
const String _kTestBrowserKey = 'FLUTTER_TEST_BROWSER';
const String _kWebRendererKey = 'FLUTTER_WEB_RENDERER';

/// Exception thrown when an error is returned from the [SkiaClient].
class SkiaException implements Exception {
/// Creates a new `SkiaException` with a required error [message].
const SkiaException(this.message);

/// A message describing the error.
final String message;

/// Returns a description of the Skia exception.
///
/// The description always contains the [message].
@override
String toString() => 'SkiaException: $message';
}

/// A client for uploading image tests and making baseline requests to the
/// Flutter Gold Dashboard.
class SkiaGoldClient {
Expand Down Expand Up @@ -103,10 +118,10 @@ class SkiaGoldClient {
..writeln('Luci environments authenticate using the file provided '
'by LUCI_CONTEXT. There may be an error with this file or Gold '
'authentication.')
..writeln('Debug information for Gold:')
..writeln('Debug information for Gold --------------------------------')
..writeln('stdout: ${result.stdout}')
..writeln('stderr: ${result.stderr}');
throw Exception(buf.toString());
throw SkiaException(buf.toString());
}
}

Expand Down Expand Up @@ -155,7 +170,7 @@ class SkiaGoldClient {
..writeln('Please confirm the settings of your golden file test.')
..writeln('Arguments provided:');
imgtestInitCommand.forEach(buf.writeln);
throw Exception(buf.toString());
throw SkiaException(buf.toString());
}

final io.ProcessResult result = await process.run(imgtestInitCommand);
Expand All @@ -167,10 +182,10 @@ class SkiaGoldClient {
..writeln('An error occurred when initializing golden file test with ')
..writeln('goldctl.')
..writeln()
..writeln('Debug information for Gold:')
..writeln('Debug information for Gold --------------------------------')
..writeln('stdout: ${result.stdout}')
..writeln('stderr: ${result.stderr}');
throw Exception(buf.toString());
throw SkiaException(buf.toString());
}
_initialized = true;
}
Expand Down Expand Up @@ -202,6 +217,14 @@ class SkiaGoldClient {
if (result.exitCode != 0) {
// If an unapproved image has made it to post-submit, throw to close the
// tree.
String? resultContents;
final File resultFile = workDirectory.childFile(fs.path.join(
'result-state.json',
));
if(await resultFile.exists()) {
resultContents = await resultFile.readAsString();
}

final StringBuffer buf = StringBuffer()
..writeln('Skia Gold received an unapproved image in post-submit ')
..writeln('testing. Golden file images in flutter/flutter are triaged ')
Expand All @@ -212,10 +235,12 @@ class SkiaGoldClient {
..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('Debug information for Gold --------------------------------')
..writeln('stdout: ${result.stdout}')
..writeln('stderr: ${result.stderr}');
throw Exception(buf.toString());
..writeln('stderr: ${result.stderr}')
..writeln()
..writeln('result-state.json: ${resultContents ?? 'No result file found.'}');
throw SkiaException(buf.toString());
}

return true;
Expand Down Expand Up @@ -269,7 +294,7 @@ class SkiaGoldClient {
..writeln('Please confirm the settings of your golden file test.')
..writeln('Arguments provided:');
imgtestInitCommand.forEach(buf.writeln);
throw Exception(buf.toString());
throw SkiaException(buf.toString());
}

final io.ProcessResult result = await process.run(imgtestInitCommand);
Expand All @@ -281,10 +306,10 @@ class SkiaGoldClient {
..writeln('An error occurred when initializing golden file tryjob with ')
..writeln('goldctl.')
..writeln()
..writeln('Debug information for Gold:')
..writeln('Debug information for Gold --------------------------------')
..writeln('stdout: ${result.stdout}')
..writeln('stderr: ${result.stderr}');
throw Exception(buf.toString());
throw SkiaException(buf.toString());
}
_tryjobInitialized = true;
}
Expand Down Expand Up @@ -315,16 +340,25 @@ class SkiaGoldClient {
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 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('Debug information for Gold --------------------------------')
..writeln('stdout: ${result.stdout}')
..writeln('stderr: ${result.stderr}')
..writeln();
throw Exception(buf.toString());
..writeln()
..writeln()
..writeln('result-state.json: ${resultContents ?? 'No result file found.'}');
throw SkiaException(buf.toString());
}
}

Expand Down Expand Up @@ -432,14 +466,14 @@ class SkiaGoldClient {
/// Returns the current commit hash of the Flutter repository.
Future<String> _getCurrentCommit() async {
if (!_flutterRoot.existsSync()) {
throw Exception('Flutter root could not be found: $_flutterRoot\n');
throw SkiaException('Flutter root could not be found: $_flutterRoot\n');
} else {
final io.ProcessResult revParse = await process.run(
<String>['git', 'rev-parse', 'HEAD'],
workingDirectory: _flutterRoot.path,
);
if (revParse.exitCode != 0) {
throw Exception('Current commit of Flutter can not be found.');
throw const SkiaException('Current commit of Flutter can not be found.');
}
return (revParse.stdout as String/*!*/).trim();
}
Expand Down

0 comments on commit 6a24f2d

Please sign in to comment.