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

Screenshot taken with LiveTestWidgetsFlutterBinding is truncated #66006

Open
jiahaog opened this issue Sep 17, 2020 · 7 comments · Fixed by #88609
Open

Screenshot taken with LiveTestWidgetsFlutterBinding is truncated #66006

jiahaog opened this issue Sep 17, 2020 · 7 comments · Fixed by #88609
Labels
a: tests "flutter test", flutter_test, or one of our tests customer: google Various Google teams framework flutter/packages/flutter repository. See also f: labels. P2 Important issues not at the top of the work list team-framework Owned by Framework team triaged-framework Triaged by Framework team

Comments

@jiahaog
Copy link
Member

jiahaog commented Sep 17, 2020

Internal: b/168776765

When using LiveTestWidgetsFlutterBinding, screenshots of top level widgets are truncated.

Context: We're trying to support screenshot testing in g3 with package:integration_test, where the widgets binding inherits from LiveTestWidgetsFlutterBinding. The following is a minimal repro with just flutter test.

Steps to Reproduce

Run flutter test with the following test file:

import 'package:flutter/material.dart';
import 'package:flutter_test/flutter_test.dart';

void main() {
  LiveTestWidgetsFlutterBinding();
  testWidgets('goldens', (tester) async {
    await tester.pumpWidget(buildTestApp());
    await expectLater(
        find.byType(MaterialApp), matchesGoldenFile('screenshot.png'));
  });
}

Widget buildTestApp() {
  final app = MaterialApp(
    title: 'Test',
    home: Scaffold(
      appBar: AppBar(
        title: Text('Title'),
      ),
    ),
  );

  // screenshot is taken for _LiveTestRenderView#c6f40
  return app;

  // screenshot is taken for RenderRepaintBoundary#f72ed
  // return RepaintBoundary(child: app);
}

Expected behavior: Screenshot covers the full MaterialApp.

Similar to this image from AutomatedTestWidgetsFlutterBinding (with L5 commented out)

screenshot_automated_test_binding

Actual Image (which looks truncated)

screenshot_live_test_binding

When I added some instrumentation to the renderObject used for taking a screenshot at

return layer.toImage(renderObject.paintBounds);

The renderObject is a LiveTestRenderView: _LiveTestRenderView#c6f40.

Adding a RepaintBoundary around MaterialApp (the last second line of the snippet) fixes the issue.

LiveTestWidgetsFlutterBinding with RepaintBoundary Image

screenshot_live_test_binding_with_repaint_boundary

#12994 looks like it could be related. It is pretty unexpected that a RepaintBoundary is needed just for LiveTestWidgetsFlutterBinding, but not for the default AutomatedTestWidgetsFlutterBinding, so this could be a bug. With AutomatedTestWidgetsFlutterBinding, the renderObject used for the screenshot is RenderRepaintBoundary#1d14f.

Another thing I noticed is that if the following

: _paintMatrix = _getMatrix(size, window.devicePixelRatio, window),
is set to

_paintMatrix = _getMatrix(size, 1.0, window)

the correct bounds are used for the image, though this probably isn't the correct fix.

Doctor Output
[✓] Flutter (Channel master, 1.22.0-10.0.pre.39, on Mac OS X 10.15.3 19D76, locale en-US)
    • Flutter version 1.22.0-10.0.pre.39 at /Users/jiahaog/dev/flutter
    • Framework revision a8281e31af (2 weeks ago), 2020-09-01 10:57:59 -0700
    • Engine revision 165abef0a2
    • Dart version 2.10.0 (build 2.10.0-77.0.dev)

[✓] Android toolchain - develop for Android devices (Android SDK version 30.0.0-rc2)
    • Android SDK at /Users/jiahaog/Library/Android/sdk
    • Platform android-stable, build-tools 30.0.0-rc2
    • Java binary at: /Applications/Android Studio with Blaze.app/Contents/jre/jdk/Contents/Home/bin/java
    • Java version OpenJDK Runtime Environment (build 1.8.0_242-release-1644-b3-6222593)
    • All Android licenses accepted.

[✓] Xcode - develop for iOS and macOS (Xcode 11.3.1)
    • Xcode at /Applications/Xcode.app/Contents/Developer
    • Xcode 11.3.1, Build version 11C505
    • CocoaPods version 1.9.3

[✓] Chrome - develop for the web
    • Chrome at /Applications/Google Chrome.app/Contents/MacOS/Google Chrome

[!] Android Studio (version 4.1)
    • Android Studio at /Applications/Android Studio with Blaze.app/Contents
    ✗ Flutter plugin not installed; this adds Flutter specific functionality.
    ✗ Dart plugin not installed; this adds Dart specific functionality.
    • Java version OpenJDK Runtime Environment (build 1.8.0_242-release-1644-b3-6222593)

[✓] Android Studio
    • Android Studio at /Applications/Android Studio 3.5 Preview.app/Contents
    • Flutter plugin version 33.4.2
    • Dart plugin version 183.5901
    • Java version OpenJDK Runtime Environment (build 1.8.0_152-release-1343-b01)

[✓] VS Code (version 1.48.2)
    • VS Code at /Applications/Visual Studio Code.app/Contents
    • Flutter extension version 3.14.0

[✓] Connected device (3 available)
    • iPhone 11 (mobile) • 5D77C64F-14CB-4C4B-B47D-62443C8678ED • ios            • com.apple.CoreSimulator.SimRuntime.iOS-13-3 (simulator)
    • Web Server (web)   • web-server                           • web-javascript • Flutter Tools
    • Chrome (web)       • chrome                               • web-javascript • Google Chrome 85.0.4183.102

! Doctor found issues in 1 category.
@jiahaog jiahaog added the a: tests "flutter test", flutter_test, or one of our tests label Sep 17, 2020
@TahaTesser TahaTesser added framework flutter/packages/flutter repository. See also f: labels. passed first triage labels Sep 17, 2020
@mehmetf mehmetf added customer: google Various Google teams P1 High-priority issues at the top of the work list labels Sep 17, 2020
@goderbauer
Copy link
Member

/cc @Piinks

@dnfield
Copy link
Contributor

dnfield commented Nov 11, 2020

What if we just have the binding throw a repaint boundary around the whole view?

@jiahaog
Copy link
Member Author

jiahaog commented Nov 12, 2020

That's a great idea for a workaround! Sending #70368

@dnfield
Copy link
Contributor

dnfield commented Aug 20, 2021

The problem here is that TestViewConfiguration does not override the setting of devicePixelRatio, so it gets set to 1.0 no matter what.

@github-actions
Copy link

github-actions bot commented Sep 7, 2021

This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 7, 2021
@jiahaog jiahaog reopened this Jul 4, 2022
@flutter flutter unlocked this conversation Jul 4, 2022
@jiahaog
Copy link
Member Author

jiahaog commented Jul 4, 2022

This is roughly what happened for reference:

  1. Fix DPR in test view configuration #88609 is supposed to fix the issue and remove the workaround of adding a repaint boundary in flutter_test
  2. Failure for the above pr detected b/197699775#comment3. I don't remember the complete details but cl/392929256 was suggested as the fix but it seemed like a deeper issue with the framework so it wasn't submitted
  3. Fix DPR in test view configuration #88609 was reverted in Revert "Fix DPR in test view configuration" #88889
  4. Sometime later Does not replace the root layer unnecessarily #101748 was submitted which contains Fix DPR in test view configuration #88609 partially, but does not include the removal of the repaint boundary workaround
  5. Remove workaround since #66006 is fixed #106913 was sent to remove the workaround.
    • This is causing an internal test documented in b/168776765 to have inconsistent screenshots.
    • Note that the failure in step (2) does not appear now.

The next step would be for a Googler to debug this test and figure out why it the screenshots are inconsistent without the repaint boundary.

@flutter-triage-bot
Copy link

This issue is marked P1 but has had no recent status updates.

The P1 label indicates high-priority issues that are at the top of the work list. This is the highest priority level a bug can have if it isn't affecting a top-tier customer or breaking the build. Bugs marked P1 are generally actively being worked on unless the assignee is dealing with a P0 bug (or another P1 bug). Issues at this level should be resolved in a matter of months and should have monthly updates on GitHub.

Please consider where this bug really falls in our current priorities, and label it or assign it accordingly. This allows people to have a clearer picture of what work is actually planned. Thanks!

@goderbauer goderbauer added P2 Important issues not at the top of the work list and removed P1 High-priority issues at the top of the work list labels Dec 4, 2023
@goderbauer goderbauer added the triaged-framework Triaged by Framework team label Dec 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a: tests "flutter test", flutter_test, or one of our tests customer: google Various Google teams framework flutter/packages/flutter repository. See also f: labels. P2 Important issues not at the top of the work list team-framework Owned by Framework team triaged-framework Triaged by Framework team
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants