Android e2e screenshot

[blasten]

Implement Android view screenshot in integration_tests.

This PR adds support for the existing takeScreenshot(String name) method on Android.

List of changes:

• takeScreenshot(String name) return type is now a Future of the byte array. This works with the existing dependencies in g3. Example:

final bytes = binding.takeScreenshot('myApp');
await expectLater(bytes, matchesGoldenFile('screenshot.png'));

The byte array is also sent to the host via the driver script where it can be sent to services like Skia Gold for pixel comparisons. Here's an example:

test_driver/integration_test.dart:

  import 'dart:async';
  import 'package:integration_test/integration_test_driver.dart';
  Future<void> main() async => integrationDriver(onScreenshot: async (String name, List<int> bytes) {
     // name is "myApp" from the example above.
     // Send bytes to Skia Gold. 
  });

• Adds binding.convertFlutterSurfaceToImage() as suggested by @dnfield. This is required prior to calling takeScreenshot().

Pre-launch Checklist

• I read the Contributor Guide and followed the process outlined there for submitting PRs.
• I read the Tree Hygiene wiki page, which explains my responsibilities.
• I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
• I signed the CLA.
• I listed at least one issue that this PR fixes in the description above.
• I updated/added relevant documentation (doc comments with ///).
• I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt.
• All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

[blasten]

@jiahaog this is still WIP for #83856, but can you please confirm that this works internally?

[jiahaog]

@blasten Please correct me if I'm wrong, I think you mean for the API to be used like so:

final myApp = find.byType(MyApp);

final bytes = binding.deviceScreenshot(myApp);
await expectLater(bytes, matchesGoldenFile('screenshot.png'));

This will work internally. I'm wondering if it would make sense to contain the screenshotting logic within the matcher, for consistency with the same API as flutter test screenshots. Something like:

// [matchesDeviceScreenshotGoldenFile] provided by `package:integration_test`

await expectLater(myApp, deviceScreenshotMatchesGoldenFile('screenshot.png'));

[blasten]

@jiahaog, @dnfield

There would be two different ways to get bytes out of the device.

External users

External users will use integration_tests with the flutter drive command to pull in the screenshots from the device.
They will only need to use await binding.takeScreenshot('<screenshot-name>');.

A comparator can be added to the file under test_driver:

integrationDriver(onScreenshot: (List<int> png) { 
  /* do something with it. Like saving to disk or sending them to Skia Gold. */
});

Internal users

Since the internal infra already knows how to reach the service for image comparison. There won't be need for integrationDriver. It could just use the raw bytes from await binding.takeScreenshot('<screenshot-name>');.

final bytes = binding.deviceScreenshot(myApp);
await expectLater(bytes, anyComparator('screenshot.png'));

Any thoughts?

[jiahaog]

(for external users) Is there a mechanism for users to identify which screenshot is received? Or are users expected to maintain the ordering between the binding.takeScreenshot calls on the host and device.

I think this works, but it's also worth noting that this approach to use the driver script may be a stopgap solution in the meantime. Since #66264, users can run integration tests with flutter test integration_test, and we would eventually like to converge on an approach where the driver script is obsolete, and where the diffing logic happens entirely on the device. The model internally is that all user initiated screenshots are saved to a known directory where tooling goes to pull them to the host at the end of the tests, which can be considered in the future if / when this feature is added to the Flutter Tool.

(For the internal example) Did you mean to use binding.takeScreenshot instead? We override the goldenFileComparator global at the start of tests, so I'd expect users to use the matchesGoldenFile matcher directly:

final bytes = binding.takeScreenshot('myApp');
await expectLater(bytes, matchesGoldenFile('screenshot.png'));

The string parameter to takeScreenshot will also be unused, which is a little awkward. How about removing the parameter or changing it to take a Finder where it can be used to find the bounds and crop the screenshot to just the widget of interest?

Future<List<int>> takeScreenshot(Finder finder) {
  // Take screenshot of the dimensions bound by [finder].
}

[dnfield]

My major concern is over synchronization. Right now, as implemented, at least two frames will get pumped after requesting a screenshot, but maybe more.

How will test authors know they're getting the frame they really care about, particularly when native UI might be involved and still animating/loading?

[dnfield]

This has some pretty serious performance implications right?

IOW, doing this is incompatible with using integration_test for performance testing.

Is there some other way of doing this without impacting global test state for the rest of the test execution?

[dnfield]

So the answer seems to be yes. Why would we have this fallback though? Why not just fail if instrumentation is unavailable?

[dnfield]

Discussed offline: we should probably make this part of the integration_test API so developers can opt into this if they want. If they ahven't opted in, we should fail if instrumentation isn't available.

[dnfield]

This seems to mean that we'll pump at least two frames after calling this method, right?

How will this be stable for screenshot tests?

[dnfield]

For image views: Thsi should work because we'll know we've had at least one frame drawn by flutter.

For instrumentation tests: This should work because the Dart part of the test should have gotten us to a stable non-animating point in the application, and now should be waiting before tapping or doing anything else.

@blasten - will the new method block if awaited until the screenshot is finished?

[blasten]

Removed support for instrumentation tests until #56591 is addressed.

It rasterizes in a separate background thread, so it doesn't block the main thread.

[dnfield]

In that case, do we need to fail if !acquired?

[dnfield]

This still seems to be an issue - I'm not sure if I'm just missing somethign here, but it seems like if acquired is false we'll fail to produce a screenshot.

[blasten]

It’s recursive, so it will keep waiting.

[dnfield]

Ahh I get it now. Ok.

Will the test just timeout if they failed to convert to imageview first?

[blasten]

Right

[blasten]

@dnfield PTAL

[skia-gold]

Gold has detected about 4 new digest(s) on patchset 36.
View them at https://flutter-gold.skia.org/cl/github/84472

[skia-gold]

Gold has detected about 4 new digest(s) on patchset 37.
View them at https://flutter-gold.skia.org/cl/github/84472

[flutter-dashboard]

Golden file changes have been found for this pull request. Click here to view and triage (e.g. because this is an intentional change).

If you are still iterating on this change and are not ready to resolve the images on the Flutter Gold dashboard, consider marking this PR as a draft pull request above. You will still be able to view image results on the dashboard, commenting will be silenced, and the check will not try to resolve itself until marked ready for review.

For more guidance, visit Writing a golden file test for package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

Changes reported for pull request #84472 at sha 5a99c62