Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove or replaced unused or duplicate code in SkiaGoldClient. #51399

Merged
merged 1 commit into from
Mar 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/web_ui/dev/steps/run_suite_step.dart
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ class RunSuiteStep implements PipelineStep {
Future<bool> _checkSkiaClient(SkiaGoldClient skiaClient) async {
// Now let's check whether Skia Gold is reachable or not.
if (isLuci) {
if (isSkiaGoldClientAvailable) {
if (SkiaGoldClient.isAvailable()) {
try {
await skiaClient.auth();
return true;
Expand Down
2 changes: 1 addition & 1 deletion testing/dart/goldens.dart
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ class ImageComparer {
final Map<String, String> dimensions = <String, String>{
'impeller_enabled': impellerEnabled.toString(),
};
final SkiaGoldClient client = isSkiaGoldClientAvailable && _useSkiaGold
final SkiaGoldClient client = SkiaGoldClient.isAvailable() && _useSkiaGold
? SkiaGoldClient(workDirectory,
dimensions: dimensions, verbose: verbose)
: _FakeSkiaGoldClient(workDirectory, dimensions, verbose: verbose);
Expand Down
4 changes: 2 additions & 2 deletions testing/scenario_app/bin/run_android_tests.dart
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ Future<void> _run({
if (verbose) {
log('wrote ${goldenFile.absolute.path}');
}
if (isSkiaGoldClientAvailable) {
if (SkiaGoldClient.isAvailable()) {
final Future<void> comparison = skiaGoldClient!
// Each color channel can be off by 2.
.addImg(fileName, goldenFile, screenshotSize: screenshot.pixelCount, pixelColorDelta: 8)
Expand Down Expand Up @@ -311,7 +311,7 @@ Future<void> _run({
});

await step('Skia Gold auth...', () async {
if (isSkiaGoldClientAvailable) {
if (SkiaGoldClient.isAvailable()) {
await skiaGoldClient!.auth();
log('skia gold client is available');
} else {
Expand Down
4 changes: 2 additions & 2 deletions testing/skia_gold_client/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ The web UI is available on https://flutter-engine-gold.skia.org/.
1. In `.ci.yaml`, ensure that the task has a dependency on `goldctl`:

```yaml
dependencies: [{"dependency": "goldctl"}]
dependencies: [{ "dependency": "goldctl" }]
```

2. In the builder `.json` file, ensure the drone has a dependency on `goldctl`:
Expand Down Expand Up @@ -50,7 +50,7 @@ Future<void> main() {
);

try {
if (isSkiaGoldClientAvailable) {
if (SkiaGoldClient.isAvailable()) {
await client.auth();

await client.addImg(
Expand Down
56 changes: 21 additions & 35 deletions testing/skia_gold_client/lib/skia_gold_client.dart
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,6 @@ const String _kLuciEnvName = 'LUCI_CONTEXT';
const String _skiaGoldHost = 'https://flutter-engine-gold.skia.org';
const String _instance = 'flutter-engine';

/// Whether the Skia Gold client is available and can be used in this
/// environment.
bool get isSkiaGoldClientAvailable => SkiaGoldClient.isAvailable();

/// Returns true if the current environment is a LUCI builder.
bool get isLuciEnv => io.Platform.environment.containsKey(_kLuciEnvName);

/// A client for uploading image tests and making baseline requests to the
/// Flutter Gold Dashboard.
interface class SkiaGoldClient {
Expand Down Expand Up @@ -463,32 +456,28 @@ interface class SkiaGoldClient {
Future<String?> getExpectationForTest(String testName) async {
late String? expectation;
final String traceID = getTraceID(testName);
await io.HttpOverrides.runWithHttpOverrides<Future<void>>(() async {
final Uri requestForExpectations = Uri.parse(
'$_skiaGoldHost/json/v2/latestpositivedigest/$traceID'
);
late String rawResponse;
try {
final io.HttpClientRequest request = await httpClient.getUrl(requestForExpectations);
final io.HttpClientResponse response = await request.close();
rawResponse = await utf8.decodeStream(response);
final dynamic jsonResponse = json.decode(rawResponse);
if (jsonResponse is! Map<String, dynamic>) {
throw const FormatException('Skia gold expectations do not match expected format.');
}
expectation = jsonResponse['digest'] as String?;
} on FormatException catch (error) {
_stderr.writeln(
'Formatting error detected requesting expectations from Flutter Gold.\n'
'error: $error\n'
'url: $requestForExpectations\n'
'response: $rawResponse'
);
rethrow;
}
},
SkiaGoldHttpOverrides(),
final Uri requestForExpectations = Uri.parse(
'$_skiaGoldHost/json/v2/latestpositivedigest/$traceID'
);
late String rawResponse;
try {
final io.HttpClientRequest request = await httpClient.getUrl(requestForExpectations);
final io.HttpClientResponse response = await request.close();
rawResponse = await utf8.decodeStream(response);
final dynamic jsonResponse = json.decode(rawResponse);
if (jsonResponse is! Map<String, dynamic>) {
throw const FormatException('Skia gold expectations do not match expected format.');
}
expectation = jsonResponse['digest'] as String?;
} on FormatException catch (error) {
_stderr.writeln(
'Formatting error detected requesting expectations from Flutter Gold.\n'
'error: $error\n'
'url: $requestForExpectations\n'
'response: $rawResponse'
);
rethrow;
}
return expectation;
}

Expand Down Expand Up @@ -561,6 +550,3 @@ interface class SkiaGoldClient {
return md5Sum;
}
}

/// Used to make HttpRequests during testing.
class SkiaGoldHttpOverrides extends io.HttpOverrides { }
2 changes: 1 addition & 1 deletion web_sdk/web_test_utils/lib/image_compare.dart
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ Future<String> compareImage(
await screenshotFile.create(recursive: true);
await screenshotFile.writeAsBytes(encodePng(screenshot), flush: true);

if (isLuciEnv) {
if (SkiaGoldClient.isLuciEnv()) {
// This is temporary to get started by uploading existing screenshots to
// Skia Gold. The next step would be to actually use Skia Gold for
// comparison.
Expand Down