From 59fd4f97910e91425bfc2228bd50120634bbb4b0 Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Tue, 11 Feb 2025 09:54:05 -0800 Subject: [PATCH] Fix `SkiaException` -> `TestFailure`, add tests. (#163054) Fixes https://github.com/flutter/flutter/issues/163051, mitigates https://github.com/flutter/flutter/issues/162362. --- .../lib/skia_gold.dart | 40 ++++++++++--- .../test/matches_golden_file_test.dart | 58 +++++++++++++++++++ .../test/skia_gold_test.dart | 44 ++++++++++++++ 3 files changed, 135 insertions(+), 7 deletions(-) create mode 100644 dev/tools/android_driver_extensions/test/matches_golden_file_test.dart create mode 100644 dev/tools/android_driver_extensions/test/skia_gold_test.dart diff --git a/dev/tools/android_driver_extensions/lib/skia_gold.dart b/dev/tools/android_driver_extensions/lib/skia_gold.dart index aa1c49d8be..21f2f1e41b 100644 --- a/dev/tools/android_driver_extensions/lib/skia_gold.dart +++ b/dev/tools/android_driver_extensions/lib/skia_gold.dart @@ -13,9 +13,11 @@ import 'dart:typed_data'; import 'package:file/local.dart'; import 'package:flutter_goldens/skia_client.dart'; +import 'package:meta/meta.dart'; import 'package:path/path.dart' as path; import 'package:platform/platform.dart'; import 'package:process/process.dart'; +import 'package:test_api/test_api.dart'; import 'native_driver.dart'; @@ -69,16 +71,30 @@ Future enableSkiaGoldComparator({String? namePrefix}) async { httpClient: io.HttpClient(), log: io.stderr.writeln, ); - await skiaGoldClient.auth(); - goldenFileComparator = _GoldenFileComparator( + await enableSkiaGoldComparatorForTesting( skiaGoldClient, namePrefix: namePrefix, - isPresubmit: isPresubmit, + presubmit: isPresubmit, ); } -final class _GoldenFileComparator extends GoldenFileComparator { - _GoldenFileComparator(this.skiaClient, {required this.isPresubmit, this.namePrefix, Uri? baseDir}) +/// Configures [goldenFileComparator] to use Skia Gold (for unit testing). +@visibleForTesting +Future enableSkiaGoldComparatorForTesting( + SkiaGoldClient skiaGoldClient, { + required bool presubmit, + String? namePrefix, +}) async { + await skiaGoldClient.auth(); + goldenFileComparator = _SkiaGoldComparator( + skiaGoldClient, + namePrefix: namePrefix, + isPresubmit: presubmit, + ); +} + +final class _SkiaGoldComparator extends GoldenFileComparator { + _SkiaGoldComparator(this.skiaClient, {required this.isPresubmit, this.namePrefix, Uri? baseDir}) : baseDir = baseDir ?? Uri.parse(path.dirname(io.Platform.script.path)); final Uri baseDir; @@ -108,8 +124,18 @@ final class _GoldenFileComparator extends GoldenFileComparator { io.stderr.writeln('Skia Gold comparison succeeded comparing "$golden".'); } return true; - } else { - return skiaClient.imgtestAdd(golden.path, _localFs.file(goldenFile.path)); + } + + try { + return await skiaClient.imgtestAdd(golden.path, _localFs.file(goldenFile.path)); + } on SkiaException catch (e) { + // Convert SkiaException -> TestFailure so that this class implements the + // contract of GoldenFileComparator, and matchesGoldenFile() converts the + // TestFailure into a standard reported test error (with a better stack + // trace, for example). + // + // https://github.com/flutter/flutter/issues/162621 + throw TestFailure('$e'); } } diff --git a/dev/tools/android_driver_extensions/test/matches_golden_file_test.dart b/dev/tools/android_driver_extensions/test/matches_golden_file_test.dart new file mode 100644 index 0000000000..6a24047265 --- /dev/null +++ b/dev/tools/android_driver_extensions/test/matches_golden_file_test.dart @@ -0,0 +1,58 @@ +// Copyright 2014 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'dart:typed_data'; + +import 'package:android_driver_extensions/native_driver.dart'; +// Similar to `flutter_test`, we ignore the implementation import. +// ignore: implementation_imports +import 'package:matcher/src/expect/async_matcher.dart'; +import 'package:test/fake.dart'; +import 'package:test/test.dart'; + +void main() { + test('passes when the comparator passes', () async { + goldenFileComparator = _FakeGoldenFileComparator(_CannedComparisonMode.alwaysPass); + final AsyncMatcher matcher = matchesGoldenFile('test'); + await expectLater(matcher.matchAsync(Uint8List(0)), completion(isNull)); + }); + + test('fails with a default message when the comparator fails', () async { + goldenFileComparator = _FakeGoldenFileComparator(_CannedComparisonMode.alwaysFail); + final AsyncMatcher matcher = matchesGoldenFile('test'); + await expectLater(matcher.matchAsync(Uint8List(0)), completion(contains('does not match'))); + }); + + test('fails when the comparator throws a TestFailure', () async { + goldenFileComparator = _FakeGoldenFileComparator(_CannedComparisonMode.alwaysThrowTestFailure); + final AsyncMatcher matcher = matchesGoldenFile('test'); + await expectLater(matcher.matchAsync(Uint8List(0)), completion(contains('An expected error'))); + }); + + test('unhandled exception when the comparator throws anything but TestFailure', () async { + goldenFileComparator = _FakeGoldenFileComparator(_CannedComparisonMode.alwaysThrowStateError); + final AsyncMatcher matcher = matchesGoldenFile('test'); + await expectLater(matcher.matchAsync(Uint8List(0)), throwsStateError); + }); +} + +final class _FakeGoldenFileComparator extends Fake implements GoldenFileComparator { + _FakeGoldenFileComparator(this._mode); + final _CannedComparisonMode _mode; + + @override + Future compare(Uint8List imageBytes, Uri golden) async { + return switch (_mode) { + _CannedComparisonMode.alwaysPass => true, + _CannedComparisonMode.alwaysFail => false, + _CannedComparisonMode.alwaysThrowTestFailure => throw TestFailure('An expected error'), + _CannedComparisonMode.alwaysThrowStateError => throw StateError('An unexpected error'), + }; + } + + @override + Uri getTestUri(Uri key, int? version) => key; +} + +enum _CannedComparisonMode { alwaysPass, alwaysFail, alwaysThrowTestFailure, alwaysThrowStateError } diff --git a/dev/tools/android_driver_extensions/test/skia_gold_test.dart b/dev/tools/android_driver_extensions/test/skia_gold_test.dart new file mode 100644 index 0000000000..7a13e8f6f4 --- /dev/null +++ b/dev/tools/android_driver_extensions/test/skia_gold_test.dart @@ -0,0 +1,44 @@ +// Copyright 2014 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'dart:typed_data'; + +import 'package:android_driver_extensions/native_driver.dart'; +import 'package:android_driver_extensions/skia_gold.dart'; +import 'package:file/src/interface/file.dart'; +import 'package:flutter_goldens/skia_client.dart'; +import 'package:test/fake.dart'; +import 'package:test/test.dart'; + +void main() { + // Regression test for https://github.com/flutter/flutter/issues/163051. + test('converts SkiaException to TestFailure in postsubmit', () async { + final SkiaGoldClient skiaGold = _ThrowsSkiaException(); + await enableSkiaGoldComparatorForTesting(skiaGold, presubmit: false); + + await expectLater( + goldenFileComparator.compare(Uint8List(0), Uri(path: 'test.png')), + throwsA( + isA().having( + (Object e) => '$e', + 'description', + contains('Skia Gold received an unapproved image in post'), + ), + ), + ); + }); +} + +final class _ThrowsSkiaException extends Fake implements SkiaGoldClient { + @override + Future auth() async {} + + @override + Future imgtestInit() async {} + + @override + Future imgtestAdd(String testName, File goldenFile) async { + throw const SkiaException('Skia Gold received an unapproved image in post'); + } +}