From d25e0eb0d3b43b15b97a522d20d60004b96d2f2e Mon Sep 17 00:00:00 2001 From: Ian Hickson Date: Fri, 8 Dec 2017 14:37:24 -0800 Subject: [PATCH] Fix tapping on a test in flutter run (#13397) Also: * Remove find.byIcon since it's identical to find.icon. (I sent mail to flutter-dev about this.) * Fix IconData's operator== and hashCode, which had not been updated when we added fields. * Add the byTooltip finder to the list of suggested finders. * Make the suggested Key finder prettier. --- .../catalog/test/app_bar_bottom_test.dart | 2 +- examples/catalog/test/basic_app_bar_test.dart | 4 +-- .../catalog/test/tabbed_app_bar_test.dart | 4 +-- .../flutter/lib/src/widgets/icon_data.dart | 9 +++-- .../material/paginated_data_table_test.dart | 6 ++-- packages/flutter/test/widgets/icon_test.dart | 12 +++++++ packages/flutter_test/lib/src/binding.dart | 7 ++-- packages/flutter_test/lib/src/finders.dart | 31 ++--------------- .../flutter_test/lib/src/widget_tester.dart | 34 ++++++++++++++----- 9 files changed, 57 insertions(+), 52 deletions(-) diff --git a/examples/catalog/test/app_bar_bottom_test.dart b/examples/catalog/test/app_bar_bottom_test.dart index 0b78fbc2ba7..4ebffd9869a 100644 --- a/examples/catalog/test/app_bar_bottom_test.dart +++ b/examples/catalog/test/app_bar_bottom_test.dart @@ -11,7 +11,7 @@ final int choiceCount = app_bar_bottom_sample.choices.length; IconData iconAt(int index) => app_bar_bottom_sample.choices[index].icon; Finder findChoiceCard(IconData icon) { - return find.descendant(of: find.byType(Card), matching: find.icon(icon)); + return find.descendant(of: find.byType(Card), matching: find.byIcon(icon)); } void main() { diff --git a/examples/catalog/test/basic_app_bar_test.dart b/examples/catalog/test/basic_app_bar_test.dart index b4f44d26bbb..f22d825a80f 100644 --- a/examples/catalog/test/basic_app_bar_test.dart +++ b/examples/catalog/test/basic_app_bar_test.dart @@ -12,11 +12,11 @@ IconData iconAt(int index) => basic_app_bar_sample.choices[index].icon; String titleAt(int index) => basic_app_bar_sample.choices[index].title; Finder findAppBarIcon(IconData icon) { - return find.descendant(of: find.byType(AppBar), matching: find.icon(icon)); + return find.descendant(of: find.byType(AppBar), matching: find.byIcon(icon)); } Finder findChoiceCard(IconData icon) { - return find.descendant(of: find.byType(Card), matching: find.icon(icon)); + return find.descendant(of: find.byType(Card), matching: find.byIcon(icon)); } void main() { diff --git a/examples/catalog/test/tabbed_app_bar_test.dart b/examples/catalog/test/tabbed_app_bar_test.dart index 2b1ef58e59b..fdb4e1cab8e 100644 --- a/examples/catalog/test/tabbed_app_bar_test.dart +++ b/examples/catalog/test/tabbed_app_bar_test.dart @@ -11,11 +11,11 @@ final int choiceCount = tabbed_app_bar_sample.choices.length; IconData iconAt(int index) => tabbed_app_bar_sample.choices[index].icon; Finder findChoiceCard(IconData icon) { - return find.descendant(of: find.byType(Card), matching: find.icon(icon)); + return find.descendant(of: find.byType(Card), matching: find.byIcon(icon)); } Finder findTab(IconData icon) { - return find.descendant(of: find.byType(Tab), matching: find.icon(icon)); + return find.descendant(of: find.byType(Tab), matching: find.byIcon(icon)); } void main() { diff --git a/packages/flutter/lib/src/widgets/icon_data.dart b/packages/flutter/lib/src/widgets/icon_data.dart index 35698c01381..fc837dbad72 100644 --- a/packages/flutter/lib/src/widgets/icon_data.dart +++ b/packages/flutter/lib/src/widgets/icon_data.dart @@ -2,6 +2,8 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +import 'dart:ui' show hashValues; + import 'package:flutter/foundation.dart'; /// A description of an icon fulfilled by a font glyph. @@ -52,11 +54,14 @@ class IconData { if (runtimeType != other.runtimeType) return false; final IconData typedOther = other; - return codePoint == typedOther.codePoint; + return codePoint == typedOther.codePoint + && fontFamily == typedOther.fontFamily + && fontPackage == typedOther.fontPackage + && matchTextDirection == typedOther.matchTextDirection; } @override - int get hashCode => codePoint.hashCode; + int get hashCode => hashValues(codePoint, fontFamily, fontPackage, matchTextDirection); @override String toString() => 'IconData(U+${codePoint.toRadixString(16).toUpperCase().padLeft(5, '0')})'; diff --git a/packages/flutter/test/material/paginated_data_table_test.dart b/packages/flutter/test/material/paginated_data_table_test.dart index 36fcc937f9d..8592801e1cb 100644 --- a/packages/flutter/test/material/paginated_data_table_test.dart +++ b/packages/flutter/test/material/paginated_data_table_test.dart @@ -80,7 +80,7 @@ void main() { expect(find.text('Eclair (0)'), findsOneWidget); expect(find.text('Gingerbread (0)'), findsNothing); - await tester.tap(find.icon(Icons.chevron_left)); + await tester.tap(find.byIcon(Icons.chevron_left)); expect(log, ['page-changed: 0']); log.clear(); @@ -91,7 +91,7 @@ void main() { expect(find.text('Eclair (0)'), findsNothing); expect(find.text('Gingerbread (0)'), findsNothing); - await tester.tap(find.icon(Icons.chevron_left)); + await tester.tap(find.byIcon(Icons.chevron_left)); expect(log, isEmpty); @@ -188,7 +188,7 @@ void main() { expect(find.text('Gingerbread (1)'), findsNothing); expect(find.text('Gingerbread (2)'), findsOneWidget); - await tester.tap(find.icon(Icons.adjust)); + await tester.tap(find.byIcon(Icons.adjust)); expect(log, ['action: adjust']); log.clear(); }); diff --git a/packages/flutter/test/widgets/icon_test.dart b/packages/flutter/test/widgets/icon_test.dart index e5d8ecf971a..3f88ef77381 100644 --- a/packages/flutter/test/widgets/icon_test.dart +++ b/packages/flutter/test/widgets/icon_test.dart @@ -192,4 +192,16 @@ void main() { // semanticLabel to make sure the subtree was not rebuilt. expect(richText2, same(richText1)); }); + + testWidgets('IconData comparison', (WidgetTester tester) async { + expect(const IconData(123), const IconData(123)); + expect(const IconData(123), isNot(const IconData(123, matchTextDirection: true))); + expect(const IconData(123), isNot(const IconData(123, fontFamily: 'f'))); + expect(const IconData(123), isNot(const IconData(123, fontPackage: 'p'))); + expect(const IconData(123).hashCode, const IconData(123).hashCode); + expect(const IconData(123).hashCode, isNot(const IconData(123, matchTextDirection: true).hashCode)); + expect(const IconData(123).hashCode, isNot(const IconData(123, fontFamily: 'f').hashCode)); + expect(const IconData(123).hashCode, isNot(const IconData(123, fontPackage: 'p').hashCode)); + expect(const IconData(123).toString(), 'IconData(U+0007B)'); + }); } diff --git a/packages/flutter_test/lib/src/binding.dart b/packages/flutter_test/lib/src/binding.dart index f70cf1a3fa9..df5571e129d 100644 --- a/packages/flutter_test/lib/src/binding.dart +++ b/packages/flutter_test/lib/src/binding.dart @@ -984,8 +984,9 @@ class TestViewConfiguration extends ViewConfiguration { super(size: size); static Matrix4 _getMatrix(Size size, double devicePixelRatio) { - final double actualWidth = ui.window.physicalSize.width; - final double actualHeight = ui.window.physicalSize.height; + final double inverseRatio = devicePixelRatio / ui.window.devicePixelRatio; + final double actualWidth = ui.window.physicalSize.width * inverseRatio; + final double actualHeight = ui.window.physicalSize.height * inverseRatio; final double desiredWidth = size.width; final double desiredHeight = size.height; double scale, shiftX, shiftY; @@ -1020,8 +1021,6 @@ class TestViewConfiguration extends ViewConfiguration { /// /// This is useful because pointers are described in logical pixels, as /// opposed to graphics which are expressed in physical pixels. - // TODO(ianh): We should make graphics and pointers use the same coordinate space. - // See: https://github.com/flutter/flutter/issues/1360 Matrix4 toHitTestMatrix() => _hitTestMatrix.clone(); @override diff --git a/packages/flutter_test/lib/src/finders.dart b/packages/flutter_test/lib/src/finders.dart index 776cc198d26..4e815513a51 100644 --- a/packages/flutter_test/lib/src/finders.dart +++ b/packages/flutter_test/lib/src/finders.dart @@ -34,17 +34,6 @@ class CommonFinders { /// nodes that are [Offstage] or that are from inactive [Route]s. Finder text(String text, { bool skipOffstage: true }) => new _TextFinder(text, skipOffstage: skipOffstage); - /// Finds [Icon] widgets containing icon data equal to the `icon` - /// argument. - /// - /// Example: - /// - /// expect(find.icon(Icons.chevron_left), findsOneWidget); - /// - /// If the `skipOffstage` argument is true (the default), then this skips - /// nodes that are [Offstage] or that are from inactive [Route]s. - Finder icon(IconData icon, { bool skipOffstage: true }) => new _IconFinder(icon, skipOffstage: skipOffstage); - /// Looks for widgets that contain a [Text] descendant with `text` /// in it. /// @@ -90,7 +79,8 @@ class CommonFinders { /// nodes that are [Offstage] or that are from inactive [Route]s. Finder byType(Type type, { bool skipOffstage: true }) => new _WidgetTypeFinder(type, skipOffstage: skipOffstage); - /// Finds widgets by searching for widgets with a particular icon data. + /// Finds [Icon] widgets containing icon data equal to the `icon` + /// argument. /// /// Example: /// @@ -421,23 +411,6 @@ class _TextFinder extends MatchFinder { } } -class _IconFinder extends MatchFinder { - _IconFinder(this.icon, { bool skipOffstage: true }) : super(skipOffstage: skipOffstage); - - final IconData icon; - - @override - String get description => 'icon "$icon"'; - - @override - bool matches(Element candidate) { - if (candidate.widget is! Icon) - return false; - final Icon iconWidget = candidate.widget; - return iconWidget.icon == icon; - } -} - class _WidgetWithTextFinder extends Finder { _WidgetWithTextFinder(this.widgetType, this.text, { bool skipOffstage: true }) : super(skipOffstage: skipOffstage); diff --git a/packages/flutter_test/lib/src/widget_tester.dart b/packages/flutter_test/lib/src/widget_tester.dart index ee7db2cb2ff..788d5f8e771 100644 --- a/packages/flutter_test/lib/src/widget_tester.dart +++ b/packages/flutter_test/lib/src/widget_tester.dart @@ -5,6 +5,7 @@ import 'dart:async'; import 'package:flutter/gestures.dart'; +import 'package:flutter/material.dart'; import 'package:flutter/rendering.dart'; import 'package:flutter/scheduler.dart'; import 'package:flutter/widgets.dart'; @@ -306,12 +307,18 @@ class WidgetTester extends WidgetController implements HitTestDispatcher, Ticker if (event is PointerDownEvent) { final RenderObject innerTarget = result.path.firstWhere( (HitTestEntry candidate) => candidate.target is RenderObject, - orElse: () => null - )?.target; - if (innerTarget == null) - return null; - final Element innerTargetElement = collectAllElementsFrom(binding.renderViewElement, skipOffstage: true) - .lastWhere((Element element) => element.renderObject == innerTarget); + ).target; + final Element innerTargetElement = collectAllElementsFrom( + binding.renderViewElement, + skipOffstage: true, + ).lastWhere( + (Element element) => element.renderObject == innerTarget, + orElse: () => null, + ); + if (innerTargetElement == null) { + debugPrint('No widgets found at ${binding.globalToLocal(event.position)}.'); + return; + } final List candidates = []; innerTargetElement.visitAncestorElements((Element element) { candidates.add(element); @@ -324,9 +331,18 @@ class WidgetTester extends WidgetController implements HitTestDispatcher, Ticker int totalNumber = 0; debugPrint('Some possible finders for the widgets at ${binding.globalToLocal(event.position)}:'); for (Element element in candidates) { - if (totalNumber > 10) + if (totalNumber > 13) // an arbitrary number of finders that feels useful without being overwhelming break; - totalNumber += 1; + totalNumber += 1; // optimistically assume we'll be able to describe it + + if (element.widget is Tooltip) { + final Tooltip widget = element.widget; + final Iterable matches = find.byTooltip(widget.message).evaluate(); + if (matches.length == 1) { + debugPrint(' find.byTooltip(\'${widget.message}\')'); + continue; + } + } if (element.widget is Text) { assert(descendantText == null); @@ -347,7 +363,7 @@ class WidgetTester extends WidgetController implements HitTestDispatcher, Ticker key is ValueKey)) { keyLabel = 'const ${element.widget.key.runtimeType}(${key.value})'; } else if (key is ValueKey) { - keyLabel = 'const ${element.widget.key.runtimeType}(\'${key.value}\')'; + keyLabel = 'const Key(\'${key.value}\')'; } if (keyLabel != null) { final Iterable matches = find.byKey(key).evaluate();