Fix dependency skew. (#3306)
...by adding tests to our examples that don't import flutter_test, which
pins the relevant dependencies.
Also, provide more information when complaining about leaked transient
callbacks in tests.
Also, make tests display full information when they have an exception,
by bypassing the throttling we have for Android logging in tests.
Also, make the word wrapping not wrap stack traces if they happen to
be included in exception output.
Also, fix a leaked transient callback in the checkbox code.
diff --git a/dev/manual_tests/pubspec.yaml b/dev/manual_tests/pubspec.yaml
index b3708d8..ab13b22 100644
--- a/dev/manual_tests/pubspec.yaml
+++ b/dev/manual_tests/pubspec.yaml
@@ -2,3 +2,7 @@
dependencies:
flutter:
path: ../../packages/flutter
+dev_dependencies:
+ test: any # flutter_test provides the version constraints
+ flutter_test:
+ path: ../../packages/flutter_test
diff --git a/dev/manual_tests/test/card_collection_test.dart b/dev/manual_tests/test/card_collection_test.dart
new file mode 100644
index 0000000..e574a44
--- /dev/null
+++ b/dev/manual_tests/test/card_collection_test.dart
@@ -0,0 +1,42 @@
+// Copyright 2016 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/material.dart';
+import 'package:flutter_test/flutter_test.dart';
+import 'package:test/test.dart';
+
+import '../card_collection.dart' as card_collection;
+
+void main() {
+ test("Card Collection smoke test", () {
+ testWidgets((WidgetTester tester) {
+ card_collection.main(); // builds the app and schedules a frame but doesn't trigger one
+ tester.pump(); // see https://github.com/flutter/flutter/issues/1865
+ tester.pump(); // triggers a frame
+
+ Element navigationMenu = tester.findElement((Element element) {
+ Widget widget = element.widget;
+ if (widget is Tooltip)
+ return widget.message == 'Open navigation menu';
+ return false;
+ });
+
+ expect(navigationMenu, isNotNull);
+
+ tester.tap(navigationMenu);
+ tester.pump(); // start opening menu
+ tester.pump(const Duration(seconds: 1)); // wait til it's really opened
+
+ // smoke test for various checkboxes
+ tester.tap(tester.findText('Make card labels editable'));
+ tester.pump();
+ tester.tap(tester.findText('Let the sun shine'));
+ tester.pump();
+ tester.tap(tester.findText('Make card labels editable'));
+ tester.pump();
+ tester.tap(tester.findText('Vary font sizes'));
+ tester.pump();
+ });
+ });
+}
diff --git a/examples/hello_world/pubspec.yaml b/examples/hello_world/pubspec.yaml
index a61659d..0c9b34f 100644
--- a/examples/hello_world/pubspec.yaml
+++ b/examples/hello_world/pubspec.yaml
@@ -2,3 +2,7 @@
dependencies:
flutter:
path: ../../packages/flutter
+dev_dependencies:
+ test: any # flutter_test provides the version constraints
+ flutter_test:
+ path: ../../packages/flutter_test
diff --git a/examples/hello_world/test/hello_test.dart b/examples/hello_world/test/hello_test.dart
new file mode 100644
index 0000000..b7f5952
--- /dev/null
+++ b/examples/hello_world/test/hello_test.dart
@@ -0,0 +1,19 @@
+// Copyright 2016 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_test/flutter_test.dart';
+import 'package:test/test.dart';
+
+import '../lib/main.dart' as hello_world;
+
+void main() {
+ test("Hello world smoke test", () {
+ testWidgets((WidgetTester tester) {
+ hello_world.main(); // builds the app and schedules a frame but doesn't trigger one
+ tester.pump(); // triggers a frame
+
+ expect(tester.findText('Hello, world!'), isNotNull);
+ });
+ });
+}
diff --git a/examples/layers/pubspec.yaml b/examples/layers/pubspec.yaml
index eda3add..2eb57a7 100644
--- a/examples/layers/pubspec.yaml
+++ b/examples/layers/pubspec.yaml
@@ -2,3 +2,7 @@
dependencies:
flutter:
path: ../../packages/flutter
+dev_dependencies:
+ test: any # flutter_test provides the version constraints
+ flutter_test:
+ path: ../../packages/flutter_test
diff --git a/examples/layers/test/sector_test.dart b/examples/layers/test/sector_test.dart
new file mode 100644
index 0000000..078d225
--- /dev/null
+++ b/examples/layers/test/sector_test.dart
@@ -0,0 +1,12 @@
+// Copyright 2015 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 '../rendering/src/sector_layout.dart';
+import 'package:test/test.dart';
+
+void main() {
+ test('SectorConstraints', () {
+ expect(const SectorConstraints().isTight, isFalse);
+ });
+}
diff --git a/examples/material_gallery/pubspec.yaml b/examples/material_gallery/pubspec.yaml
index 6ef8144..38040c2 100644
--- a/examples/material_gallery/pubspec.yaml
+++ b/examples/material_gallery/pubspec.yaml
@@ -11,3 +11,8 @@
flutter_markdown:
path: ../../packages/flutter_markdown
flutter_gallery_assets: '0.0.15'
+
+dev_dependencies:
+ test: any # flutter_test provides the version constraints
+ flutter_test:
+ path: ../../packages/flutter_test
diff --git a/examples/material_gallery/test/smoke_test.dart b/examples/material_gallery/test/smoke_test.dart
new file mode 100644
index 0000000..a65c10d
--- /dev/null
+++ b/examples/material_gallery/test/smoke_test.dart
@@ -0,0 +1,62 @@
+// Copyright 2016 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/material.dart';
+import 'package:flutter_test/flutter_test.dart';
+import 'package:test/test.dart';
+
+import '../lib/main.dart' as material_gallery;
+
+void main() {
+ test('Material Gallery app smoke test', () {
+ testWidgets((WidgetTester tester) {
+ material_gallery.main(); // builds the app and schedules a frame but doesn't trigger one
+ tester.pump(); // see https://github.com/flutter/flutter/issues/1865
+ tester.pump(); // triggers a frame
+
+ // Try loading Weather demo
+ tester.tap(tester.findText('Demos'));
+ tester.pump();
+ tester.pump(const Duration(seconds: 1)); // wait til it's really opened
+
+ tester.tap(tester.findText('Weather'));
+ tester.pump();
+ tester.pump(const Duration(seconds: 1)); // wait til it's really opened
+
+ // Go back
+ Element backButton = tester.findElement((Element element) {
+ Widget widget = element.widget;
+ if (widget is Tooltip)
+ return widget.message == 'Back';
+ return false;
+ });
+ expect(backButton, isNotNull);
+ tester.tap(backButton);
+ tester.pump(); // start going back
+ tester.pump(const Duration(seconds: 1)); // wait til it's finished
+
+ // Open menu
+ Element navigationMenu = tester.findElement((Element element) {
+ Widget widget = element.widget;
+ if (widget is Tooltip)
+ return widget.message == 'Open navigation menu';
+ return false;
+ });
+ expect(navigationMenu, isNotNull);
+ tester.tap(navigationMenu);
+ tester.pump(); // start opening menu
+ tester.pump(const Duration(seconds: 1)); // wait til it's really opened
+
+ // switch theme
+ tester.tap(tester.findText('Dark'));
+ tester.pump();
+ tester.pump(const Duration(seconds: 1)); // wait til it's changed
+
+ // switch theme
+ tester.tap(tester.findText('Light'));
+ tester.pump();
+ tester.pump(const Duration(seconds: 1)); // wait til it's changed
+ });
+ });
+}
diff --git a/packages/flutter/lib/src/material/toggleable.dart b/packages/flutter/lib/src/material/toggleable.dart
index 3120379..2dc8f4e 100644
--- a/packages/flutter/lib/src/material/toggleable.dart
+++ b/packages/flutter/lib/src/material/toggleable.dart
@@ -117,6 +117,38 @@
TapGestureRecognizer _tap;
Point _downPosition;
+ @override
+ void attach(PipelineOwner owner) {
+ super.attach(owner);
+ if (_positionController != null) {
+ if (value)
+ _positionController.forward();
+ else
+ _positionController.reverse();
+ }
+ if (_reactionController != null && isInteractive) {
+ switch (_reactionController.status) {
+ case AnimationStatus.forward:
+ _reactionController.forward();
+ break;
+ case AnimationStatus.reverse:
+ _reactionController.reverse();
+ break;
+ case AnimationStatus.dismissed:
+ case AnimationStatus.completed:
+ // nothing to do
+ break;
+ }
+ }
+ }
+
+ @override
+ void detach() {
+ _positionController?.stop();
+ _reactionController?.stop();
+ super.detach();
+ }
+
void _handlePositionStateChanged(AnimationStatus status) {
if (isInteractive) {
if (status == AnimationStatus.completed && !_value)
diff --git a/packages/flutter/lib/src/scheduler/scheduler.dart b/packages/flutter/lib/src/scheduler/scheduler.dart
index 361a2e9..b369ee7 100644
--- a/packages/flutter/lib/src/scheduler/scheduler.dart
+++ b/packages/flutter/lib/src/scheduler/scheduler.dart
@@ -30,10 +30,26 @@
///
/// Combines the task and its priority.
class _TaskEntry {
+ const _TaskEntry(this.task, this.priority);
final VoidCallback task;
final int priority;
+}
- const _TaskEntry(this.task, this.priority);
+class _FrameCallbackEntry {
+ _FrameCallbackEntry(this.callback, { bool rescheduling: false }) {
+ assert(() {
+ if (rescheduling) {
+ assert(currentCallbackStack != null);
+ stack = currentCallbackStack;
+ } else {
+ stack = StackTrace.current;
+ }
+ return true;
+ });
+ }
+ static StackTrace currentCallbackStack;
+ final FrameCallback callback;
+ StackTrace stack;
}
class Priority {
@@ -141,7 +157,7 @@
}
int _nextFrameCallbackId = 0; // positive
- Map<int, FrameCallback> _transientCallbacks = <int, FrameCallback>{};
+ Map<int, _FrameCallbackEntry> _transientCallbacks = <int, _FrameCallbackEntry>{};
final Set<int> _removedIds = new HashSet<int>();
int get transientCallbackCount => _transientCallbacks.length;
@@ -150,9 +166,14 @@
///
/// Adds the given callback to the list of frame-callbacks and ensures that a
/// frame is scheduled.
- int scheduleFrameCallback(FrameCallback callback) {
+ ///
+ /// If `rescheduling` is true, the call must be in the context of a
+ /// frame callback, and for debugging purposes the stack trace
+ /// stored for this callback will be the same stack trace as for the
+ /// current callback.
+ int scheduleFrameCallback(FrameCallback callback, { bool rescheduling: false }) {
_ensureBeginFrameCallback();
- return addFrameCallback(callback);
+ return addFrameCallback(callback, rescheduling: rescheduling);
}
/// Adds a frame callback.
@@ -162,9 +183,18 @@
///
/// The registered callbacks are executed in the order in which they have been
/// registered.
- int addFrameCallback(FrameCallback callback) {
+ ///
+ /// Callbacks registered with this method will not be invoked until
+ /// a frame is requested. To register a callback and ensure that a
+ /// frame is immediately scheduled, use [scheduleFrameCallback].
+ ///
+ /// If `rescheduling` is true, the call must be in the context of a
+ /// frame callback, and for debugging purposes the stack trace
+ /// stored for this callback will be the same stack trace as for the
+ /// current callback.
+ int addFrameCallback(FrameCallback callback, { bool rescheduling: false }) {
_nextFrameCallbackId += 1;
- _transientCallbacks[_nextFrameCallbackId] = callback;
+ _transientCallbacks[_nextFrameCallbackId] = new _FrameCallbackEntry(callback, rescheduling: rescheduling);
return _nextFrameCallbackId;
}
@@ -217,11 +247,11 @@
void _invokeTransientFrameCallbacks(Duration timeStamp) {
Timeline.startSync('Animate');
assert(_debugInFrame);
- Map<int, FrameCallback> callbacks = _transientCallbacks;
- _transientCallbacks = new Map<int, FrameCallback>();
- callbacks.forEach((int id, FrameCallback callback) {
+ Map<int, _FrameCallbackEntry> callbacks = _transientCallbacks;
+ _transientCallbacks = new Map<int, _FrameCallbackEntry>();
+ callbacks.forEach((int id, _FrameCallbackEntry callbackEntry) {
if (!_removedIds.contains(id))
- invokeFrameCallback(callback, timeStamp);
+ invokeFrameCallback(callbackEntry.callback, timeStamp, callbackEntry.stack);
});
_removedIds.clear();
Timeline.finishSync();
@@ -264,8 +294,12 @@
/// Wraps the callback in a try/catch and forwards any error to
/// [debugSchedulerExceptionHandler], if set. If not set, then simply prints
/// the error.
- void invokeFrameCallback(FrameCallback callback, Duration timeStamp) {
+ ///
+ /// Must not be called reentrantly from within a frame callback.
+ void invokeFrameCallback(FrameCallback callback, Duration timeStamp, [ StackTrace stack ]) {
assert(callback != null);
+ assert(_FrameCallbackEntry.currentCallbackStack == null);
+ assert(() { _FrameCallbackEntry.currentCallbackStack = stack; return true; });
try {
callback(timeStamp);
} catch (exception, stack) {
@@ -273,9 +307,55 @@
exception: exception,
stack: stack,
library: 'scheduler library',
- context: 'during a scheduler callback'
+ context: 'during a scheduler callback',
+ informationCollector: (stack == null) ? null : (StringBuffer information) {
+ information.writeln('When this callback was registered, this was the stack:\n$stack');
+ }
));
}
+ assert(() { _FrameCallbackEntry.currentCallbackStack = null; return true; });
+ }
+
+ /// Asserts that there are no registered transient callbacks; if
+ /// there are, prints their locations and throws an exception.
+ ///
+ /// This is expected to be called at the end of tests (the
+ /// flutter_test framework does it automatically in normal cases).
+ ///
+ /// To invoke this method, call it, when you expect there to be no
+ /// transient callbacks registered, in an assert statement with a
+ /// message that you want printed when a transient callback is
+ /// registered, as follows:
+ ///
+ /// ```dart
+ /// assert(Scheduler.instance.debugAssertNoTransientCallbacks(
+ /// 'A leak of transient callbacks was detected while doing foo.'
+ /// ));
+ /// ```
+ ///
+ /// Does nothing if asserts are disabled. Always returns true.
+ bool debugAssertNoTransientCallbacks(String reason) {
+ assert(() {
+ if (transientCallbackCount > 0) {
+ FlutterError.reportError(new FlutterErrorDetails(
+ exception: reason,
+ library: 'scheduler library',
+ informationCollector: (StringBuffer information) {
+ information.writeln(
+ 'There ${ transientCallbackCount == 1 ? "was one transient callback" : "were $transientCallbackCount transient callbacks" } '
+ 'left. The stack traces for when they were registered are as follows:'
+ );
+ for (int id in _transientCallbacks.keys) {
+ _FrameCallbackEntry entry = _transientCallbacks[id];
+ information.writeln('-- callback $id --');
+ information.writeln(entry.stack);
+ }
+ }
+ ));
+ }
+ return true;
+ });
+ return true;
}
/// Ensures that the scheduler is woken by the event loop.
diff --git a/packages/flutter/lib/src/scheduler/ticker.dart b/packages/flutter/lib/src/scheduler/ticker.dart
index e6d453a..284f3eb 100644
--- a/packages/flutter/lib/src/scheduler/ticker.dart
+++ b/packages/flutter/lib/src/scheduler/ticker.dart
@@ -69,12 +69,12 @@
// The onTick callback may have scheduled another tick already.
if (isTicking && _animationId == null)
- _scheduleTick();
+ _scheduleTick(rescheduling: true);
}
- void _scheduleTick() {
+ void _scheduleTick({ bool rescheduling: false }) {
assert(isTicking);
assert(_animationId == null);
- _animationId = Scheduler.instance.scheduleFrameCallback(_tick);
+ _animationId = Scheduler.instance.scheduleFrameCallback(_tick, rescheduling: rescheduling);
}
}
diff --git a/packages/flutter/lib/src/services/print.dart b/packages/flutter/lib/src/services/print.dart
index c96ad37..49f7da0 100644
--- a/packages/flutter/lib/src/services/print.dart
+++ b/packages/flutter/lib/src/services/print.dart
@@ -5,6 +5,9 @@
import 'dart:async';
import 'dart:collection';
+/// Signature for [debugPrint] implementations.
+typedef void DebugPrintCallback(String message, { int wrapWidth });
+
/// Prints a message to the console, which you can access using the "flutter"
/// tool's "logs" command ("flutter logs").
///
@@ -16,9 +19,15 @@
/// to this function (directly or indirectly via [debugDumpRenderTree] or
/// [debugDumpApp]) and to the Dart [print] method can result in out-of-order
/// messages in the logs.
-void debugPrint(String message, { int wrapWidth }) {
+///
+/// The implementation of this function can be replaced by setting the
+/// variable to a new implementation that matches the
+/// [DebugPrintCallback] signature. For example, flutter_test does this.
+DebugPrintCallback debugPrint = _defaultDebugPrint;
+
+void _defaultDebugPrint(String message, { int wrapWidth }) {
if (wrapWidth != null) {
- _debugPrintBuffer.addAll(message.split('\n').expand((String line) => _wordWrap(line, wrapWidth)));
+ _debugPrintBuffer.addAll(message.split('\n').expand((String line) => debugWordWrap(line, wrapWidth)));
} else {
_debugPrintBuffer.addAll(message.split('\n'));
}
@@ -26,7 +35,7 @@
_debugPrintTask();
}
int _debugPrintedCharacters = 0;
-int _kDebugPrintCapacity = 16 * 1024;
+const int _kDebugPrintCapacity = 16 * 1024;
Duration _kDebugPrintPauseTime = const Duration(seconds: 1);
Queue<String> _debugPrintBuffer = new Queue<String>();
Stopwatch _debugPrintStopwatch = new Stopwatch();
@@ -51,10 +60,24 @@
_debugPrintStopwatch.start();
}
}
+
final RegExp _indentPattern = new RegExp('^ *(?:[-+*] |[0-9]+[.):] )?');
enum _WordWrapParseMode { inSpace, inWord, atBreak }
-Iterable<String> _wordWrap(String message, int width) sync* {
- if (message.length < width) {
+/// Wraps the given string at the given width.
+///
+/// Wrapping occurs at space characters (U+0020). Lines that start
+/// with an octothorpe ("#", U+0023) are not wrapped (so for example,
+/// Dart stack traces won't be wrapped).
+///
+/// This is not suitable for use with arbitrary Unicode text. For
+/// example, it doesn't implement UAX #14, can't handle ideographic
+/// text, doesn't hyphenate, and so forth. It is only intended for
+/// formatting error messages.
+///
+/// The default [debugPrint] implementation uses this for its line
+/// wrapping.
+Iterable<String> debugWordWrap(String message, int width) sync* {
+ if (message.length < width || message[0] == '#') {
yield message;
return;
}
diff --git a/packages/flutter_test/lib/src/widget_tester.dart b/packages/flutter_test/lib/src/widget_tester.dart
index 2b24355..f3bb481 100644
--- a/packages/flutter_test/lib/src/widget_tester.dart
+++ b/packages/flutter_test/lib/src/widget_tester.dart
@@ -80,8 +80,18 @@
super(binding: _SteppedWidgetFlutterBinding.ensureInitialized()) {
timeDilation = 1.0;
ui.window.onBeginFrame = null;
+ debugPrint = _synchronousDebugPrint;
}
+ void _synchronousDebugPrint(String message, { int wrapWidth }) {
+ if (wrapWidth != null) {
+ print(message.split('\n').expand((String line) => debugWordWrap(line, wrapWidth)).join('\n'));
+ } else {
+ print(message);
+ }
+ }
+
+
final FakeAsync async;
final Clock clock;
@@ -174,24 +184,22 @@
callback(tester);
runApp(new Container(key: new UniqueKey())); // Unmount any remaining widgets.
async.flushMicrotasks();
+ assert(Scheduler.instance.debugAssertNoTransientCallbacks(
+ 'An animation is still running even after the widget tree was disposed.'
+ ));
assert(() {
- "An animation is still running even after the widget tree was disposed.";
- return Scheduler.instance.transientCallbackCount == 0;
- });
- assert(() {
- "A Timer is still running even after the widget tree was disposed.";
+ 'A Timer is still running even after the widget tree was disposed.';
return async.periodicTimerCount == 0;
});
assert(() {
- "A Timer is still running even after the widget tree was disposed.";
+ 'A Timer is still running even after the widget tree was disposed.';
return async.nonPeriodicTimerCount == 0;
});
assert(async.microtaskCount == 0); // Shouldn't be possible.
- assert(() {
- if (tester._pendingException != null)
- FlutterError.dumpErrorToConsole(tester._pendingException);
- return tester._pendingException == null;
- });
+ if (tester._pendingException != null) {
+ FlutterError.dumpErrorToConsole(tester._pendingException);
+ throw 'An exception (shown above) was thrown during the test.';
+ }
} finally {
FlutterError.onError = oldHandler;
}
diff --git a/travis/test.sh b/travis/test.sh
index e36f2fe..4043ec5 100755
--- a/travis/test.sh
+++ b/travis/test.sh
@@ -14,4 +14,8 @@
(cd packages/flx; dart -c test/all.dart)
(cd packages/newton; dart -c test/all.dart)
+(cd dev/manual_tests; flutter test)
+(cd examples/hello_world; flutter test)
+(cd examples/layers; flutter test)
+(cd examples/material_gallery; flutter test)
(cd examples/stocks; flutter test)