Scaffolding for `NativeDriver` and `AndroidNativeDriver` for taking screenshots using `adb`. #152194

matanlurey · 2024-07-23T21:54:25Z

I have next to no clue how to configure this to run on CI, so bear with me as I rediscover the wheel.

christopherfujino · 2024-07-23T23:18:08Z

.ci.yaml

      - .ci.yaml

+  - name: Linux_android_emu flutter_driver_android_test
+    recipe: devicelab/devicelab_drone


I think the devicelab_drone recipe invokes the //dev/devicelab/bin/test_runner.dart entrypoint: https://flutter.googlesource.com/recipes/+/refs/heads/main/recipes/devicelab/devicelab_drone.py#44 and not the test.dart one. I think you will need to either:

make your test a devicelab task; or

make this target use the flutter/flutter_drone recipe, and set the shard property field, which is what test.dart uses for the key in that hash map you updated.

I ... see. Let me try the latter and see what happens I guess.

jmagman · 2024-07-23T23:44:30Z

packages/flutter_driver/lib/src/native/android.dart

+  }
+
+  @override
+  Future<NativeScreenshot> screenshot() async {


This feels like it's recreating a lot of the logic we've already added to flutter_tools. Have we considered communicating more between flutter_driver and FlutterDriverService so it can do the device poking it already knows how to do? I need to look more into the specifics of your proposal, but we have similar logic in so many places already in flutter/ it would be great to not have yet another that needs to be maintained by the platform teams (for example, the way screenshotting worked totally broke in iOS 2 years ago and we still haven't been able to totally replace it).

I think that's a valid critique of what is available so far (though, there will be lots of things that have no parallel in flutter_tools, such as navigating through applications or trimming memory).

We don't have, to my knowledge, a good model for sharing code between these different packages (for example, I'm also going to need to re-implement matchesGolden, which currently depends on dart:ui existing).

Some ideas:

Try to cobble together a flutter_common (?) package used by tools, test, driver

Consider having driver import from tool

Consider having tool import from driver

I'm not planning on immediately resolving this, but I am aware that this is duplication of at least some effort and will be a tax on platform teams (though, at least for Android, platform teams also need the functionality produced here, so maybe it's less of a tax and more of a trade?)

/cc @johnmccutchan for visibility.

Check out https://github.com/flutter/flutter/blob/main/packages/flutter_tools/lib/src/ios/core_devices.dart for the various data that is accessible host-side about an iOS device. The logic in that file would have to be duplicated over to flutter_driver unless there's a shared library, as you say, or unless the driver service is poking back to the host flutter_tool to delegate it to do these host-side operations.

Try to cobble together a flutter_common (?) package used by tools, test, driver

And devicelab harness (there's even tap() logic in there)

flutter/dev/devicelab/lib/framework/devices.dart

Lines 314 to 315 in 3c4ee10

Future<List<String>> discoverDevices() async {

final List<String> output = (await eval(adbPath, <String>['devices', '-l']))

flutter/dev/devicelab/lib/framework/devices.dart

Lines 695 to 696 in 3c4ee10

Future<void> shellExec(String command, List<String> arguments, { Map<String, String>? environment, bool silent = false }) async {

await adb(<String>['shell', command, ...arguments], environment: environment, silent: silent);

flutter/dev/devicelab/lib/framework/devices.dart

Lines 622 to 645 in 3c4ee10

Future<void> home() async {

await shellExec('input', const <String>['keyevent', '3']);

}

/// Sends `KEYCODE_POWER` (26), which causes the device to toggle its mode

/// between awake and asleep.

@override

Future<void> togglePower() async {

await shellExec('input', const <String>['keyevent', '26']);

}

/// Unlocks the device by sending `KEYCODE_MENU` (82).

///

/// This only works when the device doesn't have a secure unlock pattern.

@override

Future<void> unlock() async {

await wakeUp();

await shellExec('input', const <String>['keyevent', '82']);

}

@override

Future<void> tap(int x, int y) async {

await shellExec('input', <String>['tap', '$x', '$y']);

}

Mostly the devicelab calls back to the tool to do things, like take a screenshot:

flutter/dev/devicelab/lib/framework/utils.dart

Lines 511 to 513 in 3d79c2e

if (exitCode != 0 && !canFail) {

await _flutterScreenshot(workingDirectory: workingDirectory);

}

flutter/dev/devicelab/lib/framework/utils.dart

Lines 591 to 612 in 3d79c2e

Future<void> _flutterScreenshot({ String? workingDirectory }) async {

try {

final Directory? dumpDirectory = hostAgent.dumpDirectory;

if (dumpDirectory == null) {

return;

}

// On command failure try uploading screenshot of failing command.

final String screenshotPath = path.join(

dumpDirectory.path,

'device-screenshot-${DateTime.now().toLocal().toIso8601String()}.png',

);

final String deviceId = (await devices.workingDevice).deviceId;

print('Taking screenshot of working device $deviceId at $screenshotPath');

final List<String> args = _flutterCommandArgs(

'screenshot',

<String>[

'--out',

screenshotPath,

'-d', deviceId,

],

);

I'm not planning on immediately resolving this

Figuring out if this code is going to be a duplicate of flutter_tools seems pretty important for where you start on the larger flutter_driver project. You'll have a lot more available if you start with the ways the tool can interact with a device than if you start from scratch.

These are super valuable references, thanks for sharing the code snippets!

Sorry, my comment about not resolving this is not self-serving - I'm actually not confident that this approach will work well enough (i.e. on our CI) where investing in this past a series of 1-off "engine validation" tests is an optimal approach, and I want to de-risk this (i.e. spend 1-2 weeks to ensure the next 1-2 months is well spent).

I do agree that, once the prototype is fleshed out (read: running on CI, and showing it's capable of catching the bugs we're writing this system for), it would be good to take a step back and likely sync with @jonahwilliams and @christopherfujino on de-layering/code-sharing - I just think it would be a mistake to do that as a priority while we have ~0 test coverage of critical systems like Android platform views.

(If anything, replacing the device-lab specific code seems like a no-brainer, since it's not even part of a production tool)

In other words, I'd say the priorities roughly are:

We can take and compare screenshots of Android platform views on CI, and the tests are stable and useful

We de-duplicate code in dev/devicelab to help prove out this approach is good beyond the prototype tests

We do a proof of concept of an iOS backend

We de-duplicate code in flutter_tools

chatted offline with Matan; for #4, creating a new //flutter/packages/_flutter_common package SGTM. cc @andrewkolos

I'm wondering if a better name might be "flutter_target_control" instead of "flutter_common" (which could mean anything).

Some other potential sources of duplications are the Dart CLI tools.

We should also look at the work we need to do to modularize platform support in flutter tools. It will result in a bunch of interfaces that abstract the device/target/build system and we can use these interfaces to avoid duplicated effort.

Anyways, these cleanups are outside the scope of this PR but I agree we need to rationalize this part of the system.

See flutter/flutter#152194 where I got befuddling error messages: ```txt Unhandled exception: Null check operator used on a null value #0 DeviceLabTestOwner.getTestOwnership (package:cocoon_service/src/request_handlers/test_ownership.dart:60:59) #1 getTestOwnership (package:cocoon_service/src/request_handlers/flaky_handler_utils.dart:301:20) #2 validateOwnership (package:cocoon_service/src/foundation/utils.dart:225:27) #3 remoteCheck (file:///b/s/w/ir/x/w/cocoon/app_dart/bin/validate_task_ownership.dart:31:40) <asynchronous suspension> #4 main (file:///b/s/w/ir/x/w/cocoon/app_dart/bin/validate_task_ownership.dart:63:23) <asynchronous suspension> ```

johnmccutchan

Interface LGTM

jmagman · 2024-07-24T19:17:43Z

Your test failure is from a PR that was reverted, you can rebase onto f55ab0e

matanlurey · 2024-07-24T19:23:04Z

Filed #152257 to track a future "common" package.

…taking screenshots using `adb`. (flutter/flutter#152194)

Roll Flutter from 7d5c1c0 to 031dc3d (97 revisions) flutter/flutter@7d5c1c0...031dc3d 2024-07-26 [email protected] Roll Flutter Engine from 342a42547822 to 354abf2800a0 (7 revisions) (flutter/flutter#152385) 2024-07-26 [email protected] Use more CORS headers for flutter run server (flutter/flutter#152249) 2024-07-26 [email protected] Manual roll Flutter Engine from 8714b54a87c0 to 342a42547822 (6 revisions) (flutter/flutter#152379) 2024-07-26 [email protected] feat: Add drag handle size to be configurable based on given size (flutter/flutter#152085) 2024-07-26 [email protected] Add and use an integration test with native(ADB) screenshots (flutter/flutter#152326) 2024-07-26 [email protected] Roll Packages from 19daf6f to 3d358d9 (4 revisions) (flutter/flutter#152372) 2024-07-26 [email protected] Add test for range_slider.0.dart (flutter/flutter#152152) 2024-07-26 [email protected] Introduce `TabBar.indicatorAnimation` to customize tab indicator animation (flutter/flutter#151746) 2024-07-26 [email protected] Roll Flutter Engine from 21629ece8b72 to 8714b54a87c0 (3 revisions) (flutter/flutter#152351) 2024-07-26 [email protected] Cleanup examples/api web load logic to latest (flutter/flutter#152349) 2024-07-26 [email protected] Adds a call to the `PlatformDispatcher` whenever the focus changes (flutter/flutter#151268) 2024-07-26 [email protected] Roll Flutter Engine from d665bf82dc32 to 21629ece8b72 (4 revisions) (flutter/flutter#152344) 2024-07-25 [email protected] `docImport`s for the widgets library (flutter/flutter#152339) 2024-07-25 [email protected] Set dart defines properly while in debug mode. (flutter/flutter#152262) 2024-07-25 49699333+dependabot[bot]@users.noreply.github.com Bump github/codeql-action from 3.25.13 to 3.25.14 (flutter/flutter#152342) 2024-07-25 [email protected] Roll Flutter Engine from 74785a771ae6 to d665bf82dc32 (2 revisions) (flutter/flutter#152340) 2024-07-25 [email protected] Cleanup InputDecoration.collapsed constructor (flutter/flutter#152165) 2024-07-25 [email protected] Add test for expansion_panel_list.expansion_panel_list_radio.0_test.dart (flutter/flutter#151730) 2024-07-25 [email protected] Roll Flutter Engine from f862a620cee4 to 74785a771ae6 (2 revisions) (flutter/flutter#152333) 2024-07-25 [email protected] Roll Packages from 1c319ac to 19daf6f (3 revisions) (flutter/flutter#152327) 2024-07-25 [email protected] Add a more typical / concrete example to IntrinsicHeight / IntrinsicWidth (flutter/flutter#152246) 2024-07-25 [email protected] Roll Flutter Engine from f47b4d8e145a to f862a620cee4 (1 revision) (flutter/flutter#152320) 2024-07-25 [email protected] Roll Flutter Engine from 74737820a8ee to f47b4d8e145a (7 revisions) (flutter/flutter#152314) 2024-07-25 [email protected] Flutter Web App: adds a11y semantic attributes to slider (flutter/flutter#151985) 2024-07-25 [email protected] Manual roll Flutter Engine from eb8fac2b1703 to 74737820a8ee (8 revisions) (flutter/flutter#152305) 2024-07-25 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Roll Flutter Engine from eb8fac2b1703 to a57655cccb55 (6 revisions) (#152293)" (flutter/flutter#152304) 2024-07-25 [email protected] Roll Flutter Engine from eb8fac2b1703 to a57655cccb55 (6 revisions) (flutter/flutter#152293) 2024-07-25 [email protected] Modify stepping integration test to accommodate new DDC async semantics. (flutter/flutter#152204) 2024-07-25 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Roll Flutter Engine from eb8fac2b1703 to e1259b86ba02 (2 revisions) (#152285)" (flutter/flutter#152289) 2024-07-25 [email protected] Update fake_codec.dart to use Future.value instead of SynchronousFuture (flutter/flutter#152182) 2024-07-25 [email protected] Roll Flutter Engine from eb8fac2b1703 to e1259b86ba02 (2 revisions) (flutter/flutter#152285) 2024-07-25 [email protected] Roll Flutter Engine from 4b952093cb99 to eb8fac2b1703 (3 revisions) (flutter/flutter#152278) 2024-07-24 [email protected] [CupertinoAlertDialog] Rewrite (flutter/flutter#150410) 2024-07-24 [email protected] Revert "Make `CupertinoRadio`'s `mouseCursor` a `WidgetStateProperty`" (flutter/flutter#152254) 2024-07-24 [email protected] Fix: A selectable's selection under the active selection should not be cleared on right-click (flutter/flutter#151851) 2024-07-24 [email protected] Marks Mac channels_integration_test to be flaky (flutter/flutter#151882) 2024-07-24 [email protected] Roll Flutter Engine from c2f489d783d6 to 4b952093cb99 (3 revisions) (flutter/flutter#152264) 2024-07-24 [email protected] added Semantics label to TextField with InputDecoration to let user kâ�¦ (flutter/flutter#151996) 2024-07-24 [email protected] feat: Add alignmentOffset to DropdownMenu (flutter/flutter#151731) 2024-07-24 [email protected] Roll Flutter Engine from 490576daf810 to c2f489d783d6 (6 revisions) (flutter/flutter#152260) 2024-07-24 [email protected] Scaffolding for `NativeDriver` and `AndroidNativeDriver` for taking screenshots using `adb`. (flutter/flutter#152194) 2024-07-24 [email protected] `widgets` docImport (flutter/flutter#152146) 2024-07-24 [email protected] Roll Flutter Engine from 3078f6a90e71 to 490576daf810 (1 revision) (flutter/flutter#152239) 2024-07-24 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Use more CORS headers for `flutter run` server (#152048)" (flutter/flutter#152248) 2024-07-24 [email protected] Use more CORS headers for `flutter run` server (flutter/flutter#152048) 2024-07-24 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Enable Swift Package Manager by default on master channel (#152049)" (flutter/flutter#152243) ...

…creenshots using `adb`. (flutter#152194) Closes flutter#152189. I have next to no clue how to configure this to run on CI, so bear with me as I rediscover the wheel.

Scaffolding for NativeDriver and AndroidNativeDriver.

36c38fb

github-actions bot added a: tests "flutter test", flutter_test, or one of our tests framework flutter/packages/flutter repository. See also f: labels. labels Jul 23, 2024

matanlurey added 2 commits July 23, 2024 15:53

Try to add to CI.

2b0ca28

Uh I need an adult.

26218fc

matanlurey requested review from camsim99, johnmccutchan and tugorez July 23, 2024 23:01

matanlurey marked this pull request as ready for review July 23, 2024 23:01

christopherfujino reviewed Jul 23, 2024

View reviewed changes

jmagman reviewed Jul 23, 2024

View reviewed changes

matanlurey added 4 commits July 23, 2024 16:58

++

db9bea2

Merge remote-tracking branch 'upstream/main' into android-native-driver

966e155

Try another variant.

23cbb1e

Remove TESTOWNERS to see original error.

37aedfb

matanlurey mentioned this pull request Jul 24, 2024

Replace an NPE with a human-readable error message. flutter/cocoon#3831

Merged

matanlurey added 3 commits July 24, 2024 10:04

Tweak CI Yaml not to crash.

0d27a3a

Lets try spinning thats a good trick.

fce6f3a

Sigh.

6a346df

johnmccutchan approved these changes Jul 24, 2024

View reviewed changes

matanlurey mentioned this pull request Jul 24, 2024

Consider a flutter/flutter/packages/_flutter_common #152257

Closed

Merge branch 'master' into android-native-driver

7fd01e5

matanlurey added the autosubmit Merge PR when tree becomes green via auto submit App label Jul 24, 2024

auto-submit bot merged commit 0946192 into flutter:master Jul 24, 2024

engine-flutter-autoroll mentioned this pull request Jul 25, 2024

Roll Flutter from 7d5c1c04fb56 to 3c0302c33d10 (73 revisions) flutter/packages#7212

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 25, 2024

0946192 Scaffolding for NativeDriver and AndroidNativeDriver for …

125c178

…taking screenshots using `adb`. (flutter/flutter#152194)

engine-flutter-autoroll mentioned this pull request Jul 25, 2024

Roll Flutter from 7d5c1c04fb56 to 7d588feacde9 (74 revisions) flutter/packages#7213

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 25, 2024

0946192 Scaffolding for NativeDriver and AndroidNativeDriver for …

8992f44

…taking screenshots using `adb`. (flutter/flutter#152194)

engine-flutter-autoroll mentioned this pull request Jul 25, 2024

Roll Flutter from 7d5c1c04fb56 to 033d1adb9d62 (76 revisions) flutter/packages#7214

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 25, 2024

0946192 Scaffolding for NativeDriver and AndroidNativeDriver for …

cddb4ae

…taking screenshots using `adb`. (flutter/flutter#152194)

engine-flutter-autoroll mentioned this pull request Jul 25, 2024

Roll Flutter from 7d5c1c04fb56 to 43b653e93f35 (77 revisions) flutter/packages#7217

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 25, 2024

0946192 Scaffolding for NativeDriver and AndroidNativeDriver for …

fb9379b

…taking screenshots using `adb`. (flutter/flutter#152194)

engine-flutter-autoroll mentioned this pull request Jul 25, 2024

Roll Flutter from 7d5c1c04fb56 to 9be37535d926 (78 revisions) flutter/packages#7219

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 25, 2024

0946192 Scaffolding for NativeDriver and AndroidNativeDriver for …

98d9c01

…taking screenshots using `adb`. (flutter/flutter#152194)

engine-flutter-autoroll mentioned this pull request Jul 26, 2024

Roll Flutter from 7d5c1c04fb56 to ccd51a7f5880 (91 revisions) flutter/packages#7222

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 26, 2024

0946192 Scaffolding for NativeDriver and AndroidNativeDriver for …

a0337f0

…taking screenshots using `adb`. (flutter/flutter#152194)

engine-flutter-autoroll mentioned this pull request Jul 26, 2024

Roll Flutter from 7d5c1c04fb56 to 7f8ad8a4cb1b (93 revisions) flutter/packages#7223

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 26, 2024

0946192 Scaffolding for NativeDriver and AndroidNativeDriver for …

3e746cb

…taking screenshots using `adb`. (flutter/flutter#152194)

engine-flutter-autoroll mentioned this pull request Jul 26, 2024

Roll Flutter from 7d5c1c04fb56 to 80cc52d7d1f6 (95 revisions) flutter/packages#7224

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 26, 2024

0946192 Scaffolding for NativeDriver and AndroidNativeDriver for …

4d2087b

…taking screenshots using `adb`. (flutter/flutter#152194)

engine-flutter-autoroll mentioned this pull request Jul 26, 2024

Roll Flutter from 7d5c1c04fb56 to 031dc3d156b9 (97 revisions) flutter/packages#7225

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 26, 2024

0946192 Scaffolding for NativeDriver and AndroidNativeDriver for …

60ad1a2

…taking screenshots using `adb`. (flutter/flutter#152194)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 27, 2024

0946192 Scaffolding for NativeDriver and AndroidNativeDriver for …

63cbca0

…taking screenshots using `adb`. (flutter/flutter#152194)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 27, 2024

0946192 Scaffolding for NativeDriver and AndroidNativeDriver for …

389c9d2

…taking screenshots using `adb`. (flutter/flutter#152194)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 28, 2024

0946192 Scaffolding for NativeDriver and AndroidNativeDriver for …

541c45c

…taking screenshots using `adb`. (flutter/flutter#152194)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 28, 2024

0946192 Scaffolding for NativeDriver and AndroidNativeDriver for …

21561a3

…taking screenshots using `adb`. (flutter/flutter#152194)

	Future<List<String>> discoverDevices() async {
	final List<String> output = (await eval(adbPath, <String>['devices', '-l']))

	Future<void> shellExec(String command, List<String> arguments, { Map<String, String>? environment, bool silent = false }) async {
	await adb(<String>['shell', command, ...arguments], environment: environment, silent: silent);

	Future<void> home() async {
	await shellExec('input', const <String>['keyevent', '3']);
	}

	/// Sends `KEYCODE_POWER` (26), which causes the device to toggle its mode
	/// between awake and asleep.
	@override
	Future<void> togglePower() async {
	await shellExec('input', const <String>['keyevent', '26']);
	}

	/// Unlocks the device by sending `KEYCODE_MENU` (82).
	///
	/// This only works when the device doesn't have a secure unlock pattern.
	@override
	Future<void> unlock() async {
	await wakeUp();
	await shellExec('input', const <String>['keyevent', '82']);
	}

	@override
	Future<void> tap(int x, int y) async {
	await shellExec('input', <String>['tap', '$x', '$y']);
	}

	if (exitCode != 0 && !canFail) {
	await _flutterScreenshot(workingDirectory: workingDirectory);
	}

	Future<void> _flutterScreenshot({ String? workingDirectory }) async {
	try {
	final Directory? dumpDirectory = hostAgent.dumpDirectory;
	if (dumpDirectory == null) {
	return;
	}
	// On command failure try uploading screenshot of failing command.
	final String screenshotPath = path.join(
	dumpDirectory.path,
	'device-screenshot-${DateTime.now().toLocal().toIso8601String()}.png',
	);

	final String deviceId = (await devices.workingDevice).deviceId;
	print('Taking screenshot of working device $deviceId at $screenshotPath');
	final List<String> args = _flutterCommandArgs(
	'screenshot',
	<String>[
	'--out',
	screenshotPath,
	'-d', deviceId,
	],
	);

Scaffolding for NativeDriver and AndroidNativeDriver for taking screenshots using adb. #152194

Scaffolding for NativeDriver and AndroidNativeDriver for taking screenshots using adb. #152194

Uh oh!

Conversation

matanlurey commented Jul 23, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

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!

johnmccutchan left a comment

Choose a reason for hiding this comment

Uh oh!

jmagman commented Jul 24, 2024

Uh oh!

matanlurey commented Jul 24, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Scaffolding for `NativeDriver` and `AndroidNativeDriver` for taking screenshots using `adb`. #152194

Scaffolding for `NativeDriver` and `AndroidNativeDriver` for taking screenshots using `adb`. #152194

matanlurey commented Jul 23, 2024 •

edited

Loading