Skip to content

[file_selector_android] Attempt to close system dialogs before integration tests run #5805

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
Merged
merged 6 commits into from
Jan 4, 2024
Merged

[file_selector_android] Attempt to close system dialogs before integration tests run #5805

merged 6 commits into from
Jan 4, 2024

Conversation

bparrishMines
Copy link
Contributor

@bparrishMines bparrishMines commented Jan 4, 2024 •
edited
Loading

Attempt to close any system dialogs before running the test. This idea came from https://stackoverflow.com/questions/39457305/android-testing-waited-for-the-root-of-the-view-hierarchy-to-have-window-focus.

There may be a system dialog before the test runs that prevents the FlutterView from getting focus. Which also prevents any of the espresso actions to run

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 relevant style guides and ran the auto-formatter. (Unlike the flutter/flutter repo, the flutter/packages repo does use dart format.)
  • I signed the CLA.
  • The title of the PR starts with the name of the package surrounded by square brackets, e.g. [shared_preferences]
  • I listed at least one issue that this PR fixes in the description above.
  • I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.
  • I updated CHANGELOG.md to add a description of the change, following repository CHANGELOG style.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this 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.

@bparrishMines bparrishMines changed the title try to close dialogs before tests [file_selector_android] Attempt to close system dialogs before integration tests run Jan 4, 2024
@stuartmorgan-g
Copy link
Contributor

Can we temporarily add the roll into this PR, to test that it actually fixes the issue we're having?

stuartmorgan-g
stuartmorgan-g reviewed Jan 4, 2024
View reviewed changes
Copy link
Contributor

@stuartmorgan-g stuartmorgan-g left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes LGTM assuming they bear out in tests.

If this is it, we should also file an infra issue about the fact that the Android emulators suddenly started failing tests due to the presence of a system dialog, since that seems like a fundamental issue that we shouldn't have to work around in each test.

@bparrishMines
Copy link
Contributor Author

Changes LGTM assuming they bear out in tests.

If this is it, we should also file an infra issue about the fact that the Android emulators suddenly started failing tests due to the presence of a system dialog, since that seems like a fundamental issue that we shouldn't have to work around in each test.

I'm not sure if this is a problem that can be fixed by our infra. The stackoverflow link shows that it is also a problem with non-Flutter Android integration tests. It may just be related to Android 34 emulators since I don't remember it being flaky before.

@bparrishMines bparrishMines mentioned this pull request Jan 4, 2024
Closed
@bparrishMines bparrishMines added the autosubmit Merge PR when tree becomes green via auto submit App label Jan 4, 2024
@bparrishMines bparrishMines mentioned this pull request Jan 4, 2024
Closed
tarrinneal
tarrinneal approved these changes Jan 4, 2024
View reviewed changes
Copy link
Contributor

@tarrinneal tarrinneal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

another domino falls!

@gmackall
Copy link
Member

gmackall commented Jan 4, 2024

It looks like 2 of the 4 checks the packages tree are red on are due to this error - should we land this on red or does the fix for the other flakes need to land first?

@tarrinneal tarrinneal added the emergency Override tree-status signal (land even with closed tree), combine with the autosubmit label. label Jan 4, 2024
@tarrinneal
Copy link
Contributor

merging on red to fix tree

@tarrinneal tarrinneal merged commit c6b86c5 into flutter:main Jan 4, 2024
@bparrishMines bparrishMines deleted the file_selector_int_test branch January 5, 2024 02:33
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jan 5, 2024
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 5, 2024
@engine-flutter-autoroll
c6b86c5 [file_selector_android] Attempt to close system dialogs befor…
f8feb27 
…e integration tests run (flutter/packages#5805)
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Jan 5, 2024
@engine-flutter-autoroll
Roll Packages from 31fc7b5 to b9b6d38 (8 revisions) (#141021)
1fb12a1 
flutter/packages@31fc7b5...b9b6d38

2024-01-05 [email protected] Manual roll Flutter from 11def8e to cc40425 (118 revisions) (flutter/packages#5806)
2024-01-05 [email protected] [ci] Run 'flutter build --config-only for iOS and macOS during fetch deps (flutter/packages#5804)
2024-01-05 [email protected] [image_picker] Remove input element after completion (flutter/packages#5654)
2024-01-05 [email protected] [video_player] Fix initial frame on macOS (flutter/packages#5781)
2024-01-05 [email protected] [pigeon] java non null void (flutter/packages#5786)
2024-01-04 [email protected] [file_selector_android] Attempt to close system dialogs before integration tests run (flutter/packages#5805)
2024-01-04 [email protected] [tool] Handle Flutter dev dependencies (flutter/packages#5775)
2024-01-04 [email protected] [url_launcher] Add `InAppBrowserConfiguration` parameter (flutter/packages#5758)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages-flutter-autoroll
Please CC [email protected],[email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
@reidbaker reidbaker mentioned this pull request Jan 12, 2024
Closed
reidbaker added a commit that referenced this pull request Jan 12, 2024
@reidbaker
Revert "[file_selector_android] Attempt to close system dialogs befor…
efe43db 
…e integration tests run (#5805)"

This reverts commit c6b86c5.

Touched to retrigger testing 3
@reidbaker reidbaker mentioned this pull request Jan 19, 2024
Merged
12 tasks
@reidbaker reidbaker mentioned this pull request Feb 19, 2024
Merged
16 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stuartmorgan-g stuartmorgan-g stuartmorgan-g approved these changes

@gmackall gmackall gmackall approved these changes

@tarrinneal tarrinneal tarrinneal approved these changes

Assignees
No one assigned
Labels
autosubmit Merge PR when tree becomes green via auto submit App emergency Override tree-status signal (land even with closed tree), combine with the autosubmit label. p: file_selector platform-android
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@bparrishMines @stuartmorgan-g @gmackall @tarrinneal