-
Notifications
You must be signed in to change notification settings - Fork 9.8k
Conversation
Hmm...
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM when the tests are happy.
Huh. Well, that's why I wanted to pre-check! |
@cyanglaz Are you able to reproduce this failure locally? I tried running locally with this device and it was fine. Given the failure is around looking for that exact set of coordinates, I'm wondering if it's something like the last number being slightly different on different devices; maybe we need to be rounding somewhat in the display? |
Oh, there are two failures. The platform view one seems very strange since the other tests look for it too, and those aren't failing. |
I haven't been able to repro mapClickPage( I will try again after figuring out mapCoodinatesPage). Looking at the recent rollers, I don't see macOS tests ran? https://github.com/flutter/plugins/pull/7170/checks |
https://github.com/flutter/plugins/pull/7170/checks?check_run_id=11304881293 is the run with these tests. If you were looking at the Cirrus section out of habit, the macOS host tests are all LUCI now :) |
(I'm also trying flutter/packages#3199 to see if I can easily get this off the critical path of repo migration.) |
For the scrolling problem. I saw that the map doesn't have a gesture recognizer. We need to give the google map a gesture recognizer for it to take touch events from the list view. This is a bug from the example app, ill create a PR to fix it. (I'm not sure how it has been passing CI, it is concerning) |
Can you try to patch |
testMapClickPage passed (was it a flake?, we need to revisit the test to make it non-flaky.) Actually, for testMapCoordinatesPage I read through the test more carefully, it looks like we do want to drag the map and not wanting the gesture to move the map's visible region :( sorry. @jmagman Could you please confirm since you wrote the test? :) So we actually do not want the gesture, please revert my patch :) |
The issue with testMapCoordinatesPage is that the text region was scrolled out of the scene, it is probably hard to control for different devives. @jmagman could you please review this patch to confirm it is ok for the test? |
|
For |
Changelog override: the change here is only interesting for integration tests, so is effectively dev-only. |
I'm going to re-run tests that seem to be flake; since this is repo-merge blocking I'd like to get it landed if it fixes the non-flake issue. |
The Android failure here is the thing that was reverted just after this PR was forked. Since this PR can't possible affect Android and the failure is already fixed on master, I'm going to just land manually instead of syncing the PR and re-running every test just for that. |
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!). 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. |
* cd09d9d31 [ci] Update iOS simulator (flutter/plugins#7131) * 016c3b7f1 Roll Flutter from df41e58 to 22e17bb (28 revisions) (flutter/plugins#7186) * 7160f55e8 [ios_platform_images] Update minimum version to iOS 11 (flutter/plugins#6874) * ea048a249 [in_app_purchase] Update minimum Flutter version to 3.3 and iOS 11 (flutter/plugins#6873) * 530442456 [google_sign_in_web] Migrate to the GIS SDK. (flutter/plugins#6921) * 9a3a77e6c [image_picker] Fix images changing to incorrect orientation (flutter/plugins#7187) * 8f3419be5 Roll Flutter from 22e17bb to 298d8c7 (20 revisions) (flutter/plugins#7190)
Updates the iOS simulator used in CI from an iPhone 11 to an iPhone 13.
Part of alignment with flutter/packages in preparation for merging repositories.
Updates a Maps integration test for issues with the newer device.