Skip to content

Commit cda3515

Browse files
authored
Use flutter repo for engine golds instead of flutter-engine. (#160556)
Closes flutter/flutter#157206. I also added a `prefix` that will default to `engine.` to avoid accidentally stomping on golden names across repos. /cc @gaaclarke for visibility, @Piinks for visibility. (I would love to get rid of this "engine copy" of the client as part of longer-term mono repo deduplication).
1 parent 95c1bc1 commit cda3515

File tree

3 files changed

+70
-7
lines changed

3 files changed

+70
-7
lines changed

engine/src/flutter/testing/dart/canvas_test.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@ String get _flutterBuildPath {
173173
}
174174

175175
void main() async {
176-
final ImageComparer comparer = await ImageComparer.create();
176+
final ImageComparer comparer = await ImageComparer.create(verbose: true);
177177

178178
testNoCrashes();
179179

engine/src/flutter/testing/skia_gold_client/lib/skia_gold_client.dart

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,8 @@ const String _kGoldctlKey = 'GOLDCTL';
2020
const String _kPresubmitEnvName = 'GOLD_TRYJOB';
2121
const String _kLuciEnvName = 'LUCI_CONTEXT';
2222

23-
const String _skiaGoldHost = 'https://flutter-engine-gold.skia.org';
24-
const String _instance = 'flutter-engine';
23+
const String _skiaGoldHost = 'https://flutter-gold.skia.org';
24+
const String _instance = 'flutter';
2525

2626
/// Uploads images and makes baseline requests to Skia Gold.
2727
///
@@ -47,10 +47,16 @@ interface class SkiaGoldClient {
4747
/// spammy for regular use.
4848
factory SkiaGoldClient(
4949
io.Directory workDirectory, {
50+
String prefix = 'engine.',
5051
Map<String, String>? dimensions,
5152
bool verbose = false,
5253
}) {
53-
return SkiaGoldClient.forTesting(workDirectory, dimensions: dimensions, verbose: verbose);
54+
return SkiaGoldClient.forTesting(
55+
workDirectory,
56+
dimensions: dimensions,
57+
verbose: verbose,
58+
prefix: prefix,
59+
);
5460
}
5561

5662
/// Creates a [SkiaGoldClient] for testing.
@@ -73,13 +79,15 @@ interface class SkiaGoldClient {
7379
this.workDirectory, {
7480
this.dimensions,
7581
this.verbose = false,
82+
String? prefix,
7683
io.HttpClient? httpClient,
7784
ProcessManager? processManager,
7885
StringSink? stderr,
7986
Map<String, String>? environment,
8087
Engine? engineRoot,
8188
}) : httpClient = httpClient ?? io.HttpClient(),
8289
process = processManager ?? const LocalProcessManager(),
90+
_prefix = prefix,
8391
_stderr = stderr ?? io.stderr,
8492
_environment = environment ?? io.Platform.environment,
8593
_engineRoot = engineRoot ?? Engine.findWithin() {
@@ -166,6 +174,9 @@ interface class SkiaGoldClient {
166174
/// client will create image and JSON files for the `goldctl` tool to use.
167175
final io.Directory workDirectory;
168176

177+
/// Prefix to add to all test names, if any.
178+
final String? _prefix;
179+
169180
String get _tempPath => path.join(workDirectory.path, 'temp');
170181
String get _keysPath => path.join(workDirectory.path, 'keys.json');
171182
String get _failuresPath => path.join(workDirectory.path, 'failures.json');
@@ -345,6 +356,11 @@ interface class SkiaGoldClient {
345356
// Clean the test name to remove the file extension.
346357
testName = path.basenameWithoutExtension(testName);
347358

359+
// Add a prefix to avoid repo-wide conflicts.
360+
if (_prefix != null) {
361+
testName = '$_prefix$testName';
362+
}
363+
348364
// In release branches, we add a unique test suffix to the test name.
349365
// For example "testName" -> "testName_Release_3_21".
350366
// See ../README.md#release-testing for more information.
@@ -408,7 +424,7 @@ interface class SkiaGoldClient {
408424
..writeln('testing. Golden file images in flutter/engine are triaged ')
409425
..writeln('in pre-submit during code review for the given PR.')
410426
..writeln()
411-
..writeln('Visit https://flutter-engine-gold.skia.org/ to view and approve ')
427+
..writeln('Visit https://flutter-gold.skia.org/ to view and approve ')
412428
..writeln('the image(s), or revert the associated change. For more ')
413429
..writeln('information, visit the wiki: ')
414430
..writeln(

engine/src/flutter/testing/skia_gold_client/test/skia_gold_client_test.dart

Lines changed: 49 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ void main() {
2222
const Map<String, String> presubmitEnv = <String, String>{
2323
'GIT_BRANCH': 'master',
2424
'GOLDCTL': 'python tools/goldctl.py',
25-
'GOLD_TRYJOB': 'flutter/engine/1234567890',
25+
'GOLD_TRYJOB': 'flutter/flutter/1234567890',
2626
'LOGDOG_STREAM_PREFIX': 'buildbucket/cr-buildbucket.appspot.com/1234567890/+/logdog',
2727
'LUCI_CONTEXT': '{}',
2828
};
@@ -50,6 +50,7 @@ void main() {
5050
required Map<String, String> environment,
5151
ReleaseVersion? engineVersion,
5252
Map<String, String>? dimensions,
53+
String? prefix,
5354
bool verbose = false,
5455
io.ProcessResult Function(List<String> command) onRun = _runUnhandled,
5556
}) {
@@ -62,6 +63,7 @@ void main() {
6263
verbose: verbose,
6364
stderr: fixture.outputSink,
6465
environment: environment,
66+
prefix: prefix,
6567
);
6668
}
6769

@@ -306,6 +308,51 @@ void main() {
306308
}
307309
});
308310

311+
test('addImg uses prefix, if specified', () async {
312+
final _TestFixture fixture = _TestFixture();
313+
try {
314+
final SkiaGoldClient client = createClient(
315+
fixture,
316+
environment: presubmitEnv,
317+
prefix: 'engine.',
318+
onRun: (List<String> command) {
319+
if (command case ['git', ...]) {
320+
return io.ProcessResult(0, 0, mockCommitHash, '');
321+
}
322+
if (command case ['python tools/goldctl.py', 'imgtest', 'init', ...]) {
323+
return io.ProcessResult(0, 0, '', '');
324+
}
325+
expect(command, <String>[
326+
'python tools/goldctl.py',
327+
'imgtest',
328+
'add',
329+
'--work-dir',
330+
p.join(fixture.workDirectory.path, 'temp'),
331+
'--test-name',
332+
'engine.test-name',
333+
'--png-file',
334+
p.join(fixture.workDirectory.path, 'temp', 'golden.png'),
335+
'--add-test-optional-key',
336+
'image_matching_algorithm:fuzzy',
337+
'--add-test-optional-key',
338+
'fuzzy_max_different_pixels:10',
339+
'--add-test-optional-key',
340+
'fuzzy_pixel_delta_threshold:0',
341+
]);
342+
return io.ProcessResult(0, 0, '', '');
343+
},
344+
);
345+
346+
await client.addImg(
347+
'test-name.foo',
348+
io.File(p.join(fixture.workDirectory.path, 'temp', 'golden.png')),
349+
screenshotSize: 1000,
350+
);
351+
} finally {
352+
fixture.dispose();
353+
}
354+
});
355+
309356
test('addImg [pre-submit] executes successfully with a release version', () async {
310357
// Adds a suffix of "_Release_3_21" to the test name.
311358
final _TestFixture fixture = _TestFixture(
@@ -639,7 +686,7 @@ void main() {
639686

640687
final String hash = client.getTraceID('test-name');
641688
fixture.httpClient.setJsonResponse(
642-
Uri.parse('https://flutter-engine-gold.skia.org/json/v2/latestpositivedigest/$hash'),
689+
Uri.parse('https://flutter-gold.skia.org/json/v2/latestpositivedigest/$hash'),
643690
<String, Object?>{'digest': 'digest'},
644691
);
645692

0 commit comments

Comments
 (0)