Unverified Commit 9011cece authored by Kate Lovett's avatar Kate Lovett Committed by GitHub

Pre-Submit Tryjobs for Flutter Gold (#44474)

parent 9e94eb93
......@@ -41,16 +41,17 @@ const Tolerance _kFlingTolerance = Tolerance(
distance: 0.01,
);
/// Configures how an [AnimationController] behaves when animations are disabled.
/// Configures how an [AnimationController] behaves when animations are
/// disabled.
///
/// When [AccessibilityFeatures.disableAnimations] is true, the device is asking
/// Flutter to reduce or disable animations as much as possible. To honor this,
/// we reduce the duration and the corresponding number of frames for animations.
/// This enum is used to allow certain [AnimationController]s to opt out of this
/// behavior.
/// we reduce the duration and the corresponding number of frames for
/// animations. This enum is used to allow certain [AnimationController]s to opt
/// out of this behavior.
///
/// For example, the [AnimationController] which controls the physics simulation
/// for a scrollable list will have [AnimationBehavior.preserve] so that when
/// for a scrollable list will have [AnimationBehavior.preserve], so that when
/// a user attempts to scroll it does not jump to the end/beginning too quickly.
enum AnimationBehavior {
/// The [AnimationController] will reduce its duration when
......
......@@ -129,6 +129,11 @@ abstract class FlutterGoldenFileComparator extends GoldenFileComparator {
Uri getTestUri(Uri key, int version) => key;
/// Calculate the appropriate basedir for the current test context.
///
/// The optional [suffix] argument is used by the
/// [FlutterSkiaGoldFileComparator] and the [FlutterPreSubmitFileComparator].
/// These [FlutterGoldenFileComparators] randomize their base directories to
/// maintain thread safety while using the `goldctl` tool.
@protected
@visibleForTesting
static Directory getBaseDirectory(LocalFileComparator defaultComparator, Platform platform, {String suffix = ''}) {
......@@ -217,10 +222,7 @@ class FlutterSkiaGoldFileComparator extends FlutterGoldenFileComparator {
platform,
suffix: '${math.Random().nextInt(10000)}',
);
if(!baseDirectory.existsSync()) {
baseDirectory.createSync(recursive: true);
}
goldens ??= SkiaGoldClient(baseDirectory);
await goldens.auth();
......@@ -299,47 +301,23 @@ class FlutterPreSubmitFileComparator extends FlutterGoldenFileComparator {
final Directory baseDirectory = FlutterGoldenFileComparator.getBaseDirectory(
defaultComparator,
platform,
suffix: '${math.Random().nextInt(10000)}',
);
if(!baseDirectory.existsSync()) {
baseDirectory.createSync(recursive: true);
}
goldens ??= SkiaGoldClient(baseDirectory);
await goldens.getExpectations();
await goldens.auth();
await goldens.tryjobInit();
return FlutterPreSubmitFileComparator(baseDirectory.uri, goldens);
}
@override
Future<bool> compare(Uint8List imageBytes, Uri golden) async {
golden = _addPrefix(golden);
final String testName = skiaClient.cleanTestName(golden.path);
final List<String> testExpectations = skiaClient.expectations[testName];
if (testExpectations == null) {
// There is no baseline for this test
return true;
}
ComparisonResult result;
for (String expectation in testExpectations) {
final List<int> goldenBytes = await skiaClient.getImageBytes(expectation);
result = GoldenFileComparator.compareLists(
imageBytes,
goldenBytes,
);
if (result.passed) {
return true;
}
}
await update(golden, imageBytes);
final File goldenFile = getGoldenFile(golden);
return skiaClient.testIsIgnoredForPullRequest(
platform.environment['CIRRUS_PR'] ?? '',
golden.path,
);
return skiaClient.tryjobAdd(golden.path, goldenFile);
}
/// Decides based on the current environment whether goldens tests should be
......
......@@ -84,14 +84,12 @@ void main() {
group('Request Handling', () {
String testName;
String pullRequestNumber;
String expectation;
Uri url;
MockHttpClientRequest mockHttpRequest;
setUp(() {
testName = 'flutter.golden_test.1.png';
pullRequestNumber = '1234';
expectation = '55109a4bed52acc780530f7a9aeff6c0';
mockHttpRequest = MockHttpClientRequest();
});
......@@ -212,120 +210,6 @@ void main() {
expect(masterBytes, equals(_kTestPngBytes));
});
group('ignores', () {
Uri url;
MockHttpClientRequest mockHttpRequest;
MockHttpClientResponse mockHttpResponse;
setUp(() {
url = Uri.parse('https://flutter-gold.skia.org/json/ignores');
mockHttpRequest = MockHttpClientRequest();
mockHttpResponse = MockHttpClientResponse(utf8.encode(
ignoreResponseTemplate(
pullRequestNumber: pullRequestNumber,
expires: DateTime.now()
.add(const Duration(days: 1))
.toString(),
otherTestName: 'unrelatedTest.1'
)
));
when(mockHttpClient.getUrl(url))
.thenAnswer((_) => Future<MockHttpClientRequest>.value(mockHttpRequest));
when(mockHttpRequest.close())
.thenAnswer((_) => Future<MockHttpClientResponse>.value(mockHttpResponse));
});
test('returns true for ignored test and ignored pull request number', () async {
expect(
await skiaClient.testIsIgnoredForPullRequest(
pullRequestNumber,
testName,
),
isTrue,
);
});
test('returns true for ignored test and not ignored pull request number', () async {
expect(
await skiaClient.testIsIgnoredForPullRequest(
'5678',
testName,
),
isTrue,
);
});
test('returns false for not ignored test and ignored pull request number', () async {
expect(
await skiaClient.testIsIgnoredForPullRequest(
pullRequestNumber,
'failure.png',
),
isFalse,
);
});
test('throws exception for expired ignore', () async {
mockHttpResponse = MockHttpClientResponse(utf8.encode(
ignoreResponseTemplate(
pullRequestNumber: pullRequestNumber,
)
));
when(mockHttpRequest.close())
.thenAnswer((_) => Future<MockHttpClientResponse>.value(mockHttpResponse));
final Future<bool> test = skiaClient.testIsIgnoredForPullRequest(
pullRequestNumber,
testName,
);
expect(
test,
throwsException,
);
});
test('throws exception for first expired ignore among multiple', () async {
mockHttpResponse = MockHttpClientResponse(utf8.encode(
ignoreResponseTemplate(
pullRequestNumber: pullRequestNumber,
otherExpires: DateTime.now()
.add(const Duration(days: 1))
.toString(),
)
));
when(mockHttpRequest.close())
.thenAnswer((_) => Future<MockHttpClientResponse>.value(mockHttpResponse));
final Future<bool> test = skiaClient.testIsIgnoredForPullRequest(
pullRequestNumber,
testName,
);
expect(
test,
throwsException,
);
});
test('throws exception for later expired ignore among multiple', () async {
mockHttpResponse = MockHttpClientResponse(utf8.encode(
ignoreResponseTemplate(
pullRequestNumber: pullRequestNumber,
expires: DateTime.now()
.add(const Duration(days: 1))
.toString(),
)
));
when(mockHttpRequest.close())
.thenAnswer((_) => Future<MockHttpClientResponse>.value(mockHttpResponse));
final Future<bool> test = skiaClient.testIsIgnoredForPullRequest(
pullRequestNumber,
testName,
);
expect(
test,
throwsException,
);
});
});
group('digest parsing', () {
Uri url;
MockHttpClientRequest mockHttpRequest;
......@@ -507,39 +391,6 @@ void main() {
});
group('Pre-Submit', () {
FlutterPreSubmitFileComparator comparator;
final MockSkiaGoldClient mockSkiaClient = MockSkiaGoldClient();
setUp(() {
final Directory basedir = fs.directory('flutter/test/library/')
..createSync(recursive: true);
comparator = FlutterPreSubmitFileComparator(
basedir.uri,
mockSkiaClient,
fs: fs,
platform: FakePlatform(
environment: <String, String>{
'FLUTTER_ROOT': _kFlutterRoot,
'CIRRUS_CI' : 'true',
'CIRRUS_PR' : '1234',
'GOLD_SERVICE_ACCOUNT' : 'service account...'
},
operatingSystem: 'macos'
),
);
when(mockSkiaClient.getImageBytes('55109a4bed52acc780530f7a9aeff6c0'))
.thenAnswer((_) => Future<List<int>>.value(_kTestPngBytes));
when(mockSkiaClient.expectations)
.thenReturn(expectationsTemplate());
when(mockSkiaClient.cleanTestName('library.flutter.golden_test.1.png'))
.thenReturn('flutter.golden_test.1');
when(mockSkiaClient.isValidDigestForExpectation(
'55109a4bed52acc780530f7a9aeff6c0',
'library.flutter.golden_test.1.png',
))
.thenAnswer((_) => Future<bool>.value(false));
});
group('correctly determines testing environment', () {
test('returns true', () {
platform = FakePlatform(
......@@ -601,50 +452,6 @@ void main() {
);
});
});
test('comparison passes test that is ignored for this PR', () async {
when(mockSkiaClient.getImageBytes('55109a4bed52acc780530f7a9aeff6c0'))
.thenAnswer((_) => Future<List<int>>.value(_kTestPngBytes));
when(mockSkiaClient.testIsIgnoredForPullRequest(
'1234',
'library.flutter.golden_test.1.png',
))
.thenAnswer((_) => Future<bool>.value(true));
expect(
await comparator.compare(
Uint8List.fromList(_kFailPngBytes),
Uri.parse('flutter.golden_test.1.png'),
),
isTrue,
);
});
test('fails test that is not ignored', () async {
when(mockSkiaClient.getImageBytes('55109a4bed52acc780530f7a9aeff6c0'))
.thenAnswer((_) => Future<List<int>>.value(_kTestPngBytes));
when(mockSkiaClient.testIsIgnoredForPullRequest(
'1234',
'library.flutter.golden_test.1.png',
))
.thenAnswer((_) => Future<bool>.value(false));
expect(
await comparator.compare(
Uint8List.fromList(_kFailPngBytes),
Uri.parse('flutter.golden_test.1.png'),
),
isFalse,
);
});
test('passes non-existent baseline for new test', () async {
expect(
await comparator.compare(
Uint8List.fromList(_kFailPngBytes),
Uri.parse('flutter.new_golden_test.1.png'),
),
isTrue,
);
});
});
group('Skipping', () {
......
......@@ -169,37 +169,6 @@ String digestResponseTemplate({
''';
}
/// Json response template for Skia Gold ignore request:
/// https://flutter-gold.skia.org/json/ignores
String ignoreResponseTemplate({
String pullRequestNumber = '0000',
String testName = 'flutter.golden_test.1',
String otherTestName = 'flutter.golden_test.1',
String expires = '2019-09-06T21:28:18.815336Z',
String otherExpires = '2019-09-06T21:28:18.815336Z',
}) {
return '''
[
{
"id": "7579425228619212078",
"name": "contributor@getMail.com",
"updatedBy": "contributor@getMail.com",
"expires": "$expires",
"query": "ext=png&name=$testName",
"note": "https://github.com/flutter/flutter/pull/$pullRequestNumber"
},
{
"id": "7579425228619212078",
"name": "contributor@getMail.com",
"updatedBy": "contributor@getMail.com",
"expires": "$otherExpires",
"query": "ext=png&name=$otherTestName",
"note": "https://github.com/flutter/flutter/pull/99999"
}
]
''';
}
/// Json response template for Skia Gold image request:
/// https://flutter-gold.skia.org/img/images/[imageHash].png
List<List<int>> imageResponseTemplate() {
......
......@@ -209,6 +209,96 @@ class SkiaGoldClient {
return true;
}
/// Executes the `imgtest init` command in the goldctl tool for tryjobs.
///
/// The `imgtest` command collects and uploads test results to the Skia Gold
/// backend, the `init` argument initializes the current tryjob.
Future<void> tryjobInit() async {
final File keys = workDirectory.childFile('keys.json');
final File failures = workDirectory.childFile('failures.json');
await keys.writeAsString(_getKeysJSON());
await failures.create();
final String commitHash = await _getCurrentCommit();
final String pullRequest = platform.environment['CIRRUS_PR'];
final String cirrusTaskID = platform.environment['CIRRUS_TASK_ID'];
final List<String> imgtestInitArguments = <String>[
'imgtest', 'init',
'--instance', 'flutter',
'--work-dir', workDirectory
.childDirectory('temp')
.path,
'--commit', commitHash,
'--keys-file', keys.path,
'--failure-file', failures.path,
'--passfail',
'--crs', 'github',
'--changelist', pullRequest,
'--cis', 'cirrus',
'--jobid', cirrusTaskID,
'--patchset_id', commitHash,
];
if (imgtestInitArguments.contains(null)) {
final StringBuffer buf = StringBuffer()
..writeln('Null argument for Skia Gold tryjobInit:');
imgtestInitArguments.forEach(buf.writeln);
throw NonZeroExitCode(1, buf.toString());
}
final io.ProcessResult result = await io.Process.run(
_goldctl,
imgtestInitArguments,
);
if (result.exitCode != 0) {
final StringBuffer buf = StringBuffer()
..writeln('Skia Gold tryjobInit failure.')
..writeln('stdout: ${result.stdout}')
..writeln('stderr: ${result.stderr}');
throw NonZeroExitCode(1, buf.toString());
}
}
/// Executes the `imgtest add` command in the goldctl tool for tryjobs.
///
/// The `imgtest` command collects and uploads test results to the Skia Gold
/// backend, the `add` argument uploads the current image test. A response is
/// returned from the invocation of this command that indicates a pass or fail
/// result for the tryjob.
///
/// The testName and goldenFile parameters reference the current comparison
/// being evaluated by the [FlutterSkiaGoldFileComparator].
Future<bool> tryjobAdd(String testName, File goldenFile) async {
assert(testName != null);
assert(goldenFile != null);
final List<String> imgtestArguments = <String>[
'imgtest', 'add',
'--work-dir', workDirectory
.childDirectory('temp')
.path,
'--test-name', cleanTestName(testName),
'--png-file', goldenFile.path,
];
final io.ProcessResult result = await io.Process.run(
_goldctl,
imgtestArguments,
);
if (result.exitCode != 0) {
final StringBuffer buf = StringBuffer()
..writeln('Skia Gold tryjobAdd failure.')
..writeln('stdout: ${result.stdout}')
..writeln('stderr: ${result.stderr}\n');
throw NonZeroExitCode(1, buf.toString());
}
return result.exitCode == 0;
}
/// Requests and sets the [_expectations] known to Flutter Gold at head.
Future<void> getExpectations() async {
_expectations = <String, List<String>>{};
......@@ -262,69 +352,6 @@ class SkiaGoldClient {
return imageBytes;
}
/// Returns a boolean value for whether or not the given test and current pull
/// request are ignored on Flutter Gold.
///
/// This is only relevant when used by the [FlutterPreSubmitFileComparator]
/// when a golden file test fails. In order to land a change to an existing
/// golden file, an ignore must be set up in Flutter Gold. This will serve as
/// a flag to permit the change to land, protect against any unwanted changes,
/// and ensure that changes that have landed are triaged.
Future<bool> testIsIgnoredForPullRequest(String pullRequest, String testName) async {
bool ignoreIsActive = false;
testName = cleanTestName(testName);
String rawResponse;
await io.HttpOverrides.runWithHttpOverrides<Future<void>>(() async {
final Uri requestForIgnores = Uri.parse(
'https://flutter-gold.skia.org/json/ignores'
);
try {
final io.HttpClientRequest request = await httpClient.getUrl(requestForIgnores);
final io.HttpClientResponse response = await request.close();
rawResponse = await utf8.decodeStream(response);
final List<dynamic> ignores = json.decode(rawResponse) as List<dynamic>;
for(dynamic ignore in ignores) {
final List<String> ignoredQueries = (ignore['query'] as String).split('&');
final String ignoredPullRequest = (ignore['note'] as String).split('/').last;
final DateTime expiration = DateTime.parse(ignore['expires'] as String);
// The currently failing test is in the process of modification.
if (ignoredQueries.contains('name=$testName')) {
if (expiration.isAfter(DateTime.now())) {
ignoreIsActive = true;
} else {
// If any ignore is expired for the given test, throw with
// guidance.
final StringBuffer buf = StringBuffer()
..writeln('This test has an expired ignore in place, and the')
..writeln('change has not been triaged.')
..writeln('The associated pull request is:')
..writeln('https://github.com/flutter/flutter/pull/$ignoredPullRequest');
throw NonZeroExitCode(1, buf.toString());
}
}
}
} on FormatException catch(_) {
if (rawResponse.contains('stream timeout')) {
final StringBuffer buf = StringBuffer()
..writeln('Stream timeout on /ignores api.')
..writeln('This may be caused by a failure to triage a change.')
..writeln('Check https://flutter-gold.skia.org/ignores, or')
..writeln('https://flutter-gold.skia.org/?query=source_type%3Dflutter')
..writeln('for untriaged golden files.');
throw NonZeroExitCode(1, buf.toString());
} else {
print('Formatting error detected requesting /ignores from Flutter Gold.'
'\nrawResponse: $rawResponse');
rethrow;
}
}
},
SkiaGoldHttpOverrides(),
);
return ignoreIsActive;
}
/// The [_expectations] retrieved from Flutter Gold do not include the
/// parameters of the given test. This function queries the Flutter Gold
/// details api to determine if the given expectation for a test matches the
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment