diff --git a/bin/internal/goldens.version b/bin/internal/goldens.version index 516229d6e0..4bea394583 100644 --- a/bin/internal/goldens.version +++ b/bin/internal/goldens.version @@ -1 +1 @@ -0ea80e6a0147f1a3a59ff57f460a3f038a0d2748 +e3f3b6766b18e2461c89a371be6e30045d8e404f diff --git a/packages/flutter/lib/painting.dart b/packages/flutter/lib/painting.dart index 1296f09b71..b673f6cf13 100644 --- a/packages/flutter/lib/painting.dart +++ b/packages/flutter/lib/painting.dart @@ -29,6 +29,7 @@ export 'src/painting/box_fit.dart'; export 'src/painting/box_shadow.dart'; export 'src/painting/circle_border.dart'; export 'src/painting/colors.dart'; +export 'src/painting/debug.dart'; export 'src/painting/decoration.dart'; export 'src/painting/decoration_image.dart'; export 'src/painting/edge_insets.dart'; diff --git a/packages/flutter/lib/src/material/mergeable_material.dart b/packages/flutter/lib/src/material/mergeable_material.dart index fccea2b344..3321cbe7bc 100644 --- a/packages/flutter/lib/src/material/mergeable_material.dart +++ b/packages/flutter/lib/src/material/mergeable_material.dart @@ -686,9 +686,7 @@ class _RenderMergeableMaterialListBody extends RenderListBody { void _paintShadows(Canvas canvas, Rect rect) { for (BoxShadow boxShadow in boxShadows) { - final Paint paint = new Paint() - ..color = boxShadow.color - ..maskFilter = new MaskFilter.blur(BlurStyle.normal, boxShadow.blurSigma); + final Paint paint = boxShadow.toPaint(); // TODO(dragostis): Right now, we are only interpolating the border radii // of the visible Material slices, not the shadows; they are not getting // interpolated and always have the same rounded radii. Once shadow diff --git a/packages/flutter/lib/src/painting/box_decoration.dart b/packages/flutter/lib/src/painting/box_decoration.dart index 12439d5d0b..7fcc050e80 100644 --- a/packages/flutter/lib/src/painting/box_decoration.dart +++ b/packages/flutter/lib/src/painting/box_decoration.dart @@ -366,9 +366,7 @@ class _BoxDecorationPainter extends BoxPainter { if (_decoration.boxShadow == null) return; for (BoxShadow boxShadow in _decoration.boxShadow) { - final Paint paint = new Paint() - ..color = boxShadow.color - ..maskFilter = new MaskFilter.blur(BlurStyle.normal, boxShadow.blurSigma); + final Paint paint = boxShadow.toPaint(); final Rect bounds = rect.shift(boxShadow.offset).inflate(boxShadow.spreadRadius); _paintBox(canvas, bounds, paint, textDirection); } diff --git a/packages/flutter/lib/src/painting/box_shadow.dart b/packages/flutter/lib/src/painting/box_shadow.dart index 2ef2407ec2..1387890331 100644 --- a/packages/flutter/lib/src/painting/box_shadow.dart +++ b/packages/flutter/lib/src/painting/box_shadow.dart @@ -8,13 +8,18 @@ import 'dart:ui' as ui show lerpDouble; import 'package:flutter/foundation.dart'; import 'basic_types.dart'; +import 'debug.dart'; /// A shadow cast by a box. /// -/// BoxShadow can cast non-rectangular shadows if the box is non-rectangular +/// [BoxShadow] can cast non-rectangular shadows if the box is non-rectangular /// (e.g., has a border radius or a circular shape). /// /// This class is similar to CSS box-shadow. +/// +/// See also: +/// +/// * [Canvas.drawShadow], which is a more efficient way to draw shadows. @immutable class BoxShadow { /// Creates a box shadow. @@ -55,6 +60,24 @@ class BoxShadow { /// See the sigma argument to [MaskFilter.blur]. double get blurSigma => convertRadiusToSigma(blurRadius); + /// Create the [Paint] object that corresponds to this shadow description. + /// + /// The [offset] and [spreadRadius] are not represented in the [Paint] object. + /// To honor those as well, the shape should be inflated by [spreadRadius] pixels + /// in every direction and then translated by [offset] before being filled using + /// this [Paint]. + Paint toPaint() { + final Paint result = new Paint() + ..color = color + ..maskFilter = new MaskFilter.blur(BlurStyle.normal, blurSigma); + assert(() { + if (debugDisableShadows) + result.maskFilter = null; + return true; + }()); + return result; + } + /// Returns a new box shadow with its offset, blurRadius, and spreadRadius scaled by the given factor. BoxShadow scale(double factor) { return new BoxShadow( diff --git a/packages/flutter/lib/src/painting/debug.dart b/packages/flutter/lib/src/painting/debug.dart new file mode 100644 index 0000000000..1951a6eb63 --- /dev/null +++ b/packages/flutter/lib/src/painting/debug.dart @@ -0,0 +1,33 @@ +// Copyright 2018 The Chromium 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 'package:flutter/foundation.dart'; + +/// Whether to replace all shadows with solid color blocks. +/// +/// This is useful when writing golden file tests (see [matchesGoldenFile]) since +/// the rendering of shadows is not guaranteed to be pixel-for-pixel identical from +/// version to version (or even from run to run). +bool debugDisableShadows = false; + +/// Returns true if none of the painting library debug variables have been changed. +/// +/// This function is used by the test framework to ensure that debug variables +/// haven't been inadvertently changed. +/// +/// See for +/// a complete list. +/// +/// The `debugDisableShadowsOverride` argument can be provided to override +/// the expected value for [debugDisableShadows]. (This exists because the +/// test framework itself overrides this value in some cases.) +bool debugAssertAllPaintingVarsUnset(String reason, { bool debugDisableShadowsOverride: false }) { + assert(() { + if (debugDisableShadows != debugDisableShadowsOverride) { + throw new FlutterError(reason); + } + return true; + }()); + return true; +} diff --git a/packages/flutter/lib/src/painting/shape_decoration.dart b/packages/flutter/lib/src/painting/shape_decoration.dart index 18d6e1689c..433a88f8c2 100644 --- a/packages/flutter/lib/src/painting/shape_decoration.dart +++ b/packages/flutter/lib/src/painting/shape_decoration.dart @@ -328,12 +328,8 @@ class _ShapeDecorationPainter extends BoxPainter { _shadowCount = _decoration.shadows.length; _shadowPaths = new List(_shadowCount); _shadowPaints = new List(_shadowCount); - for (int index = 0; index < _shadowCount; index += 1) { - final BoxShadow shadow = _decoration.shadows[index]; - _shadowPaints[index] = new Paint() - ..color = shadow.color - ..maskFilter = new MaskFilter.blur(BlurStyle.normal, shadow.blurSigma); - } + for (int index = 0; index < _shadowCount; index += 1) + _shadowPaints[index] = _decoration.shadows[index].toPaint(); } for (int index = 0; index < _shadowCount; index += 1) { final BoxShadow shadow = _decoration.shadows[index]; diff --git a/packages/flutter/lib/src/rendering/layer.dart b/packages/flutter/lib/src/rendering/layer.dart index aad6951793..6f183ce22b 100644 --- a/packages/flutter/lib/src/rendering/layer.dart +++ b/packages/flutter/lib/src/rendering/layer.dart @@ -818,6 +818,12 @@ class PhysicalModelLayer extends ContainerLayer { /// /// The scene must be explicitly recomposited after this property is changed /// (as described at [Layer]). + /// + /// In tests, the [debugDisableShadows] flag is set to true by default. + /// Several widgets and render objects force all elevations to zero when this + /// flag is set. For this reason, this property will often be set to zero in + /// tests even if the layer should be raised. To verify the actual value, + /// consider setting [debugDisableShadows] to false in your test. double elevation; /// The background color. diff --git a/packages/flutter/lib/src/rendering/proxy_box.dart b/packages/flutter/lib/src/rendering/proxy_box.dart index bf51370fa9..f46275d06f 100644 --- a/packages/flutter/lib/src/rendering/proxy_box.dart +++ b/packages/flutter/lib/src/rendering/proxy_box.dart @@ -1449,6 +1449,9 @@ abstract class _RenderPhysicalModelBase extends _RenderCustomClip { super(child: child, clipper: clipper); /// The z-coordinate at which to place this material. + /// + /// If [debugDisableShadows] is set, this value is ignored and no shadow is + /// drawn (an outline is rendered instead). double get elevation => _elevation; double _elevation; set elevation(double value) { @@ -1591,20 +1594,34 @@ class RenderPhysicalModel extends _RenderPhysicalModelBase { void paint(PaintingContext context, Offset offset) { if (child != null) { _updateClip(); - final RRect offsetClipRRect = _clip.shift(offset); - final Rect offsetBounds = offsetClipRRect.outerRect; - final Path offsetClipPath = new Path()..addRRect(offsetClipRRect); + final RRect offsetRRect = _clip.shift(offset); + final Rect offsetBounds = offsetRRect.outerRect; + final Path offsetRRectAsPath = new Path()..addRRect(offsetRRect); + bool paintShadows = true; + assert(() { + if (debugDisableShadows) { + context.canvas.drawRRect( + offsetRRect, + new Paint() + ..color = shadowColor + ..style = PaintingStyle.stroke + ..strokeWidth = elevation * 2.0, + ); + paintShadows = false; + } + return true; + }()); if (needsCompositing) { final PhysicalModelLayer physicalModel = new PhysicalModelLayer( - clipPath: offsetClipPath, - elevation: elevation, + clipPath: offsetRRectAsPath, + elevation: paintShadows ? elevation : 0.0, color: color, shadowColor: shadowColor, ); context.pushLayer(physicalModel, super.paint, offset, childPaintBounds: offsetBounds); } else { final Canvas canvas = context.canvas; - if (elevation != 0.0) { + if (elevation != 0.0 && paintShadows) { // The drawShadow call doesn't add the region of the shadow to the // picture's bounds, so we draw a hardcoded amount of extra space to // account for the maximum potential area of the shadow. @@ -1614,25 +1631,25 @@ class RenderPhysicalModel extends _RenderPhysicalModelBase { _RenderPhysicalModelBase._transparentPaint, ); canvas.drawShadow( - offsetClipPath, + offsetRRectAsPath, shadowColor, elevation, color.alpha != 0xFF, ); } - canvas.drawRRect(offsetClipRRect, new Paint()..color = color); + canvas.drawRRect(offsetRRect, new Paint()..color = color); canvas.save(); - canvas.clipRRect(offsetClipRRect); + canvas.clipRRect(offsetRRect); // We only use a new layer for non-rectangular clips, on the basis that // rectangular clips won't need antialiasing. This is not really // correct, because if we're e.g. rotated, rectangles will also be // aliased. Unfortunately, it's too much of a performance win to err on // the side of correctness here. // TODO(ianh): Find a better solution. - if (!offsetClipRRect.isRect) + if (!offsetRRect.isRect) canvas.saveLayer(offsetBounds, _RenderPhysicalModelBase._defaultPaint); super.paint(context, offset); - if (!offsetClipRRect.isRect) + if (!offsetRRect.isRect) canvas.restore(); canvas.restore(); assert(context.canvas == canvas, 'canvas changed even though needsCompositing was false'); @@ -1701,17 +1718,31 @@ class RenderPhysicalShape extends _RenderPhysicalModelBase { _updateClip(); final Rect offsetBounds = offset & size; final Path offsetPath = _clip.shift(offset); + bool paintShadows = true; + assert(() { + if (debugDisableShadows) { + context.canvas.drawPath( + offsetPath, + new Paint() + ..color = shadowColor + ..style = PaintingStyle.stroke + ..strokeWidth = elevation * 2.0, + ); + paintShadows = false; + } + return true; + }()); if (needsCompositing) { final PhysicalModelLayer physicalModel = new PhysicalModelLayer( clipPath: offsetPath, - elevation: elevation, + elevation: paintShadows ? elevation : 0.0, color: color, shadowColor: shadowColor, ); context.pushLayer(physicalModel, super.paint, offset, childPaintBounds: offsetBounds); } else { final Canvas canvas = context.canvas; - if (elevation != 0.0) { + if (elevation != 0.0 && paintShadows) { // The drawShadow call doesn't add the region of the shadow to the // picture's bounds, so we draw a hardcoded amount of extra space to // account for the maximum potential area of the shadow. diff --git a/packages/flutter/lib/src/widgets/banner.dart b/packages/flutter/lib/src/widgets/banner.dart index c2efa4c19d..30fbc27a1a 100644 --- a/packages/flutter/lib/src/widgets/banner.dart +++ b/packages/flutter/lib/src/widgets/banner.dart @@ -5,6 +5,7 @@ import 'dart:math' as math; import 'package:flutter/foundation.dart'; +import 'package:flutter/painting.dart'; import 'basic.dart'; import 'debug.dart'; @@ -107,15 +108,18 @@ class BannerPainter extends CustomPainter { /// Defaults to bold, white text. final TextStyle textStyle; + static const BoxShadow _shadow = const BoxShadow( + color: const Color(0x7F000000), + blurRadius: 4.0, + ); + bool _prepared = false; TextPainter _textPainter; Paint _paintShadow; Paint _paintBanner; void _prepare() { - _paintShadow = new Paint() - ..color = const Color(0x7F000000) - ..maskFilter = const MaskFilter.blur(BlurStyle.normal, 4.0); + _paintShadow = _shadow.toPaint(); _paintBanner = new Paint() ..color = color; _textPainter = new TextPainter( diff --git a/packages/flutter/lib/src/widgets/basic.dart b/packages/flutter/lib/src/widgets/basic.dart index 0e124d450e..8e71298974 100644 --- a/packages/flutter/lib/src/widgets/basic.dart +++ b/packages/flutter/lib/src/widgets/basic.dart @@ -52,6 +52,7 @@ export 'package:flutter/rendering.dart' show RelativeRect, SemanticsBuilderCallback, ShaderCallback, + ShapeBorderClipper, SingleChildLayoutDelegate, StackFit, TextOverflow, @@ -731,6 +732,11 @@ class PhysicalModel extends SingleChildRenderObjectWidget { /// /// [PhysicalModel] does the same but only supports shapes that can be expressed /// as rectangles with rounded corners. +/// +/// See also: +/// +/// * [ShapeBorderClipper], which converts a [ShapeBorder] to a [CustomerClipper], as +/// needed by this widget. class PhysicalShape extends SingleChildRenderObjectWidget { /// Creates a physical model with an arbitrary shape clip. /// @@ -751,6 +757,10 @@ class PhysicalShape extends SingleChildRenderObjectWidget { super(key: key, child: child); /// Determines which clip to use. + /// + /// If the path in question is expressed as a [ShapeBorder] subclass, + /// consider using the [ShapeBorderClipper] delegate class to adapt the + /// shape for use with this widget. final CustomClipper clipper; /// The z-coordinate at which to place this physical object. diff --git a/packages/flutter/test/material/mergeable_material_test.dart b/packages/flutter/test/material/mergeable_material_test.dart index 74a5a4fe94..6b6d3d0b63 100644 --- a/packages/flutter/test/material/mergeable_material_test.dart +++ b/packages/flutter/test/material/mergeable_material_test.dart @@ -198,6 +198,7 @@ void main() { }); testWidgets('MergeableMaterial paints shadows', (WidgetTester tester) async { + debugDisableShadows = false; await tester.pumpWidget( new MaterialApp( home: new Scaffold( @@ -226,6 +227,7 @@ void main() { find.byType(MergeableMaterial), paints..rrect(rrect: rrect, color: boxShadow.color, hasMaskFilter: true), ); + debugDisableShadows = true; }); testWidgets('MergeableMaterial merge gap', (WidgetTester tester) async { diff --git a/packages/flutter/test/material/outline_button_test.dart b/packages/flutter/test/material/outline_button_test.dart index 1c1744a776..b00198401b 100644 --- a/packages/flutter/test/material/outline_button_test.dart +++ b/packages/flutter/test/material/outline_button_test.dart @@ -45,6 +45,7 @@ void main() { testWidgets('Outline shape and border overrides', (WidgetTester tester) async { + debugDisableShadows = false; const Color fillColor = const Color(0xFF00FF00); const Color borderColor = const Color(0xFFFF0000); const Color highlightedBorderColor = const Color(0xFF0000FF); @@ -111,6 +112,7 @@ void main() { ..clipPath(pathMatcher: coversSameAreaAs(clipPath, areaToCompare: clipRect.inflate(10.0))) ..path(color: borderColor, strokeWidth: borderWidth) ); + debugDisableShadows = true; }); diff --git a/packages/flutter/test/rendering/mock_canvas.dart b/packages/flutter/test/rendering/mock_canvas.dart index 93073182ef..50fd6564b6 100644 --- a/packages/flutter/test/rendering/mock_canvas.dart +++ b/packages/flutter/test/rendering/mock_canvas.dart @@ -325,6 +325,10 @@ abstract class PaintPattern { /// are compared to the actual [Canvas.drawShadow] call's `paint` argument, /// and any mismatches result in failure. /// + /// In tests, shadows from framework features such as [BoxShadow] or + /// [Material] are disabled by default, and thus this predicate would not + /// match. The [debugDisableShadows] flag controls this. + /// /// To introspect the Path object (as it stands after the painting has /// completed), the `includes` and `excludes` arguments can be provided to /// specify points that should be considered inside or outside the path diff --git a/packages/flutter/test/service_extensions_test_file b/packages/flutter/test/service_extensions_test_file deleted file mode 100644 index 40b450dd9d..0000000000 Binary files a/packages/flutter/test/service_extensions_test_file and /dev/null differ diff --git a/packages/flutter/test/widgets/banner_test.dart b/packages/flutter/test/widgets/banner_test.dart index c89dac3289..b527d8e0e3 100644 --- a/packages/flutter/test/widgets/banner_test.dart +++ b/packages/flutter/test/widgets/banner_test.dart @@ -248,6 +248,7 @@ void main() { }); testWidgets('Banner widget', (WidgetTester tester) async { + debugDisableShadows = false; await tester.pumpWidget( const Directionality( textDirection: TextDirection.ltr, @@ -263,9 +264,11 @@ void main() { ..paragraph(offset: const Offset(-40.0, 29.0)) ..restore() ); + debugDisableShadows = true; }); testWidgets('Banner widget in MaterialApp', (WidgetTester tester) async { + debugDisableShadows = false; await tester.pumpWidget(new MaterialApp(home: const Placeholder())); expect(find.byType(CheckedModeBanner), paints ..save() @@ -276,5 +279,6 @@ void main() { ..paragraph(offset: const Offset(-40.0, 29.0)) ..restore() ); + debugDisableShadows = true; }); } diff --git a/packages/flutter/test/widgets/nested_scroll_view_test.dart b/packages/flutter/test/widgets/nested_scroll_view_test.dart index a08e74275c..956a1e66d3 100644 --- a/packages/flutter/test/widgets/nested_scroll_view_test.dart +++ b/packages/flutter/test/widgets/nested_scroll_view_test.dart @@ -344,6 +344,7 @@ void main() { }); testWidgets('NestedScrollView and internal scrolling', (WidgetTester tester) async { + debugDisableShadows = false; const List _tabs = const ['Hello', 'World']; int buildCount = 0; await tester.pumpWidget( @@ -565,6 +566,7 @@ void main() { await tester.pumpAndSettle(); expect(buildCount, expectedBuildCount); expect(find.byType(NestedScrollView), isNot(paints..shadow())); + debugDisableShadows = true; }); testWidgets('NestedScrollView and iOS bouncing', (WidgetTester tester) async { diff --git a/packages/flutter/test/widgets/physical_model_test.dart b/packages/flutter/test/widgets/physical_model_test.dart index 7578ac7f8f..89cfc80253 100644 --- a/packages/flutter/test/widgets/physical_model_test.dart +++ b/packages/flutter/test/widgets/physical_model_test.dart @@ -5,6 +5,7 @@ import 'package:flutter_test/flutter_test.dart'; void main() { testWidgets('PhysicalModel - creates a physical model layer when it needs compositing', (WidgetTester tester) async { + debugDisableShadows = false; await tester.pumpWidget(new Directionality( textDirection: TextDirection.ltr, child: new PhysicalModel( @@ -25,5 +26,6 @@ void main() { expect(physicalModelLayer.shadowColor, Colors.red); expect(physicalModelLayer.color, Colors.grey); expect(physicalModelLayer.elevation, 1.0); + debugDisableShadows = true; }); } diff --git a/packages/flutter/test/widgets/shadow_test.dart b/packages/flutter/test/widgets/shadow_test.dart new file mode 100644 index 0000000000..29dfa802f0 --- /dev/null +++ b/packages/flutter/test/widgets/shadow_test.dart @@ -0,0 +1,136 @@ +// Copyright 2018 The Chromium 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:io' show Platform; + +import 'package:flutter_test/flutter_test.dart'; +import 'package:flutter/material.dart'; + +void main() { + testWidgets('Shadows on BoxDecoration', (WidgetTester tester) async { + await tester.pumpWidget( + new Center( + child: new RepaintBoundary( + child: new Container( + margin: const EdgeInsets.all(50.0), + decoration: new BoxDecoration( + boxShadow: kElevationToShadow[9], + ), + height: 100.0, + width: 100.0, + ), + ), + ), + ); + await expectLater( + find.byType(Container), + matchesGoldenFile('shadow.BoxDecoration.disabled.png'), + ); + debugDisableShadows = false; + tester.binding.reassembleApplication(); + await tester.pump(); + if (Platform.isLinux) { + // TODO(ianh): use the skip argument instead once that doesn't hang, https://github.com/dart-lang/test/issues/830 + await expectLater( + find.byType(Container), + matchesGoldenFile('shadow.BoxDecoration.enabled.png'), + ); // shadows render differently on different platforms + } + debugDisableShadows = true; + }); + + testWidgets('Shadows on ShapeDecoration', (WidgetTester tester) async { + debugDisableShadows = false; + Widget build(int elevation) { + return new Center( + child: new RepaintBoundary( + child: new Container( + margin: const EdgeInsets.all(150.0), + decoration: new ShapeDecoration( + shape: new BeveledRectangleBorder(borderRadius: new BorderRadius.circular(20.0)), + shadows: kElevationToShadow[elevation], + ), + height: 100.0, + width: 100.0, + ), + ), + ); + } + for (int elevation in kElevationToShadow.keys) { + await tester.pumpWidget(build(elevation)); + await expectLater( + find.byType(Container), + matchesGoldenFile('shadow.ShapeDecoration.$elevation.png'), + ); + } + debugDisableShadows = true; + }, skip: !Platform.isLinux); // shadows render differently on different platforms + + testWidgets('Shadows with PhysicalLayer', (WidgetTester tester) async { + await tester.pumpWidget( + new Center( + child: new RepaintBoundary( + child: new Container( + margin: const EdgeInsets.all(150.0), + color: Colors.yellow[200], + child: new PhysicalModel( + elevation: 9.0, + color: Colors.blue[900], + child: const SizedBox( + height: 100.0, + width: 100.0, + ), + ), + ), + ), + ), + ); + await expectLater( + find.byType(Container), + matchesGoldenFile('shadow.PhysicalModel.disabled.png'), + ); + debugDisableShadows = false; + tester.binding.reassembleApplication(); + await tester.pump(); + if (Platform.isLinux) { + // TODO(ianh): use the skip argument instead once that doesn't hang, https://github.com/dart-lang/test/issues/830 + await expectLater( + find.byType(Container), + matchesGoldenFile('shadow.PhysicalModel.enabled.png'), + ); // shadows render differently on different platforms + } + debugDisableShadows = true; + }); + + testWidgets('Shadows with PhysicalShape', (WidgetTester tester) async { + debugDisableShadows = false; + Widget build(double elevation) { + return new Center( + child: new RepaintBoundary( + child: new Container( + padding: const EdgeInsets.all(150.0), + color: Colors.yellow[200], + child: new PhysicalShape( + color: Colors.green[900], + clipper: new ShapeBorderClipper(shape: new BeveledRectangleBorder(borderRadius: new BorderRadius.circular(20.0))), + elevation: elevation, + child: const SizedBox( + height: 100.0, + width: 100.0, + ), + ), + ), + ), + ); + } + for (int elevation in kElevationToShadow.keys) { + await tester.pumpWidget(build(elevation.toDouble())); + await expectLater( + find.byType(Container), + matchesGoldenFile('shadow.PhysicalShape.$elevation.png'), + ); + } + debugDisableShadows = true; + }, skip: !Platform.isLinux); // shadows render differently on different platforms +} diff --git a/packages/flutter_goldens/lib/flutter_goldens.dart b/packages/flutter_goldens/lib/flutter_goldens.dart index db5e70cbb1..9606d40b92 100644 --- a/packages/flutter_goldens/lib/flutter_goldens.dart +++ b/packages/flutter_goldens/lib/flutter_goldens.dart @@ -14,6 +14,10 @@ import 'package:meta/meta.dart'; import 'package:platform/platform.dart'; import 'package:process/process.dart'; +// If you are here trying to figure out how to use golden files in the Flutter +// repo itself, consider reading this wiki page: +// https://github.com/flutter/flutter/wiki/Writing-a-golden-file-test-for-package%3Aflutter + const String _kFlutterRootKey = 'FLUTTER_ROOT'; /// Main method that can be used in a `flutter_test_config.dart` file to set @@ -105,20 +109,42 @@ class FlutterGoldenFileComparator implements GoldenFileComparator { /// repository. @visibleForTesting class GoldensClient { + /// Create a handle to a local clone of the goldens repository. GoldensClient({ this.fs: const LocalFileSystem(), this.platform: const LocalPlatform(), this.process: const LocalProcessManager(), }); + /// The file system to use for storing the local clone of the repository. + /// + /// This is useful in tests, where a local file system (the default) can + /// be replaced by a memory file system. final FileSystem fs; + + /// A wrapper for the [dart:io.Platform] API. + /// + /// This is useful in tests, where the system platform (the default) can + /// be replaced by a mock platform instance. final Platform platform; + + /// A controller for launching subprocesses. + /// + /// This is useful in tests, where the real process manager (the default) + /// can be replaced by a mock process manager that doesn't really create + /// subprocesses. final ProcessManager process; RandomAccessFile _lock; + /// The local [Directory] where the Flutter repository is hosted. + /// + /// Uses the [fs] file system. Directory get flutterRoot => fs.directory(platform.environment[_kFlutterRootKey]); + /// The local [Directory] where the goldens repository is hosted. + /// + /// Uses the [fs] file system. Directory get repositoryRoot => flutterRoot.childDirectory(fs.path.join('bin', 'cache', 'pkg', 'goldens')); /// Prepares the local clone of the `flutter/goldens` repository for golden @@ -222,9 +248,17 @@ class GoldensClient { /// Exception that signals a process' exit with a non-zero exit code. class NonZeroExitCode implements Exception { + /// Create an exception that represents a non-zero exit code. + /// + /// The first argument must be non-zero. const NonZeroExitCode(this.exitCode, this.stderr) : assert(exitCode != 0); + /// The code that the process will signal to th eoperating system. + /// + /// By definiton, this is not zero. final int exitCode; + + /// The message to show on standard error. final String stderr; @override diff --git a/packages/flutter_test/lib/src/binding.dart b/packages/flutter_test/lib/src/binding.dart index 6e784783f2..733bf1d76a 100644 --- a/packages/flutter_test/lib/src/binding.dart +++ b/packages/flutter_test/lib/src/binding.dart @@ -96,6 +96,7 @@ abstract class TestWidgetsFlutterBinding extends BindingBase /// [debugPrintOverride], which can be overridden by subclasses. TestWidgetsFlutterBinding() { debugPrint = debugPrintOverride; + debugDisableShadows = disableShadows; debugCheckIntrinsicSizes = checkIntrinsicSizes; } @@ -108,6 +109,15 @@ abstract class TestWidgetsFlutterBinding extends BindingBase @protected DebugPrintCallback get debugPrintOverride => debugPrint; + /// The value to set [debugDisableShadows] to while tests are running. + /// + /// This can be used to reduce the likelihood of golden file tests being + /// flaky, because shadow rendering is not always deterministic. The + /// [AutomatedTestWidgetsFlutterBinding] sets this to true, so that all tests + /// always run with shadows disabled. + @protected + bool get disableShadows => false; + /// The value to set [debugCheckIntrinsicSizes] to while tests are running. /// /// This can be used to enable additional checks. For example, @@ -525,6 +535,10 @@ abstract class TestWidgetsFlutterBinding extends BindingBase assert(debugAssertAllGesturesVarsUnset( 'The value of a gestures debug variable was changed by the test.', )); + assert(debugAssertAllPaintingVarsUnset( + 'The value of a painting debug variable was changed by the test.', + debugDisableShadowsOverride: disableShadows, + )); assert(debugAssertAllRenderVarsUnset( 'The value of a rendering debug variable was changed by the test.', debugCheckIntrinsicSizesOverride: checkIntrinsicSizes, @@ -588,6 +602,9 @@ class AutomatedTestWidgetsFlutterBinding extends TestWidgetsFlutterBinding { @override DebugPrintCallback get debugPrintOverride => debugPrintSynchronously; + @override + bool get disableShadows => true; + @override bool get checkIntrinsicSizes => true; diff --git a/packages/flutter_test/lib/src/goldens.dart b/packages/flutter_test/lib/src/goldens.dart index 73cc5c9923..6061d1fe9a 100644 --- a/packages/flutter_test/lib/src/goldens.dart +++ b/packages/flutter_test/lib/src/goldens.dart @@ -51,11 +51,19 @@ abstract class GoldenFileComparator { /// /// This comparator is used as the backend for [matchesGoldenFile]. /// -/// The default comparator, [LocalFileComparator], will treat the golden key as +/// When using `flutter test`, a comparator implemented by [LocalFileComparator] +/// is used if no other comparator is specified. It treats the golden key as /// a relative path from the test file's directory. It will then load the /// golden file's bytes from disk and perform a byte-for-byte comparison of the /// encoded PNGs, returning true only if there's an exact match. /// +/// When using `flutter test --update-goldens`, the [LocalFileComparator] +/// updates the files on disk to match the rendering. +/// +/// When using `flutter run`, the default comparator (null) is used. It prints +/// a message to the console but otherwise does nothing. This allows tests to +/// be developed visually on a real device. +/// /// Callers may choose to override the default comparator by setting this to a /// custom comparator during test set-up (or using directory-level test /// configuration). For example, some projects may wish to install a more @@ -119,7 +127,7 @@ class _UninitializedComparator implements GoldenFileComparator { } } -/// The default [GoldenFileComparator] implementation. +/// The default [GoldenFileComparator] implementation for `flutter test`. /// /// This comparator loads golden files from the local file system, treating the /// golden key as a relative path from the test file's directory. @@ -128,13 +136,16 @@ class _UninitializedComparator implements GoldenFileComparator { /// comparison of the encoded PNGs, returning true only if there's an exact /// match. This means it will fail the test if two PNGs represent the same /// pixels but are encoded differently. +/// +/// When using `flutter test --update-goldens`, [LocalFileComparator] +/// updates the files on disk to match the rendering. class LocalFileComparator implements GoldenFileComparator { /// Creates a new [LocalFileComparator] for the specified [testFile]. /// /// Golden file keys will be interpreted as file paths relative to the /// directory in which [testFile] resides. /// - /// The [testFile] URI must represent a file. + /// The [testFile] URL must represent a file. LocalFileComparator(Uri testFile, {path.Style pathStyle}) : basedir = _getBasedir(testFile, pathStyle), _path = _getPath(pathStyle); diff --git a/packages/flutter_test/lib/src/widget_tester.dart b/packages/flutter_test/lib/src/widget_tester.dart index b6c5386bca..3185350d58 100644 --- a/packages/flutter_test/lib/src/widget_tester.dart +++ b/packages/flutter_test/lib/src/widget_tester.dart @@ -177,7 +177,8 @@ Future expectLater(dynamic actual, dynamic matcher, { // We can't wrap the delegate in a guard, or we'll hit async barriers in // [TestWidgetsFlutterBinding] while we're waiting for the matcher to complete TestAsyncUtils.guardSync(); - return test_package.expectLater(actual, matcher, reason: reason, skip: skip); + return test_package.expectLater(actual, matcher, reason: reason, skip: skip) + .then((dynamic value) => null); } /// Class that programmatically interacts with widgets and the test environment.