Skip to content
This repository has been archived by the owner on Feb 20, 2023. It is now read-only.

For #15624 & #17484: improves tab drawer locator & avoids homescreen refreshing #22133

Merged
merged 1 commit into from
Oct 29, 2021
Merged

For #15624 & #17484: improves tab drawer locator & avoids homescreen refreshing #22133

merged 1 commit into from
Oct 29, 2021

Conversation

sv-ohorvath
Copy link
Contributor

For #15624 & #17484 UI tests.
I disabled the recent tabs and Pocket sections in these tests because they were refreshing in the background, and I think it interfered with the test's ability to locate the Undo snackbar.
Tested on Firebase 300 times and all passed.

Pull Request checklist

  • Tests: This PR includes thorough tests or an explanation of why it does not
  • Screenshots: This PR includes screenshots or GIFs of the changes made or an explanation of why it does not
  • Accessibility: The code in this PR follows accessibility best practices or does not include any user facing features. In addition, it includes a screenshot of a successful accessibility scan to ensure no new defects are added to the product.

To download an APK when reviewing a PR:

  1. click on Show All Checks,
  2. click Details next to "Taskcluster (pull_request)" after it appears and then finishes with a green checkmark,
  3. click on the "Fenix - assemble" task, then click "Run Artifacts".
  4. the APK links should be on the left side of the screen, named for each CPU architecture

@sv-ohorvath sv-ohorvath requested review from a team as code owners October 25, 2021 14:31
@sv-ohorvath sv-ohorvath self-assigned this Oct 25, 2021
@sv-ohorvath sv-ohorvath added eng:ui-test UI Tests needs:review PRs that need to be reviewed labels Oct 25, 2021
This was linked to issues Oct 25, 2021
Intermittent UI test failure - <TabbedBrowsingTest.closePrivateTabTest> #15624
Closed
Intermittent UI test failure - TabbedBrowsingTest.closeTabTest #17484
Closed
AaronMT
AaronMT suggested changes Oct 25, 2021
View reviewed changes
app/src/androidTest/java/org/mozilla/fenix/ui/TabbedBrowsingTest.kt Outdated
// saving default values of feature flags
private var jumpBackInCFRDefault: Boolean = settings.shouldShowJumpBackInCFR
private var recentTabsFeatureDefault: Boolean = settings.showRecentTabsFeature
private var showPocketDefault: Boolean = settings.showPocketRecommendationsFeature
Copy link
Contributor

@AaronMT AaronMT Oct 25, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Since this is the first time I think we've interacted with feature flags, I think we move this logic out of the test and create a new basic test helper class file FeatureFlagHelper and use that for basic setting of the flags. We should append the helper with some of the active features https://github.com/mozilla-mobile/fenix/blob/main/app/src/main/java/org/mozilla/fenix/FeatureFlags.kt and then in the teardown we can have a resetAllFeatureFlagDefaults() call or something

@sv-ohorvath what do you think?

Copy link
Contributor Author

@sv-ohorvath sv-ohorvath Oct 26, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

sounds good to me. I'll see what I can do

@sv-ohorvath sv-ohorvath requested a review from a team as a code owner October 26, 2021 12:49
@sv-ohorvath
Copy link
Contributor Author

sv-ohorvath commented Oct 26, 2021 •
edited
Loading

@AaronMT After making the requested changes, I re-ran the tests just in case and saw the snackbar button issue is not fixed actually, it appeared again: Firebase log of failure.
I'll switch this to draft and keep working on it.

@sv-ohorvath sv-ohorvath marked this pull request as draft October 26, 2021 13:36
@sv-ohorvath sv-ohorvath added pr:work-in-progress PRs that are not ready to be reviewed yet and are actively being worked on and removed needs:review PRs that need to be reviewed labels Oct 26, 2021
@sv-ohorvath sv-ohorvath force-pushed the oana-closeTab branch 3 times, most recently from d22d0ef to 78b927d Compare October 28, 2021 13:06
@sv-ohorvath sv-ohorvath added needs:review PRs that need to be reviewed and removed pr:work-in-progress PRs that are not ready to be reviewed yet and are actively being worked on labels Oct 28, 2021
@sv-ohorvath sv-ohorvath marked this pull request as ready for review October 28, 2021 13:07
@sv-ohorvath sv-ohorvath requested a review from AaronMT October 28, 2021 13:07
@sv-ohorvath
Copy link
Contributor Author

After running the tests hundreds of times, the snackbar button visibility might still be an issue that is not clear to me how to fix. Also, there might be a bug where you click the undo button and the tab is not restored, which is not reproducible manually.
I had to make some bigger changes and I've moved the problematic steps into 2 new tests (the snackbar tests) so we can handle them separately if needed and won't block larger tests.

@sv-ohorvath sv-ohorvath reopened this Oct 28, 2021
AaronMT
AaronMT approved these changes Oct 28, 2021
View reviewed changes
app/src/androidTest/java/org/mozilla/fenix/ui/robots/BrowserRobot.kt
@@ -546,7 +550,10 @@ class BrowserRobot {
}

fun openTabDrawer(interact: TabDrawerRobot.() -> Unit): TabDrawerRobot.Transition {
mDevice.waitNotNull(Until.findObject(By.desc("Tabs")))
mDevice.findObject(
UiSelector().descriptionContains("open tab. Tap to switch tabs.")
Copy link
Contributor

@AaronMT AaronMT Oct 28, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Is there no id for this one? (Just want to try and limit our hard coded English strings), otherwise look's good to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It did use an ID before, but it was flaky. Not sure why, but text worked better.

@sv-ohorvath sv-ohorvath added pr:needs-landing PRs that are ready to land [Will be merged by Mergify] and removed needs:review PRs that need to be reviewed labels Oct 28, 2021
@sv-ohorvath sv-ohorvath force-pushed the oana-closeTab branch 3 times, most recently from f7ac36f to ea3f1db Compare October 29, 2021 08:27
@sv-ohorvath sv-ohorvath reopened this Oct 29, 2021
@sv-ohorvath sv-ohorvath merged commit f6b83e3 into main Oct 29, 2021
@bors bors bot deleted the oana-closeTab branch October 29, 2021 13:13
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@AaronMT AaronMT AaronMT approved these changes

@isabelrios isabelrios Awaiting requested review from isabelrios

Assignees

@sv-ohorvath sv-ohorvath

Labels
eng:ui-test UI Tests pr:needs-landing PRs that are ready to land [Will be merged by Mergify]
Projects
None yet
Milestone
No milestone
2 participants
@sv-ohorvath @AaronMT