8000 Precise browser resizing with integration_test and driver by munrocket · Pull Request #160678 · flutter/flutter · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Precise browser resizing with integration_test and driver #160678

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 12 commits into
base: master
Choose a base branch
from
Open

Precise browser resizing with integration_test and driver #160678

wants to merge 12 commits into from

Conversation

munrocket
Copy link
@munrocket munrocket commented Dec 20, 2024 •
edited
Loading

Fixes #136109 P2
Related #148028 P1

Roadmap:

This PR:

  • Fixes Chrome bug from dart side
  • Adds pixelRatio mobile emulation in web
  • Adds new notation: 393,852 -> 393×852[@1]
  • Leaves previous behavior as it was, so 393,852 will give wrong size until Chrome will fix it. But you can use 393×852@1.

@github-actions github-actions bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Dec 20, 2024
@munrocket munrocket mentioned this pull request Dec 20, 2024
Closed
@munrocket munrocket mentioned this pull request Dec 22, 2024
Open
2 tasks
@munrocket munrocket marked this pull request as ready for review December 22, 2024 10:51
@flutter-dashboard
Copy link

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group.

@munrocket munrocket mentioned this pull request Dec 23, 2024
Closed
@munrocket
Copy link
Author
munrocket commented Dec 23, 2024 •
edited
Loading

This PR is ready for review. Analyze broken because tree is red.

@jmagman
Copy link
Member
jmagman commented Dec 26, 2024

The analysis error is

 Found 1 Dart file which was formatted incorrectly.
 To fix, run `dart format packages/flutter_tools/lib/src/drive/web_driver_service.dart` or:
 git apply <<DONE
 diff --git a/packages/flutter_tools/lib/src/drive/web_driver_service.dart b/packages/flutter_tools/lib/src/drive/web_driver_service.dart
 index 02cbe7a84e..0000000000 100644
 --- a/packages/flutter_tools/lib/src/drive/web_driver_service.dart
 +++ b/packages/flutter_tools/lib/src/drive/web_driver_service.dart
 @@ -178,7 +178,8 @@
8000
 class WebDriverService extends DriverService {
                'height': height,
                'pixelRatio': double.parse(browserDimension[2]),
              },
 -            'userAgent': 'Mozilla/5.0 (Linux; Android 15) AppleWebKit/537.36 '
 +            'userAgent':
 +                'Mozilla/5.0 (Linux; Android 15) AppleWebKit/537.36 '
                  '(KHTML, like Gecko) Chrome/131.0.6778.200 Mobile Safari/537.36',
            };
          }
 @@ -382,3 +383,4 @@ Map<String, dynamic> getDesiredCapabilities(
      },
    },
  };
 DONE

https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8727677075114199649/+/u/run_test.dart_for_analyze_shard_and_subshard_None/stdout

@flutter-dashboard
Copy link

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 #160678 at sha bdbf90d

@flutter-dashboard flutter-dashboard bot added the will affect goldens Changes to golden files label Dec 27, 2024
@munrocket
8000
Copy link
Author
munrocket commented Dec 27, 2024 •
edited
Loading

@jmagman thanks for answer, I have little progress.

It’s not obvious how to find proper build, because there are no link from github and searching by PR/hash is off, only by test name.

@almassolarenrgi almassolarenrgi mentioned this pull request Dec 27, 2024
Open
5 tasks
@flutter-dashboard
Copy link

Golden file changes are available for triage from new commit, Click here to view.

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 #160678 at sha 47f3e0c

@munrocket
Copy link
Author

It was not easy, but CI passed. Ready for review.

@jmagman
Copy link
Member
jmagman commented Dec 27, 2024

It was not easy, but CI passed. Ready for review.

Thank you for figuring it all out! We're a bit slower to review around the end of the year due to holidays, the web or tool team will take a look during their regularly scheduled triage.
cc @yjbanov

@bkonyi bkonyi requested review from yjbanov and bkonyi January 9, 2025 21:10
@bkonyi
Copy link
Contributor
bkonyi commented Jan 16, 2025

Hi @munrocket, just checking in. Do you have plans to pick this PR back up?

@munrocket
Copy link
Author

Sorry, didn't notice updates here.

@munrocket
Copy link
Author

We also can force mobileEmulation in any situation, because it magically works.
But in this case we probably need to update some goldens.

@bkonyi
Copy link
Contributor
bkonyi commented Jan 30, 2025

@yjbanov do you have any more comments / concerns?

@bkonyi
Copy link
Contributor
bkonyi commented Feb 13, 2025

Friendly ping @yjbanov

@flutter-dashboard
Copy link

This pull request executed golden file tests, but it has not been updated in a while (20+ days). Test results from Gold expire after as many days, so this pull request will need to be updated with a fresh commit in order to get results from Gold.

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.

@jBorkowska
Copy link

Any ETA on this? Just wondering because I really would like to use it in my project :)

@alexobviously
Copy link

+1 this bug is blocking something significant for me too :)

@mdebbar
Copy link
Contributor
mdebbar commented Apr 29, 2025
6D40

Looks good to me, thanks for the contribution!

I hit "Update branch" to bring the PR up to date. Let's see if CI is happy or not.

@mdebbar
Copy link
Contributor
mdebbar commented Apr 29, 2025

I think we need a 2nd review to land this PR. @bkonyi do you wanna take a look?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yjbanov yjbanov yjbanov left review comments

@EatonWu EatonWu EatonWu left review comments

@mdebbar mdebbar mdebbar approved these changes

@bkonyi bkonyi Awaiting requested review from bkonyi

Assignees
No one assigned
Labels
tool Affects the "flutter" command-line tool. See also t: labels. will affect goldens Changes to golden files
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Flutter drive --browser-dimension not working
8 participants
@munrocket @jmagman @bkonyi @jBorkowska @alexobviously @mdebbar @yjbanov @EatonWu
0