Unverified Commit 1c15cd86 authored by Kate Lovett's avatar Kate Lovett Committed by GitHub

Fix Gold flake from gsutil fallback (#50149)

parent 05eae425
...@@ -341,7 +341,8 @@ class AnimationController extends Animation<double> ...@@ -341,7 +341,8 @@ class AnimationController extends Animation<double>
/// Stops the animation controller and sets the current value of the /// Stops the animation controller and sets the current value of the
/// animation. /// animation.
/// ///
/// The new value is clamped to the range set by [lowerBound] and [upperBound]. /// The new value is clamped to the range set by [lowerBound] and
/// [upperBound].
/// ///
/// Value listeners are notified even if this does not change the value. /// Value listeners are notified even if this does not change the value.
/// Status listeners are notified if the animation was previously playing. /// Status listeners are notified if the animation was previously playing.
......
...@@ -136,17 +136,31 @@ abstract class FlutterGoldenFileComparator extends GoldenFileComparator { ...@@ -136,17 +136,31 @@ abstract class FlutterGoldenFileComparator extends GoldenFileComparator {
/// maintain thread safety while using the `goldctl` tool. /// maintain thread safety while using the `goldctl` tool.
@protected @protected
@visibleForTesting @visibleForTesting
static Directory getBaseDirectory(LocalFileComparator defaultComparator, Platform platform, {String suffix = ''}) { static Directory getBaseDirectory(
LocalFileComparator defaultComparator,
Platform platform, {
String suffix = '',
bool local = false,
}) {
const FileSystem fs = LocalFileSystem(); const FileSystem fs = LocalFileSystem();
final Directory flutterRoot = fs.directory(platform.environment[_kFlutterRootKey]); final Directory flutterRoot = fs.directory(platform.environment[_kFlutterRootKey]);
final Directory comparisonRoot = flutterRoot.childDirectory( Directory comparisonRoot;
if (!local) {
comparisonRoot = fs.systemTempDirectory.childDirectory(
'skia_goldens$suffix'
);
} else {
comparisonRoot = flutterRoot.childDirectory(
fs.path.join( fs.path.join(
'bin', 'bin',
'cache', 'cache',
'pkg', 'pkg',
'skia_goldens$suffix', 'skia_goldens',
) )
); );
}
final Directory testDirectory = fs.directory(defaultComparator.basedir); final Directory testDirectory = fs.directory(defaultComparator.basedir);
final String testDirectoryRelativePath = fs.path.relative( final String testDirectoryRelativePath = fs.path.relative(
testDirectory.path, testDirectory.path,
...@@ -550,6 +564,7 @@ class FlutterLocalFileComparator extends FlutterGoldenFileComparator with LocalC ...@@ -550,6 +564,7 @@ class FlutterLocalFileComparator extends FlutterGoldenFileComparator with LocalC
baseDirectory ??= FlutterGoldenFileComparator.getBaseDirectory( baseDirectory ??= FlutterGoldenFileComparator.getBaseDirectory(
defaultComparator, defaultComparator,
platform, platform,
local: true,
); );
if(!baseDirectory.existsSync()) { if(!baseDirectory.existsSync()) {
......
...@@ -68,8 +68,9 @@ void main() { ...@@ -68,8 +68,9 @@ void main() {
}); });
test('auth performs minimal work if already authorized', () async { test('auth performs minimal work if already authorized', () async {
fs.file('/workDirectory/temp/auth_opt.json') final File authFile = fs.file('/workDirectory/temp/auth_opt.json')
..createSync(recursive: true); ..createSync(recursive: true);
authFile.writeAsStringSync(authTemplate());
when(process.run(any)) when(process.run(any))
.thenAnswer((_) => Future<ProcessResult> .thenAnswer((_) => Future<ProcessResult>
.value(ProcessResult(123, 0, '', ''))); .value(ProcessResult(123, 0, '', '')));
...@@ -81,6 +82,16 @@ void main() { ...@@ -81,6 +82,16 @@ void main() {
)); ));
}); });
test('gsutil is checked when authorization file is present', () async {
final File authFile = fs.file('/workDirectory/temp/auth_opt.json')
..createSync(recursive: true);
authFile.writeAsStringSync(authTemplate(gsutil: true));
expect(
await skiaClient.clientIsAuthorized(),
isFalse,
);
});
test('throws for error state from auth', () async { test('throws for error state from auth', () async {
platform = FakePlatform( platform = FakePlatform(
environment: <String, String>{ environment: <String, String>{
...@@ -110,7 +121,7 @@ void main() { ...@@ -110,7 +121,7 @@ void main() {
); );
}); });
test(' throws for error state from init', () { test('throws for error state from init', () {
platform = FakePlatform( platform = FakePlatform(
environment: <String, String>{ environment: <String, String>{
'FLUTTER_ROOT': _kFlutterRoot, 'FLUTTER_ROOT': _kFlutterRoot,
...@@ -458,7 +469,7 @@ void main() { ...@@ -458,7 +469,7 @@ void main() {
); );
}); });
test('calculates the basedir correctly from defaultComparator', () async { test('calculates the basedir correctly from defaultComparator for local testing', () async {
final MockLocalFileComparator defaultComparator = MockLocalFileComparator(); final MockLocalFileComparator defaultComparator = MockLocalFileComparator();
final Directory flutterRoot = fs.directory(platform.environment['FLUTTER_ROOT']) final Directory flutterRoot = fs.directory(platform.environment['FLUTTER_ROOT'])
..createSync(recursive: true); ..createSync(recursive: true);
...@@ -467,6 +478,7 @@ void main() { ...@@ -467,6 +478,7 @@ void main() {
final Directory basedir = FlutterGoldenFileComparator.getBaseDirectory( final Directory basedir = FlutterGoldenFileComparator.getBaseDirectory(
defaultComparator, defaultComparator,
platform, platform,
local: true,
); );
expect( expect(
basedir.uri, basedir.uri,
......
...@@ -2,6 +2,19 @@ ...@@ -2,6 +2,19 @@
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
/// JSON template for the contents of the auth_opt.json file created by goldctl.
String authTemplate({
bool gsutil = false,
}) {
return '''
{
"Luci":false,
"ServiceAccount":"${gsutil ? '' : '/packages/flutter/test/widgets/serviceAccount.json'}",
"GSUtil":$gsutil
}
''';
}
/// JSON response template for Skia Gold expectations request: /// JSON response template for Skia Gold expectations request:
/// https://flutter-gold.skia.org/json/expectations/commit/HEAD /// https://flutter-gold.skia.org/json/expectations/commit/HEAD
String rawExpectationsTemplate() { String rawExpectationsTemplate() {
......
...@@ -98,7 +98,7 @@ class SkiaGoldClient { ...@@ -98,7 +98,7 @@ class SkiaGoldClient {
/// will only be called once for each instance of /// will only be called once for each instance of
/// [FlutterSkiaGoldFileComparator]. /// [FlutterSkiaGoldFileComparator].
Future<void> auth() async { Future<void> auth() async {
if (_clientIsAuthorized()) if (await clientIsAuthorized())
return; return;
if (_serviceAccount.isEmpty) { if (_serviceAccount.isEmpty) {
...@@ -604,12 +604,18 @@ class SkiaGoldClient { ...@@ -604,12 +604,18 @@ class SkiaGoldClient {
/// Returns a boolean value to prevent the client from re-authorizing itself /// Returns a boolean value to prevent the client from re-authorizing itself
/// for multiple tests. /// for multiple tests.
bool _clientIsAuthorized() { Future<bool> clientIsAuthorized() async {
final File authFile = workDirectory?.childFile(fs.path.join( final File authFile = workDirectory?.childFile(fs.path.join(
'temp', 'temp',
'auth_opt.json', 'auth_opt.json',
)); ));
return authFile.existsSync();
if(await authFile.exists()) {
final String contents = await authFile.readAsString();
final Map<String, dynamic> decoded = json.decode(contents) as Map<String, dynamic>;
return !(decoded['GSUtil'] as bool);
}
return false;
} }
} }
......
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