Add --watch mode to flutter test (Redo)
#27192
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
So that it can instead be used inside of `test/runner.dart`
So that it can be used in 'test/runner.dart'
To allow for test watching and incremental compilation
Allows for incremental compile and running of test suites
Use new runTests method, and handle cleanup manually when tests complete. This is blocked by dart-lang/test#973 being merged and the flutter/flutter repo updating its dependency on a version that includes that PR
Also, a little polish on the UX, following hot reload semantics.
Removed references to 'hot reload', and cleaned style nits. Also, updated packages to pass analyze.
zoechi
added
a: tests
Hixie
reviewed
Jan 28, 2019
|
Killing a watch via Ctrl+C does not correctly take down the watcher: |
|
Notes:
|
|
Sorry for the late reply, been doing work to get ready for this release. The issue with Ctrl+C is known. The issue is that the The resolution was to wrap the exception in a try{} catch and basically eat the exception, only printing it with the As for your notes:
|
|
The ⚡ flutter test --watch --plain-name 'backdrop_demo' test/accessibility_test.dart
Building flutter tool...
Running "flutter packages get" in flutter_gallery... 0.4s
00:00 +0: All material demos meet recommended tap target sizes backdrop_demo
00:00 +1: All material demos have labeled tap targets backdrop_demo
00:00 +2: All material demos meet text contrast guidelines backdrop_demo kLightGalleryTheme
00:00 +3: All material demos meet text contrast guidelines backdrop_demo ThemeData.light()
00:01 +4: All tests passed!
Press 'r' to rerun your tests, 'q' to quit |
* Cleaned up trailing whitespace * Refactored watcher to use await for * Addressed style nits * Added flaky annotation to new devicelab test * Ensured that --watch is only used with valid other flags
|
I compared the time on flutter_gallery
The performance improvement of compilation is not huge, but definitely noticeable. However, the biggest win is the removal of the 1+ seconds it takes just to start the test suite. This is a serious problem we should look into (separately) for the flutter tool |
Fail early when mismatched flags are used with --watch
Stop from having an indefinitely growing list of files to invalidate
|
With all of your feedback addressed(I think), do we look good to merge? |
|
You still need to resolve the conflicting files. I also haven't seen an performance comparison yet, so that would be good to note for future archeologists. |
Performance ComparisonThis compares the performance between flutter test in different scenarios between HEAD of this branch versus master/HEAD @ HEADWhole flutter_gallery suiteflutter_gallery[master]⚡ time flutter test
00:35 +129 ~15: .../flutter/examples/flutter_gallery/test/smoke_test.dart: Flutter Gallery app smoke test
> VideoDemo initController "butterfly"
> VideoDemo initController "bee"
> VideoDemo dispose
< VideoDemo dispose
01:05 +130 ~15: .../flutter/examples/flutter_gallery/test/smoke_test.dart: Flutter Gallery app smoke test with semantics
> VideoDemo initController "butterfly"
> VideoDemo initController "bee"
> VideoDemo dispose
< VideoDemo dispose
01:06 +131 ~15: All tests passed!
real 1m7.442s
user 2m21.090s
sys 0m9.429sIndividual spec "drawer_test.dart"flutter_gallery[master]⚡ time flutter test test/drawer_test.dart
00:02 +1: All tests passed!
real 0m3.753s
user 0m4.902s
sys 0m0.726sAfter this PRWhole flutter_gallery suite - single runflutter_gallery[]⚡ time flutter test
00:33 +129 ~15: .../flutter/examples/flutter_gallery/test/smoke_test.dart: Flutter Gallery app smoke test
> VideoDemo initController "butterfly"
> VideoDemo initController "bee"
> VideoDemo dispose
< VideoDemo dispose
01:04 +130 ~15: .../flutter_gallery/test/smoke_test.dart: Flutter Gallery app smoke test with semantics
> VideoDemo initController "butterfly"
> VideoDemo initController "bee"
> VideoDemo dispose
< VideoDemo dispose
01:05 +131 ~15: All tests passed!
real 1m9.738s
user 2m25.467s
sys 0m10.202sIndividual spec "drawer_test.dart"time flutter test test/drawer_test.dart
00:01 +1: All tests passed!
real 0m3.533s
user 0m4.650s
sys 0m0.688sA whole directory "test/demo/**/*.dart" - single runflutter_gallery[ !]⚡ time flutter test test/demo/**/*.dart
Compiling Test Files... 1.9s
00:01 +4: All tests passed!
real 0m4.546s
user 0m9.934s
sys 0m1.222sA whole directory with --watch "test/demo/**/*.dart" - multiple runsThe first run... flutter_gallery[ !]⚡ time flutter test --watch test/demo/**/*.dart
Compiling Test Files... 1.8s
00:02 +4: All tests passed!
Press 'r' to rerun your tests, 'q' to quitSecond run - no changes 00:01 +4: All tests passed!
Press 'r' to rerun your tests, 'q' to quitThird run - introduce error in test file Recompiling test files...
1.0s
00:00 +0: .../flutter/examples/flutter_gallery/test/demo/material/drawer_demo_test.dart: Drawer header does not scroll
══╡ EXCEPTION CAUGHT BY FLUTTER TEST FRAMEWORK ╞════════════════════════════════════════════════════
The following StateError was thrown running a test:
Bad state: No element
When the exception was thrown, this was the stack:
... [STACK TRACE] ...
(elided 28 frames from class _FakeAsync, package dart:async, and package stack_trace)
The test description was:
Drawer header does not scroll
════════════════════════════════════════════════════════════════════════════════════════════════════
00:00 +0 -1: .../flutter/examples/flutter_gallery/test/demo/material/drawer_demo_test.dart: Drawer header does not scroll [E]
Test failed. See exception logs above.
The test description was: Drawer header does not scroll
00:01 +3 -1: Some tests failed.
Press 'r' to rerun your tests, 'q' to quitFourth run - fix error in test file Recompiling test files...
0.9s
00:01 +4: All tests passed!
Press 'r' to rerun your tests, 'q' to quitFifth run - introduce error to source file Recompiling test files...
1.1s
00:00 +0: .../flutter/examples/flutter_gallery/test/demo/material/drawer_demo_test.dart: Drawer header does not scroll
══╡ EXCEPTION CAUGHT BY FLUTTER TEST FRAMEWORK ╞════════════════════════════════════════════════════
The following StateError was thrown running a test:
Bad state: No element
When the exception was thrown, this was the stack:
... [STACK TRACE] ...
(elided 28 frames from class _FakeAsync, package dart:async, and package stack_trace)
The test description was:
Drawer header does not scroll
════════════════════════════════════════════════════════════════════════════════════════════════════
00:00 +0 -1: .../flutter/examples/flutter_gallery/test/demo/material/drawer_demo_test.dart: Drawer header does not scroll [E]
Test failed. See exception logs above.
The test description was: Drawer header does not scroll
00:02 +3 -1: Some tests failed.
Press 'r' to rerun your tests, 'q' to quitSixth run - Fix error in source file Recompiling test files...
1.1s
00:01 +4: All tests passed!
Press 'r' to rerun your tests, 'q' to quitAs can be seen the biggest source of improvement is by cutting compilation time in roughly half, and cutting out the 1-1.5 second delay of starting the flutter tools. So the gains are really only realized for scenarios in which either compilation is very complex, or where that startup time represents a large percentage of the overall test execution time (say 20+%). |
|
@gamebox have the changes needed in the test packages rolled into google3 yet? |
And don't allow another run until the tests have completed, not just compiled.
|
The changes are basically ready to merge. Waiting for an LGTM |
There was a problem hiding this comment.
I checked out your branch again, cleaned and ran flutter test --watch test/material/slider_theme_test.dart.
It worked on the first run and subsequently failed on a compile error with no changes:
jonahwilliams-macbookpro2:flutter jonahwilliams$ flutter test --watch test/material/slider_theme_test.dart
00:01 +7: All tests passed!
DONE: 1506 ms
Press 'r' to rerun your tests, 'q' to quit
00:00 +0: loading /Users/jonahwilliams/Documents/flutter/packages/flutter/test/material/slider_theme_test.dart Shell: [ERROR:flutter/shell/common/engine.cc(175)] Could not prepare to run the isolate.
Shell: [ERROR:flutter/shell/common/engine.cc(122)] Engine not prepare and launch isolate.
Shell: [ERROR:flutter/shell/testing/tester_main.cc(199)] Could not launch the engine with configuration.
00:00 +0 -1: loading /Users/jonahwilliams/Documents/flutter/packages/flutter/test/material/slider_theme_test.dart [E]
Failed to load "/Users/jonahwilliams/Documents/flutter/packages/flutter/test/material/slider_theme_test.dart":
Shell subprocess cleanly reported an error before connecting to test harness. Check the logs above for an error message.
Test: /Users/jonahwilliams/Documents/flutter/packages/flutter/test/material/slider_theme_test.dart
Shell: /Users/jonahwilliams/Documents/flutter/bin/cache/artifacts/engine/darwin-x64/flutter_tester
00:00 +0 -1: Some tests failed.
DONE: 81 ms
unhandled error during finalization of test:
/Users/jonahwilliams/Documents/flutter/packages/flutter/test/material/slider_theme_test.dart
FileSystemException: Deletion failed, path = '/var/folders/sq/vd7vtp0j60ld19wt58vg7n5c00c4f_/T/flutter_test_listener.tAHNze' (OS Error: No such file or directory, errno = 2)
#0 _Directory._deleteSync (dart:io/directory_impl.dart:206:7)
#1 FileSystemEntity.deleteSync (dart:io/file_system_entity.dart:466:47)
#2 ForwardingFileSystemEntity.deleteSync (package:file/src/forwarding/forwarding_file_system_entity.dart:72:16)
#3 createListenerDart.<anonymous closure> (package:flutter_tools/src/test/bootstrap.dart:143:15)
<asynchronous suspension>
#4 _FlutterPlatform._startTest (package:flutter_tools/src/test/flutter_platform.dart:478:26)
<asynchronous suspension>
#5 _FlutterPlatform.loadChannel (package:flutter_tools/src/test/flutter_platform.dart:193:36)
#6 _FlutterPlatform.load (package:flutter_tools/src/test/flutter_platform.dart:151:46)
<asynchronous suspension>
#7 Loader.loadFile.<anonymous closure> (package:test_core/src/runner/loader.dart:223:36)
<asynchronous suspension>
#8 new LoadSuite.<anonymous closure>.<anonymous closure> (package:test_core/src/runner/load_suite.dart:98:31)
<asynchronous suspension>
#9 invoke (package:test_api/src/utils.dart:241:5)
#10 new LoadSuite.<anonymous closure> (package:test_core/src/runner/load_suite.dart:97:7)
#11 Invoker._onRun.<anonymous closure>.<anonymous closure>.<anonymous closure>.<anonymous closure> (package:test_api/src/backend/invoker.dart:400:25)
<asynchronous suspension>
#12 new Future.<anonymous closure> (dart:async/future.dart:176:37)
#13 StackZoneSpecification._run (package:stack_trace/src/stack_zone_specification.dart:209:15)
#14 StackZoneSpecification._registerCallback.<anonymous closure> (package:stack_trace/src/stack_zone_specification.dart:119:48)
#15 _rootRun (dart:async/zone.dart:1120:38)
#16 _CustomZone.run (dart:async/zone.dart:1021:19)
#17 _CustomZone.runGuarded (dart:async/zone.dart:923:7)
#18 _CustomZone.bindCallbackGuarded.<anonymous closure> (dart:async/zone.dart:963:23)
#19 StackZoneSpecification._run (package:stack_trace/src/stack_zone_specification.dart:209:15)
#20 StackZoneSpecification._registerCallback.<anonymous closure> (package:stack_trace/src/stack_zone_specification.dart:119:48)
#21 _rootRun (dart:async/zone.dart:1124:13)
#22 _CustomZone.run (dart:async/zone.dart:1021:19)
#23 _CustomZone.bindCallback.<anonymous closure> (dart:async/zone.dart:947:23)
#24 Timer._createTimer.<anonymous closure> (dart:async/runtime/libtimer_patch.dart:21:15)
#25 _Timer._runTimers (dart:isolate/runtime/libtimer_impl.dart:382:19)
#26 _Timer._handleMessage (dart:isolate/runtime/libtimer_impl.dart:416:5)
#27 _RawReceivePortImpl._handleMessage (dart:isolate/runtime/libisolate_patch.dart:171:12)
Press 'r' to rerun your tests, 'q' to quit
I've fetched/updated, are you seeing this too or do I have a local issue?
| @@ -0,0 +1,185 @@ | |||
| // Copyright 2019 The Chromium Authors. All rights reserved. | |||
There was a problem hiding this comment.
This needs the original copyright year
There was a problem hiding this comment.
The year that is attached to the original file?
| @@ -0,0 +1,169 @@ | |||
| // Copyright 2019 The Chromium Authors. All rights reserved. | |||
There was a problem hiding this comment.
This needs the original copyright year
| } | ||
| } | ||
| } |
|
Cannot reproduce your issue. May be local. Happy to take a look |
|
It seems as if the temporary files are not being cleaned up |
|
Did you rebuild the snapshot? I get the following when run with the 'v' flag [ +7 ms] test 0: awaiting initial result for pid 247071
[ +239 ms] test 0: process with pid 247071 connected to test harness
[ ] test 0: awaiting test result for pid 247071
00:01 +22: The ... skip all component painting except the value indicator [ +765 ms] test 0: process with pid 247071 no longer needed by test harness
[ ] test 0: cleaning up...
[ ] test 0: ensuring end-of-process for shell
[ +4 ms] test 0: shutting down test harness socket server
[ +1 ms] test 0: deleting temporary directory
[ +2 ms] test 0: finished
00:01 +22: All tests passed!
[ +6 ms] Deleting /tmp/flutter_test_fonts.MXPXBB...
[ +6 ms] Press 'r' to rerun your tests, 'q' to quit |
Will be replaced soon with package:build
|
I can reliably reproduce this when re-running a single test: |
|
Another point in favor of this mode of running tests: when integrating package build we will have some non-trivial overhead in verifying an incremental build. This can also help with that. |
11 tasks
|
@gamebox could you pick this great PR up again? It's such a great addition and removes SO MUCH friction when running tests / building a test suite. |
|
I hope that work done in this PR will not be wasted. This feature greatly improves high quality software development. |
|
@gamebox if you're not working on this right now, could we close it to get it off the PR queue? |
christopherfujino
added
the
waiting for customer response
|
I'm closing this pull request since it is out of sync with head, has had no activity in roughly a month, and does not appear to be resolved. @gamebox if you plan to continue with this, please feel free to re-open after synchronizing this with HEAD. Otherwise, this work is tracked by the linked issue. |
9 tasks
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a clean rebase of my previous PR (#26247). Merge conflicts got out of control on old branch. This should read much easier.
Other people looking at this, please consult the other PR for previous conversation.l