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

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

Merged
merged 6 commits into from
Jan 4, 2024

Conversation

bparrishMines
Copy link
Contributor

@bparrishMines bparrishMines commented Jan 4, 2024

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
Copy link
Contributor

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

Copy link
Contributor

@stuartmorgan stuartmorgan 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.

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 warning: land on red to fix tree breakage 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
79 of 80 checks passed
@bparrishMines bparrishMines deleted the file_selector_int_test branch January 5, 2024 02:33
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 5, 2024
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Jan 5, 2024
flutter/packages@31fc7b5...b9b6d38

2024-01-05 engine-flutter-autoroll@skia.org Manual roll Flutter from 11def8e to cc40425 (118 revisions) (flutter/packages#5806)
2024-01-05 magder@google.com [ci] Run 'flutter build --config-only for iOS and macOS during fetch deps (flutter/packages#5804)
2024-01-05 amirpanahandeh@yahoo.com [image_picker] Remove input element after completion (flutter/packages#5654)
2024-01-05 stuartmorgan@google.com [video_player] Fix initial frame on macOS (flutter/packages#5781)
2024-01-05 tarrinneal@gmail.com [pigeon] java non null void (flutter/packages#5786)
2024-01-04 10687576+bparrishMines@users.noreply.github.com [file_selector_android] Attempt to close system dialogs before integration tests run (flutter/packages#5805)
2024-01-04 stuartmorgan@google.com [tool] Handle Flutter dev dependencies (flutter/packages#5775)
2024-01-04 40719830+Alex-Usmanov@users.noreply.github.com [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 flutter-ecosystem@google.com,rmistry@google.com 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 added a commit that referenced this pull request Jan 12, 2024
@reidbaker reidbaker mentioned this pull request Jan 12, 2024
reidbaker added a commit that referenced this pull request Jan 12, 2024
…e integration tests run (#5805)"

This reverts commit c6b86c5.

Touched to retrigger testing
reidbaker added a commit that referenced this pull request Jan 12, 2024
…e integration tests run (#5805)"

This reverts commit c6b86c5.

Touched to retrigger testing 2
reidbaker added a commit that referenced this pull request Jan 12, 2024
…e integration tests run (#5805)"

This reverts commit c6b86c5.

Touched to retrigger testing 3
arc-yong pushed a commit to Arctuition/packages-arc that referenced this pull request Jun 14, 2024
…ation tests run (flutter#5805)

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

- [x] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [x] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [x] 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`.)
- [x] I signed the [CLA].
- [x] 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].
- [x] I updated/added relevant documentation (doc comments with `///`).
- [x] I added new tests to check the change I am making, or this PR is
[test-exempt].
- [x] All existing and new tests are passing.

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

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/packages/blob/main/CONTRIBUTING.md
[Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene
[relevant style guides]:
https://github.com/flutter/packages/blob/main/CONTRIBUTING.md#style
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes
[Discord]: https://github.com/flutter/flutter/wiki/Chat
[pub versioning philosophy]: https://dart.dev/tools/pub/versioning
[exempt from version changes]:
https://github.com/flutter/flutter/wiki/Contributing-to-Plugins-and-Packages#version-and-changelog-updates
[following repository CHANGELOG style]:
https://github.com/flutter/flutter/wiki/Contributing-to-Plugins-and-Packages#changelog-style
[test-exempt]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
autosubmit Merge PR when tree becomes green via auto submit App p: file_selector platform-android warning: land on red to fix tree breakage Override tree-status signal (land even with closed tree), combine with the autosubmit label.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants