[flutter_tools] Support Integration Tests #76200

jiahaog · 2021-02-17T12:06:40Z

Implements https://flutter.dev/go/tool-integration-test-support

Usage

Examples below are not exhaustive. Flags compatible with flutter test should work as well.

# Run an Integration Test.
flutter test integration_test/some_test.dart

# Run an Integration Test on the specified device.
flutter -d my-device-id test integration_test/some_test.dart

# Run all tests in `integration_test/` as Integration Tests.
flutter test integration_test

Minor changes:

Translate received test file paths to absolute paths before passing them to the package:test runner
Some tweaking of the FakeDevice to support mocking a LaunchResult

Upcoming PR

Support web
Device lab tests (Add tests for integration tests to devicelab #79258)

Related issue: #66264

jiahaog · 2021-02-24T11:23:12Z

packages/flutter_tools/lib/src/test/test_device.dart

+  /// It is up to the device to determine if [entrypointPath] is a precompiled
+  /// or raw source file.


Unfortunately the abstraction here isn't very ideal.

For flutter_test, we can accept the precompiled dill, or if we move the compiler into FlutterTesterTestDevice, we can accept Dart source files.

For a raw Device, it accepts a source file, or a precompiled APK/IPA.

I can't quite think of a way to abstract this better given the awkward compilation of sources that happen in the FlutterTestPlatform for the flutter_tester. let me know if there's any better ideas.

I think its OK, provided you document the convention.

jiahaog · 2021-02-24T11:26:25Z

@jonahwilliams Let me know if you have any early feedback for this - Most of the problems I've been having here have been resolved. (Still working on adding the tests)

packages/flutter_tools/lib/src/commands/test.dart

jonahwilliams · 2021-02-24T16:56:53Z

packages/flutter_tools/lib/src/commands/test.dart

@@ -23,7 +23,7 @@ import '../test/runner.dart';
 import '../test/test_wrapper.dart';
 import '../test/watcher.dart';

-class TestCommand extends FlutterCommand {
+class TestCommand extends FlutterCommand with DeviceBasedDevelopmentArtifacts {


Consider extending the description of the test command to describe the difference in behavior between test and integration test commands

Updated the doc comment for this

jonahwilliams · 2021-02-24T16:58:39Z

packages/flutter_tools/lib/src/test/integration_test_device.dart

+  /// [entrypointPath] must be a path to an uncompiled source file.
+  @override
+  Future<StreamChannel<String>> start(String entrypointPath) async {
+    final TargetPlatform targetPlatform = await device.targetPlatform;


Its worth thinking about how this implementation could be updated in the future to support testing without rebuilding the APK/IPA on each go. For example, by hot restarting the main isolate with the new entrypoint

Yes, I think we could make flutter_platform.dart keep track of some sort of mutable state where rebuilding is skipped for subsequent workflows. For the first run we basically do what is here in this PR, and for subsequent runs we would invoke portions of the attach workflow, to attach and then hot restart the application. I can't be sure if it's sufficient without a prototype, but it seems like a reloadExistingApp boolean parameter here would be sufficient. It's also fine if we want to make breaking changes to the interface here because it's all internal anyway.

Sounds reasonable to me

jonahwilliams · 2021-02-24T17:00:14Z

packages/flutter_tools/lib/src/test/integration_test_device.dart

+    final vm_service.VmService vmService = await connectToVmService(launchResult.observatoryUri);
+
+    globals.printTrace('test $id: Finding the correct isolate with the integration test service extension');
+    final vm_service.IsolateRef isolateRef = await _findExtensionIsolate(vmService);


There may be some timing issues here, since the kIntegrationTestMethod will not be registered until after main finishes, but this may be after the app/isolate has started. There is a waitForExtension method in the resident_runner library that handles this cases, though it doesn't identify the correct Isolate:

flutter/packages/flutter_tools/lib/src/resident_devtools_handler.dart

Line 186 in dea1c46

Future<void> waitForExtension(vm_service.VmService vmService, String extension) async {

Would there be issues with the implementation of _findExtensionIsolate method defined below? It's implementing something similar to the waitForExtension method (both subscribing to registered extension events as well as querying for current extensions, which should solve the timing issues here

Yeah that makes sense - I would prefer trying to reuse the existing helper if possible.

Refactoring to make the existing helper reusable here was done in #77220, and just updated this PR to use the helper here.

jonahwilliams · 2021-02-24T17:00:46Z

packages/flutter_tools/lib/src/test/test_device.dart

+  /// It is up to the device to determine if [entrypointPath] is a precompiled
+  /// or raw source file.


I think its OK, provided you document the convention.

jiahaog · 2021-03-29T14:28:31Z

Sorry for the delay, I was caught up with some other things, and just found some time to clean this PR up and add more tests. It is finally ready for review 🙂

(I also have a follow up PR #79258 to add device lab tests)

jonahwilliams

Overall this LGTM.

How comfortable do you feel with these changes, do you think we should wait until the next release cut, or until the devicelab test is ready?

As long as the existing flutter test behavior` is unchanged it should be OK

@isolate

Canonical paths become lowercased on Windows. This causes less information to be returned from the VM after adding a breakpoint (observe that the UnresolvedSourceLocation does not have a `script` below: bad (at previous commit) ``` {"jsonrpc":"2.0","method":"streamNotify","params":{"streamId":"Debug","event":{"type":"Event","kind":"BreakpointAdded","isolate":{"type":"@isolate","id":"isolates/2153249320239607","name":"main","number":"2153249320239607","isSystemIsolate":false},"timestamp":1617078845364,"breakpoint":{"type":"Breakpoint","fixedId":true,"id":"breakpoints/1","breakpointNumber":1,"resolved":false,"location":{"type":"UnresolvedSourceLocation","scriptUri":"file:///C:/Users/Jia%20Hao/AppData/Local/Temp/flutter_test_expression_eval_test.c001d9de/test/test.dart","line":5}}}}} ``` good (at HEAD) ``` {"jsonrpc":"2.0","method":"streamNotify","params":{"streamId":"Debug","event":{"type":"Event","kind":"BreakpointAdded","isolate":{"type":"@isolate","id":"isolates/2029881360067999","name":"main","number":"2029881360067999","isSystemIsolate":false},"timestamp":1617079201477,"breakpoint":{"type":"Breakpoint","fixedId":true,"id":"breakpoints/1","breakpointNumber":1,"resolved":false,"location":{"type":"UnresolvedSourceLocation","script":{"type":"@script","fixedId":true,"id":"libraries/@22372424/scripts/file%3A%2F%2F%2FC%3A%2FUsers%2FJia%2520Hao%2FAppData%2FLocal%2FTemp%2Fflutter_test_expression_eval_test.17a2bb2c%2Ftest%2Ftest.dart/178816e6d05","uri":"file:///C:/Users/Jia%20Hao/AppData/Local/Temp/flutter_test_expression_eval_test.17a2bb2c/test/test.dart","_kind":"kernel"},"line":5}}}}} ``` This makes the test [1] confused as it keeps waiting forever. [1]: https://github.com/flutter/flutter/blob/e43738824e88827984bbd7a85fdb11e0ef5398c9/packages/flutter_tools/test/integration.shard/expression_evaluation_test.dart#L113

jiahaog · 2021-03-30T07:29:54Z

I think it should be fine to land this without waiting for the next release cut. The only change to the normal flutter test flow is the change to use canonical paths which didn't work on windows when debugging, so I've just updated the PR to use absolute paths. Also just manually tested adding a breakpoint and expression evaluation of the widget test flow on macOS, Linux and Windows in VSCode, and it seems to work fine. Is there anything else I should test manually?

Also, I figured out how to test the device lab changes locally in #79258 (found an issue and updated this PR to disable port publication like how drive does it), and we should be good to land the device lab change right after this PR lands. Splitting it off into its own PR so that would be easier to revert, as I'm a bit unsure of whether dev/prod_builders.json is specified correctly there.

DanTup · 2021-03-30T11:26:27Z

FWIW, I have an open PR in Dart-Code to support this. I don't plan to merge it for the upcoming release (this week) as I wanted to re-test it all once this PR is complete/merged (it also needs some automated tests). I'll likely merge it just after my release though (and I can make a preview build of the extension that includes the changes for testing/using in the meantime).

jonahwilliams

only have a nit.

LGTM

packages/flutter_tools/lib/src/commands/test.dart

Co-authored-by: Jonah Williams <jonahwilliams@google.com>

jiahaog · 2021-03-31T00:18:38Z

@DanTup sure, no hurry. I'll merge this and then try and patch your change as well to test it out

flutter-dashboard bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Feb 17, 2021

google-cla bot added the cla: yes label Feb 17, 2021

jiahaog force-pushed the integration-test branch 2 times, most recently from 84eb4f9 to 68e8984 Compare February 24, 2021 01:15

jiahaog mentioned this pull request Feb 24, 2021

Update all packages #76673

Merged

jiahaog force-pushed the integration-test branch 2 times, most recently from c565fdc to 29ecbd7 Compare February 24, 2021 08:24

[flutter_tools] Support Integration Tests

590ede2

jiahaog force-pushed the integration-test branch from 29ecbd7 to 590ede2 Compare February 24, 2021 09:05

jiahaog commented Feb 24, 2021

View reviewed changes

jonahwilliams reviewed Feb 24, 2021

View reviewed changes

packages/flutter_tools/lib/src/commands/test.dart Outdated Show resolved Hide resolved

jonahwilliams reviewed Feb 24, 2021

View reviewed changes

packages/flutter_tools/lib/src/commands/test.dart Outdated Show resolved Hide resolved

jonahwilliams reviewed Feb 24, 2021

View reviewed changes

jiahaog mentioned this pull request Mar 5, 2021

[flutter_tools] Generalize waitForExtension #77220

Merged

jiahaog added 8 commits March 11, 2021 09:47

Merge branch 'master' into integration-test

8d3f7fd

Indentation, docs

a9f160f

Fix an issue with locked devices

b392662

Merge branch 'master' into integration-test

bca4926

Add tests, OS agnostic paths, more exception handling

bb02b15

Merge branch 'master' into integration-test

b86eea7

Additional refactoring, changes to UX

64f0254

More tests

3f94d90

jiahaog mentioned this pull request Mar 29, 2021

Add tests for integration tests to devicelab #79258

Merged

jiahaog added 2 commits March 29, 2021 17:34

Fix analyzer

928b433

Fix wrong comment

2db0db0

jiahaog marked this pull request as ready for review March 29, 2021 14:12

flutter-dashboard bot added the c: contributor-productivity Team-specific productivity, code health, technical debt. label Mar 29, 2021

jiahaog requested a review from jonahwilliams March 29, 2021 14:28

jonahwilliams reviewed Mar 29, 2021

View reviewed changes

jiahaog added 2 commits March 30, 2021 10:03

Disable port publication so device lab doesn't show the prompt on iOS 14

64f8a63

jonahwilliams approved these changes Mar 30, 2021

View reviewed changes

packages/flutter_tools/lib/src/commands/test.dart Outdated Show resolved Hide resolved

Update packages/flutter_tools/lib/src/commands/test.dart

6774716

Co-authored-by: Jonah Williams <jonahwilliams@google.com>

jiahaog added the waiting for tree to go green label Mar 31, 2021

fluttergithubbot merged commit 9e3eadb into flutter:master Mar 31, 2021

jiahaog deleted the integration-test branch March 31, 2021 02:41

This was referenced Mar 31, 2021

Use writeTimelineToFile flutter/gallery#470

Merged

[flutter_tools] Fix breakage when test main is tagged with @doNotStore #79501

Merged

DanTup mentioned this pull request Apr 12, 2021

Support debugging flutter drive integration tests from the editors #24385

Closed

HakkyuKim mentioned this pull request Jul 27, 2021

Drive command can't detect or display test failures for some plugins. flutter-tizen/flutter-tizen#168

Closed

3 tasks

		/// It is up to the device to determine if [entrypointPath] is a precompiled
		/// or raw source file.

[flutter_tools] Support Integration Tests #76200

[flutter_tools] Support Integration Tests #76200

Uh oh!

Conversation

jiahaog commented Feb 17, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Usage

Upcoming PR

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jiahaog commented Feb 24, 2021

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jiahaog Feb 25, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jiahaog commented Mar 29, 2021

Uh oh!

jonahwilliams left a comment

Choose a reason for hiding this comment

Uh oh!

jiahaog commented Mar 30, 2021

Uh oh!

DanTup commented Mar 30, 2021

Uh oh!

jonahwilliams left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jiahaog commented Mar 31, 2021

Uh oh!

Uh oh!

jiahaog commented Feb 17, 2021 •

edited

Loading

jiahaog Feb 25, 2021 •

edited

Loading