Skip to content

Commit

Permalink
Use flutter repo for engine golds instead of flutter-engine. (flu…
Browse files Browse the repository at this point in the history
…tter#160556)

Closes 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).
  • Loading branch information
matanlurey authored Jan 7, 2025
1 parent 95c1bc1 commit cda3515
Show file tree
Hide file tree
Showing 3 changed files with 70 additions and 7 deletions.
2 changes: 1 addition & 1 deletion engine/src/flutter/testing/dart/canvas_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ String get _flutterBuildPath {
}

void main() async {
final ImageComparer comparer = await ImageComparer.create();
final ImageComparer comparer = await ImageComparer.create(verbose: true);

testNoCrashes();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ const String _kGoldctlKey = 'GOLDCTL';
const String _kPresubmitEnvName = 'GOLD_TRYJOB';
const String _kLuciEnvName = 'LUCI_CONTEXT';

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

/// Uploads images and makes baseline requests to Skia Gold.
///
Expand All @@ -47,10 +47,16 @@ interface class SkiaGoldClient {
/// spammy for regular use.
factory SkiaGoldClient(
io.Directory workDirectory, {
String prefix = 'engine.',
Map<String, String>? dimensions,
bool verbose = false,
}) {
return SkiaGoldClient.forTesting(workDirectory, dimensions: dimensions, verbose: verbose);
return SkiaGoldClient.forTesting(
workDirectory,
dimensions: dimensions,
verbose: verbose,
prefix: prefix,
);
}

/// Creates a [SkiaGoldClient] for testing.
Expand All @@ -73,13 +79,15 @@ interface class SkiaGoldClient {
this.workDirectory, {
this.dimensions,
this.verbose = false,
String? prefix,
io.HttpClient? httpClient,
ProcessManager? processManager,
StringSink? stderr,
Map<String, String>? environment,
Engine? engineRoot,
}) : httpClient = httpClient ?? io.HttpClient(),
process = processManager ?? const LocalProcessManager(),
_prefix = prefix,
_stderr = stderr ?? io.stderr,
_environment = environment ?? io.Platform.environment,
_engineRoot = engineRoot ?? Engine.findWithin() {
Expand Down Expand Up @@ -166,6 +174,9 @@ interface class SkiaGoldClient {
/// client will create image and JSON files for the `goldctl` tool to use.
final io.Directory workDirectory;

/// Prefix to add to all test names, if any.
final String? _prefix;

String get _tempPath => path.join(workDirectory.path, 'temp');
String get _keysPath => path.join(workDirectory.path, 'keys.json');
String get _failuresPath => path.join(workDirectory.path, 'failures.json');
Expand Down Expand Up @@ -345,6 +356,11 @@ interface class SkiaGoldClient {
// Clean the test name to remove the file extension.
testName = path.basenameWithoutExtension(testName);

// Add a prefix to avoid repo-wide conflicts.
if (_prefix != null) {
testName = '$_prefix$testName';
}

// In release branches, we add a unique test suffix to the test name.
// For example "testName" -> "testName_Release_3_21".
// See ../README.md#release-testing for more information.
Expand Down Expand Up @@ -408,7 +424,7 @@ interface class SkiaGoldClient {
..writeln('testing. Golden file images in flutter/engine are triaged ')
..writeln('in pre-submit during code review for the given PR.')
..writeln()
..writeln('Visit https://flutter-engine-gold.skia.org/ to view and approve ')
..writeln('Visit https://flutter-gold.skia.org/ to view and approve ')
..writeln('the image(s), or revert the associated change. For more ')
..writeln('information, visit the wiki: ')
..writeln(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ void main() {
const Map<String, String> presubmitEnv = <String, String>{
'GIT_BRANCH': 'master',
'GOLDCTL': 'python tools/goldctl.py',
'GOLD_TRYJOB': 'flutter/engine/1234567890',
'GOLD_TRYJOB': 'flutter/flutter/1234567890',
'LOGDOG_STREAM_PREFIX': 'buildbucket/cr-buildbucket.appspot.com/1234567890/+/logdog',
'LUCI_CONTEXT': '{}',
};
Expand Down Expand Up @@ -50,6 +50,7 @@ void main() {
required Map<String, String> environment,
ReleaseVersion? engineVersion,
Map<String, String>? dimensions,
String? prefix,
bool verbose = false,
io.ProcessResult Function(List<String> command) onRun = _runUnhandled,
}) {
Expand All @@ -62,6 +63,7 @@ void main() {
verbose: verbose,
stderr: fixture.outputSink,
environment: environment,
prefix: prefix,
);
}

Expand Down Expand Up @@ -306,6 +308,51 @@ void main() {
}
});

test('addImg uses prefix, if specified', () async {
final _TestFixture fixture = _TestFixture();
try {
final SkiaGoldClient client = createClient(
fixture,
environment: presubmitEnv,
prefix: 'engine.',
onRun: (List<String> command) {
if (command case ['git', ...]) {
return io.ProcessResult(0, 0, mockCommitHash, '');
}
if (command case ['python tools/goldctl.py', 'imgtest', 'init', ...]) {
return io.ProcessResult(0, 0, '', '');
}
expect(command, <String>[
'python tools/goldctl.py',
'imgtest',
'add',
'--work-dir',
p.join(fixture.workDirectory.path, 'temp'),
'--test-name',
'engine.test-name',
'--png-file',
p.join(fixture.workDirectory.path, 'temp', 'golden.png'),
'--add-test-optional-key',
'image_matching_algorithm:fuzzy',
'--add-test-optional-key',
'fuzzy_max_different_pixels:10',
'--add-test-optional-key',
'fuzzy_pixel_delta_threshold:0',
]);
return io.ProcessResult(0, 0, '', '');
},
);

await client.addImg(
'test-name.foo',
io.File(p.join(fixture.workDirectory.path, 'temp', 'golden.png')),
screenshotSize: 1000,
);
} finally {
fixture.dispose();
}
});

test('addImg [pre-submit] executes successfully with a release version', () async {
// Adds a suffix of "_Release_3_21" to the test name.
final _TestFixture fixture = _TestFixture(
Expand Down Expand Up @@ -639,7 +686,7 @@ void main() {

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

Expand Down

0 comments on commit cda3515

Please sign in to comment.