Merge pull request #59 from eBay/image-loading

Switch to better implementation of priming images for goldens

packages/golden_toolkit/CHANGELOG.md

@@ -2,6 +2,25 @@
 
 ## 0.5.0
 
+### More intelligent behavior for loading assets
+
+A new mechanism has been added for ensuring that images have been decoded before capturing goldens. The old implementation worked most of the time, but was non-deterministic and hacky. The new implementation inspects the widget tree to identify images that need to be loaded. It should display images more consistently in goldens.
+
+This may be a breaking change for some consumers. If you run into issues, you can revert to the old behavior, by applying the following configuration:
+
+```GoldenToolkitConfiguration(primeAssets: legacyPrimeAssets);```
+
+Additionally, you can provide your own implementation that extends the new default behavior:
+
+```dart
+GoldenToolkitConfiguration(primeAssets: (tester) async {
+ await defaultPrimeAssets(tester);
+ /* do anything custom */
+});
+```
+
+If you run into issues, please submit issues so we can expand on the default behavior. We expect that it is likely missing some cases.
+
 ### New API for configuring the toolkit
 
 Reworked the configuration API introduced in 0.4.0. The prior method relied on global state and could be error prone. The old implementation still functions, but has been marked as deprecated and will be removed in a future release.
@@ -37,16 +56,20 @@ Added the following extensions. These can be used with any vanilla golden assert
 
 ```dart
 // configures the simulated device to mirror the supplied device configuration (dimensions, pixel density, safe area, etc)
-tester.binding.applyDeviceOverrides(device);
+await tester.binding.applyDeviceOverrides(device);
 
 // resets any configuration applied by applyDeviceOverrides
-tester.binding.resetDeviceOverrides();
+await tester.binding.resetDeviceOverrides();
 
 // runs a block of code with the simulated device settings and automatically clears upon completion
-tester.binding.runWithDeviceOverrides(device, body: (){});
+await tester.binding.runWithDeviceOverrides(device, body: (){});
 
 // convenience helper for configurating the safe area... the built-in paddingTestValue is difficult to work with
 tester.binding.window.safeAreaTestValue = EdgeInsets.all(8);
+
+// a stand-alone version of the image loading mechanism described at the top of these release notes. Will wait for all images to be decoded
+// so that they will for sure appear in the golden.
+await tester.waitForAssets();
 ```
 
 ### Misc Changes

packages/golden_toolkit/example/test/example_test.dart

@@ -85,6 +85,24 @@ void main() {
         overrideGoldenHeight: 1200,
       );
     });
+
+    group('GoldenBuilder examples of accessibility testing', () {
+      // With those test we want to make sure our widgets look right when user changes system font size
+      testGoldens('Card should look right when user bumps system font size', (tester) async {
+        const widget = WeatherCard(temp: 56, weather: Weather.cloudy);
+
+        final gb = GoldenBuilder.column(bgColor: Colors.white, wrap: _simpleFrame)
+          ..addScenario('Regular font size', widget)
+          ..addTextScaleScenario('Large font size', widget, textScaleFactor: 2.0)
+          ..addTextScaleScenario('Largest font size', widget, textScaleFactor: 3.2);
+
+        await tester.pumpWidgetBuilder(
+          gb.build(),
+          surfaceSize: const Size(200, 1000),
+        );
+        await screenMatchesGolden(tester, 'weather_accessibility');
+      });
+    });
   });
 
   group('Multi-Screen Golden', () {

packages/golden_toolkit/lib/src/configuration.dart

@@ -10,8 +10,9 @@ import 'package:flutter/foundation.dart';
 /// https://opensource.org/licenses/BSD-3-Clause
 /// ***************************************************
 
-import 'package:meta/meta.dart';
 import 'package:flutter_test/flutter_test.dart';
+import 'package:meta/meta.dart';
+import '../golden_toolkit.dart';
 import 'device.dart';
 
 /// Manages global state & behavior for the Golden Toolkit
@@ -49,6 +50,7 @@ class GoldenToolkit {
   }
 }
 
+/// A func that will be evaluated at runtime to determine if the golden assertion should be skipped
 typedef SkipGoldenAssertion = bool Function();
 
 /// A factory to determine an actual file name/path from a given name.
@@ -65,17 +67,31 @@ typedef FileNameFactory = String Function(String name);
 /// * [GoldenToolkitConfiguration] to configure a global device file name factory.
 typedef DeviceFileNameFactory = String Function(String name, Device device);
 
+/// A function that primes all needed assets for the given [tester].
+///
+/// For ready to use implementations see:
+/// * [legacyPrimeAssets], which is the default [PrimeAssets] used by the global configuration by default.
+/// * [defaultPrimeAssets], which just waits for all [Image] widgets in the widget tree to finish decoding.
+typedef PrimeAssets = Future<void> Function(WidgetTester tester);
+
 /// Represents configuration options for the GoldenToolkit. These are akin to environmental flags.
 @immutable
 class GoldenToolkitConfiguration {
   /// GoldenToolkitConfiguration constructor
   ///
   /// [skipGoldenAssertion] a func that returns a bool as to whether the golden assertion should be skipped.
   /// A typical example may be to skip when the assertion is invoked on certain platforms. For example: () => !Platform.isMacOS
+  ///
+  /// [fileNameFactory] a func used to decide the final filename for screenMatchesGolden() invocations
+  ///
+  /// [deviceFileNameFactory] a func used to decide the final filename for multiScreenGolden() invocations
+  ///
+  /// [primeAssets] a func that is used to ensure that all images have been decoded before trying to render
   const GoldenToolkitConfiguration({
     this.skipGoldenAssertion = _doNotSkip,
     this.fileNameFactory = defaultFileNameFactory,
     this.deviceFileNameFactory = defaultDeviceFileNameFactory,
+    this.primeAssets = defaultPrimeAssets,
   });
 
   /// a function indicating whether a golden assertion should be skipped
@@ -87,16 +103,21 @@ class GoldenToolkitConfiguration {
   /// A function to determine the file name/path [multiScreenGolden] uses to call [matchesGoldenFile].
   final DeviceFileNameFactory deviceFileNameFactory;
 
+  /// A function that primes all needed assets for the given [tester]. Defaults to [defaultPrimeAssets].
+  final PrimeAssets primeAssets;
+
   /// Copies the configuration with the given values overridden.
   GoldenToolkitConfiguration copyWith({
     SkipGoldenAssertion skipGoldenAssertion,
     FileNameFactory fileNameFactory,
     DeviceFileNameFactory deviceFileNameFactory,
+    PrimeAssets primeAssets,
   }) {
     return GoldenToolkitConfiguration(
       skipGoldenAssertion: skipGoldenAssertion ?? this.skipGoldenAssertion,
       fileNameFactory: fileNameFactory ?? this.fileNameFactory,
       deviceFileNameFactory: deviceFileNameFactory ?? this.deviceFileNameFactory,
+      primeAssets: primeAssets ?? this.primeAssets,
     );
   }
 
@@ -107,11 +128,13 @@ class GoldenToolkitConfiguration {
             runtimeType == other.runtimeType &&
             skipGoldenAssertion == other.skipGoldenAssertion &&
             fileNameFactory == other.fileNameFactory &&
-            deviceFileNameFactory == other.deviceFileNameFactory;
+            deviceFileNameFactory == other.deviceFileNameFactory &&
+            primeAssets == other.primeAssets;
   }
 
   @override
-  int get hashCode => skipGoldenAssertion.hashCode ^ fileNameFactory.hashCode ^ deviceFileNameFactory.hashCode;
+  int get hashCode =>
+      skipGoldenAssertion.hashCode ^ fileNameFactory.hashCode ^ deviceFileNameFactory.hashCode ^ primeAssets.hashCode;
 }
 
 bool _doNotSkip() => false;

packages/golden_toolkit/lib/src/multi_screen_golden.dart

@@ -11,7 +11,7 @@
 import 'package:flutter/material.dart';
 import 'package:flutter_test/flutter_test.dart';
 
-import 'configuration.dart';
+import '../golden_toolkit.dart';
 import 'device.dart';
 import 'testing_tools.dart';
 import 'widget_tester_extensions.dart';
@@ -75,9 +75,9 @@ Future<void> multiScreenGolden(
         await compareWithGolden(
           tester,
           name,
+          customPump: customPump,
           autoHeight: autoHeight,
           finder: finder,
-          customPump: customPump,
           //ignore: deprecated_member_use_from_same_package
           skip: skip,
           device: device,

packages/golden_toolkit/lib/src/testing_tools.dart

@@ -9,14 +9,17 @@
 //ignore_for_file: deprecated_member_use_from_same_package
 
 import 'dart:async';
+import 'dart:ui';
 
 import 'package:flutter/material.dart';
+import 'package:flutter/rendering.dart';
 import 'package:flutter_test/flutter_test.dart';
 import 'package:meta/meta.dart';
 
 import 'configuration.dart';
 import 'device.dart';
 import 'test_asset_bundle.dart';
+import 'widget_tester_extensions.dart';
 
 const Size _defaultSize = Size(800, 600);
 
@@ -201,10 +204,10 @@ Future<void> compareWithGolden(
   final fileName = fileNameFactory(name, device);
   final originalWindowSize = tester.binding.window.physicalSize;
 
-  // This is a minor optimization and works around an issue with the current hacky implementation of invoking the golden assertion method.
   if (!shouldSkipGoldenGeneration) {
-    await _primeImages(fileName, actualFinder);
+    await tester.waitForAssets();
   }
+
   await pumpAfterPrime(tester);
 
   if (autoHeight == true) {
@@ -249,7 +252,54 @@ Future<void> compareWithGolden(
   }
 }
 
-// Matches Golden file is the easiest way for the images to be requested.
-Future<void> _primeImages(String fileName, Finder finder) => matchesGoldenFile(fileName).matchAsync(finder);
+/// A function that primes all assets by just wasting time and hoping that it is enough for all assets to
+/// finish loading. Doing so is not recommended and very flaky. Consider switching to [defaultPrimeAssets] or
+/// a custom implementation.
+///
+/// See also:
+/// * [GoldenToolkitConfiguration.primeAssets] to configure a global asset prime function.
+Future<void> legacyPrimeAssets(WidgetTester tester) async {
+  final renderObject = tester.binding.renderView;
+  assert(!renderObject.debugNeedsPaint);
+
+  final OffsetLayer layer = renderObject.debugLayer;
+
+  // This is a very flaky hack which should be avoided if possible.
+  // We are just trying to waste some time that matches the time needed to call matchesGoldenFile.
+  // This should be enough time for most images/assets to be ready.
+  await tester.runAsync<void>(() async {
+    final image = await layer.toImage(renderObject.paintBounds);
+    await image.toByteData(format: ImageByteFormat.png);
+    await image.toByteData(format: ImageByteFormat.png);
+  });
+}
+
+/// A function that waits for all [Image] widgets found in the widget tree to finish decoding.
+///
+/// Currently this supports images included via Image widgets, or as part of BoxDecorations.
+///
+/// See also:
+/// * [GoldenToolkitConfiguration.primeAssets] to configure a global asset prime function.
+Future<void> defaultPrimeAssets(WidgetTester tester) async {
+  final imageElements = find.byType(Image).evaluate();
+  final containerElements = find.byType(Container).evaluate();
+  await tester.runAsync(() async {
+    for (final imageElement in imageElements) {
+      final widget = imageElement.widget;
+      if (widget is Image) {
+        await precacheImage(widget.image, imageElement);
+      }
+    }
+    for (final container in containerElements) {
+      final Container widget = container.widget;
+      final decoration = widget.decoration;
+      if (decoration is BoxDecoration) {
+        if (decoration.image != null) {
+          await precacheImage(decoration.image.image, container);
+        }
+      }
+    }
+  });
+}
 
 Future<void> _onlyPumpAndSettle(WidgetTester tester) async => tester.pumpAndSettle();

packages/golden_toolkit/lib/src/widget_tester_extensions.dart

@@ -3,10 +3,20 @@ import 'dart:ui';
 import 'package:flutter/widgets.dart';
 import 'package:flutter_test/flutter_test.dart';
 
+import 'configuration.dart';
 import 'device.dart';
 
+/// Convenience extensions on WidgetTester
+extension WidgetTesterImageLoadingExtensions on WidgetTester {
+  /// Waits for images to decode. Use this to ensure that images are properly displayed
+  /// in Goldens. The implementation of this can be configured as part of GoldenToolkitConfiguration
+  ///
+  /// If you have assets that are not loading with this implementation, please file an issue and we will explore solutions.
+  Future<void> waitForAssets() => GoldenToolkit.configuration.primeAssets(this);
+}
+
 /// Convenience extensions for more easily configuring WidgetTester for pre-set configurations
-extension WidgetTesterExtensions on TestWidgetsFlutterBinding {
+extension WidgetFlutterBindingExtensions on TestWidgetsFlutterBinding {
   /// Configure the Test device for the duration of the supplied operation and revert
   ///
   /// [device] the desired configuration to apply

packages/golden_toolkit/test/configuration_test.dart

@@ -76,6 +76,40 @@ void main() {
       );
     });
 
+    testGoldens('screenMatchesGolden method uses primeAssets from global configuration', (tester) async {
+      var globalPrimeCalledCount = 0;
+      return GoldenToolkit.runWithConfiguration(
+        () async {
+          await tester.pumpWidgetBuilder(Image.asset('packages/sample_dependency/images/image.png'));
+          await screenMatchesGolden(tester, 'screen_matches_golden_defers_primeAssets');
+          expect(globalPrimeCalledCount, 1);
+        },
+        config: GoldenToolkitConfiguration(primeAssets: (WidgetTester tester) async {
+          globalPrimeCalledCount += 1;
+          await legacyPrimeAssets(tester);
+        }),
+      );
+    });
+
+    testGoldens('multiScreenGolden method uses primeAssets from global configuration', (tester) async {
+      var globalPrimeCalledCount = 0;
+      return GoldenToolkit.runWithConfiguration(
+        () async {
+          await tester.pumpWidgetBuilder(Image.asset('packages/sample_dependency/images/image.png'));
+          await multiScreenGolden(
+            tester,
+            'multi_screen_golden_defers_primeAssets',
+            devices: [const Device(size: Size(200, 200), name: 'custom')],
+          );
+          expect(globalPrimeCalledCount, 1);
+        },
+        config: GoldenToolkitConfiguration(primeAssets: (WidgetTester tester) async {
+          globalPrimeCalledCount += 1;
+          await legacyPrimeAssets(tester);
+        }),
+      );
+    });
+
     test('Default Configuration', () {
       const config = GoldenToolkitConfiguration();
       expect(config.skipGoldenAssertion(), isFalse);
@@ -90,11 +124,13 @@ void main() {
       bool skipGoldenAssertion() => false;
       String fileNameFactory(String filename) => '';
       String deviceFileNameFactory(String filename, Device device) => '';
+      Future<void> primeAssets(WidgetTester tester) async {}
 
       final config = GoldenToolkitConfiguration(
         skipGoldenAssertion: skipGoldenAssertion,
         deviceFileNameFactory: deviceFileNameFactory,
         fileNameFactory: fileNameFactory,
+        primeAssets: primeAssets,
       );
 
       test('config with identical params should be equal', () {
@@ -115,6 +151,10 @@ void main() {
           expect(config, isNot(equals(config.copyWith(deviceFileNameFactory: (file, dev) => ''))));
           expect(config.hashCode, isNot(equals(config.copyWith(deviceFileNameFactory: (file, dev) => '').hashCode)));
         });
+        test('primeImages', () {
+          expect(config, isNot(equals(config.copyWith(primeAssets: (_) async {}))));
+          expect(config.hashCode, isNot(equals(config.copyWith(primeAssets: (_) async {}).hashCode)));
+        });
       });
     });
   });