From a5609dfa5aa8fadbd7d2f152df9b5d51fb3730d0 Mon Sep 17 00:00:00 2001 From: Ian Hickson Date: Wed, 31 Jul 2024 12:52:08 -0700 Subject: [PATCH] Remove redundant usages of zones in skia_client.dart (#149366) The SkiaGoldHttpOverrides don't have any effect since we never build our own HttpClient, it's always passed in. This is part 18 of a broken down version of the #140101 refactor. This particular change is more risky than other changes in this series. While by inspection and some instrumentation testing I'm reasonably sure that my assumptions here are correct, it would behoove us to make sure Skia Gold testing still works post-commit after this lands. To this end, I've included a change to the "inconsequential" test. It should fail the tests. I recommend we land this _with this failure_ to make sure it also fails post-commit, then immediately flag the image as passing and rerun the relevant shard. --- packages/flutter/test/widgets/basic_test.dart | 2 +- .../flutter_goldens/lib/flutter_goldens.dart | 8 ++- packages/flutter_goldens/lib/skia_client.dart | 63 ++++++++----------- 3 files changed, 34 insertions(+), 39 deletions(-) diff --git a/packages/flutter/test/widgets/basic_test.dart b/packages/flutter/test/widgets/basic_test.dart index ee472a9085..9528795dcc 100644 --- a/packages/flutter/test/widgets/basic_test.dart +++ b/packages/flutter/test/widgets/basic_test.dart @@ -750,7 +750,7 @@ void main() { // golden file can be approved at any time. await tester.pumpWidget(RepaintBoundary( child: Container( - color: const Color(0xFFF40125), + color: const Color(0xFF161145), ), )); diff --git a/packages/flutter_goldens/lib/flutter_goldens.dart b/packages/flutter_goldens/lib/flutter_goldens.dart index cbe4de712e..234be6b369 100644 --- a/packages/flutter_goldens/lib/flutter_goldens.dart +++ b/packages/flutter_goldens/lib/flutter_goldens.dart @@ -35,7 +35,7 @@ bool _isMainBranch(String? branch) { /// Main method that can be used in a `flutter_test_config.dart` file to set /// [goldenFileComparator] to an instance of [FlutterGoldenFileComparator] that -/// works for the current test. _Which_ FlutterGoldenFileComparator is +/// works for the current test. _Which_ [FlutterGoldenFileComparator] is /// instantiated is based on the current testing environment. /// /// When set, the `namePrefix` is prepended to the names of all gold images. @@ -45,6 +45,12 @@ bool _isMainBranch(String? branch) { /// tests using `flutter test`. This should not be called when running a test /// using `flutter run`, as in that environment, the [goldenFileComparator] is a /// [TrivialComparator]. +/// +/// An [HttpClient] is created when this method is called. That client is used +/// to communicate with the Skia Gold servers. Any [HttpOverrides] set in this +/// will affect whether this is effective or not. For example, if the current +/// override provides a mock client that always fails, then all calls to gold +/// comparison functions will fail. Future testExecutable(FutureOr Function() testMain, {String? namePrefix}) async { assert( goldenFileComparator is LocalFileComparator, diff --git a/packages/flutter_goldens/lib/skia_client.dart b/packages/flutter_goldens/lib/skia_client.dart index 7066f0c5f9..6950c0c883 100644 --- a/packages/flutter_goldens/lib/skia_client.dart +++ b/packages/flutter_goldens/lib/skia_client.dart @@ -420,32 +420,28 @@ class SkiaGoldClient { Future getExpectationForTest(String testName) async { late String? expectation; final String traceID = getTraceID(testName); - await io.HttpOverrides.runWithHttpOverrides>(() async { - final Uri requestForExpectations = Uri.parse( - 'https://flutter-gold.skia.org/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) { - throw const FormatException('Skia gold expectations do not match expected format.'); - } - expectation = jsonResponse['digest'] as String?; - } on FormatException catch (error) { - log( - 'Formatting error detected requesting expectations from Flutter Gold.\n' - 'error: $error\n' - 'url: $requestForExpectations\n' - 'response: $rawResponse' - ); - rethrow; - } - }, - SkiaGoldHttpOverrides(), + final Uri requestForExpectations = Uri.parse( + 'https://flutter-gold.skia.org/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) { + throw const FormatException('Skia gold expectations do not match expected format.'); + } + expectation = jsonResponse['digest'] as String?; + } on FormatException catch (error) { + log( + 'Formatting error detected requesting expectations from Flutter Gold.\n' + 'error: $error\n' + 'url: $requestForExpectations\n' + 'response: $rawResponse' + ); + rethrow; + } return expectation; } @@ -455,16 +451,12 @@ class SkiaGoldClient { /// The provided image hash represents an expectation from Flutter Gold. Future>getImageBytes(String imageHash) async { final List imageBytes = []; - await io.HttpOverrides.runWithHttpOverrides>(() async { - final Uri requestForImage = Uri.parse( - 'https://flutter-gold.skia.org/img/images/$imageHash.png', - ); - final io.HttpClientRequest request = await httpClient.getUrl(requestForImage); - final io.HttpClientResponse response = await request.close(); - await response.forEach((List bytes) => imageBytes.addAll(bytes)); - }, - SkiaGoldHttpOverrides(), + final Uri requestForImage = Uri.parse( + 'https://flutter-gold.skia.org/img/images/$imageHash.png', ); + final io.HttpClientRequest request = await httpClient.getUrl(requestForImage); + final io.HttpClientResponse response = await request.close(); + await response.forEach((List bytes) => imageBytes.addAll(bytes)); return imageBytes; } @@ -594,6 +586,3 @@ class SkiaGoldClient { return md5Sum; } } - -/// Used to make HttpRequests during testing. -class SkiaGoldHttpOverrides extends io.HttpOverrides { }