Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add --watch mode to flutter test #26247

Closed
wants to merge 38 commits into from

Conversation

gamebox
Copy link
Contributor

@gamebox gamebox commented Jan 8, 2019

When flag is provided watch application directory for file changes and re-run tests on change, only incrementally compiling changed files. This provides a significant speedup on incremental edit/test cycle.

Catch up to flutter/flutter master
Merge from flutter/flutter
@jonahwilliams
Copy link
Member

Its pretty difficult to tell what changed here. I would start by undoing the formatting and general refactoring changes

@zoechi zoechi added a: tests "flutter test", flutter_test, or one of our tests tool Affects the "flutter" command-line tool. See also t: labels. labels Jan 9, 2019
@gamebox
Copy link
Contributor Author

gamebox commented Jan 9, 2019

@jonahwilliams Updated with a much more readable series of commits.

@jonahwilliams
Copy link
Member

I haven't forgotten about this! Taking a longer look at it today.

@gamebox
Copy link
Contributor Author

gamebox commented Jan 11, 2019

The test failures are expected due to the facts laid out in the commit message of the commit just added

@jonahwilliams
Copy link
Member

Refactoring is fine but not necessary to this PR.

I pulled your gamebox/flutter and pointed test at gamebox/test with dependency overrides. flutter test --watch ran correctly the first time, but it did not detect any changes to the test directory or lib. I also could not exit the process without CTRL+C.

What is the intended workflow here? am I watching everything, a specific test file, or a library file?. I can see different cases where a watch command is useful depending on what I am working on:

  • I am adjusting a library file and I want to rerun one test, multiple tests, multiple test files on change
  • I am adjusting a test file and I want to rerun it on change.

Have you considered any other approaches? For example, I could wire up either command myself with some shell commands, but it would be too slow. Thus it seems like caching the kernel files to disk might also speed up incremental runs. Of course, you would have to be more careful with invalidation!

@apaatsio
Copy link
Contributor

I'll just mention #4719 here since it's relevant.

@gamebox
Copy link
Contributor Author

gamebox commented Jan 17, 2019

Thanks @apaatsio , I'll check in with @Hixie about the operational semantics of this feature.

@gamebox
Copy link
Contributor Author

gamebox commented Jan 17, 2019

@jonahwilliams Just an update, my change dart-lang/test was approved. Once it's merged, @grouma will cut a new release, and then I can update this branch to use it

@gamebox
Copy link
Contributor Author

gamebox commented Jan 24, 2019

Alright @jonahwilliams , this includes the dart-lang/test updates, so this should reflect generally what I would like to release as a WIP feature in the tool. The semantics have been changed from rerun-on-save, to the same as hot reload('r' to rerun, 'q' to quit).

@jonahwilliams
Copy link
Member

Got everything working locally, this is pretty neato!

packages/flutter_tools/lib/src/commands/test.dart Outdated Show resolved Hide resolved
packages/flutter_tools/lib/src/test/bootstrap.dart Outdated Show resolved Hide resolved
packages/flutter_tools/lib/src/test/runner.dart Outdated Show resolved Hide resolved
packages/flutter_tools/lib/src/commands/test.dart Outdated Show resolved Hide resolved
packages/flutter_tools/lib/src/commands/test.dart Outdated Show resolved Hide resolved
packages/flutter_tools/lib/src/commands/test.dart Outdated Show resolved Hide resolved
@gamebox
Copy link
Contributor Author

gamebox commented Jan 25, 2019

Will address the rest of the comments in the morning. Thanks for the thorough review. Most of my issues remaining have to do with updating the dependencies. Will discuss offline.

@gamebox gamebox changed the title WIP: Add --watch mode to flutter test Add --watch mode to flutter test Jan 25, 2019
devoncarew and others added 6 commits January 25, 2019 14:17
* Revert "Replace netls and netaddr with dev_finder (flutter#26090)"

This reverts commit eee154a.
…r change (flutter#26239)

* Force DatePicker value to stay within firstDate and lastDate upon year change

* updated per review feedback
flutter/engine@7112b72...e5ec3cf

git log
7112b72..e5ec3cf
--no-merges --oneline
e5ec3cf Dart SDK roll for 2019-01-08
08c95d2 Roll src/third_party/skia 55ff5d3ba881..1337f5b85978 (10
commits) (flutter/engine#7407)
e385f5c Roll src/third_party/skia 26d173fee72b..55ff5d3ba881 (12
commits) (flutter/engine#7406)
0f8273b Dart SDK roll for 2019-01-07
4036b26 Reset ParagraphBuilder after build() (flutter/engine#7401)
4820cbe Dart SDK roll for 2019-01-07
8eccb86 Add onStart hook to FlutterFragmentActivity
(flutter/engine#6719)
f2ea838 Roll src/third_party/skia b2fdcbf3033f..26d173fee72b (10
commits) (flutter/engine#7400)
5ca8aad Announce in/out of list (flutter/engine#6918)
4487d39 Replace Java code with equivalent, more concise code.
(flutter/engine#7398)
395b785 Roll src/third_party/skia 46ee3f7a8ff5..b2fdcbf3033f (11
commits) (flutter/engine#7394)
5965f90 Make `ParagraphConstraints` have const constructor
(flutter/engine#7346)
e02dd41 Roll src/third_party/skia a47eb455360f..46ee3f7a8ff5 (2
commits) (flutter/engine#7390)
f0038b3 Roll src/third_party/skia 3ac3a4053f86..a47eb455360f (2
commits) (flutter/engine#7389)
jonahwilliams and others added 24 commits January 25, 2019 14:17
* Remove *.lock from gitignore

* Remove pubspec.lock from gitignore
* the onStart callback will report the location of the pointer where it wins the gesture arena by default instead of the pointer down location. Fixes all tests related to changing this default value.
The variable isn't interpolating because it's using "${}" when ruby uses "#{}".
* update docs for editable_text (TextField)

* typo, clarify behavior when lines > 1
When setting the Fuchsia logging function, it should happen before any
initialization code, as init can still cause warning/error/info messages
to get printed to logs. Since the default stderr/stdout fd's aren't
correct, this can cause a program to crash for unclear reasons.
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.
@gamebox
Copy link
Contributor Author

gamebox commented Jan 25, 2019

That last "queued" test has actually completed and passed. Don't know if that's something wrong on Cirrus or GH's side.

@gamebox
Copy link
Contributor Author

gamebox commented Jan 28, 2019

Closing in favor of (#27192)

@gamebox gamebox closed this Jan 28, 2019
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
a: tests "flutter test", flutter_test, or one of our tests tool Affects the "flutter" command-line tool. See also t: labels.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet