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

Fix disabled toggleSearchSuggestions UI test #21517

Merged
merged 1 commit into from
Sep 28, 2021

Conversation

AndiAJ
Copy link
Collaborator

@AndiAJ AndiAJ commented Sep 27, 2021

For #479
✔️ Successfully ran 50x on Firebase

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

@AndiAJ AndiAJ self-assigned this Sep 27, 2021
@AndiAJ AndiAJ added the eng:ui-test UI Tests label Sep 27, 2021
@AndiAJ AndiAJ changed the title Potential fix for toggleSearchSuggestions UI test Fix disabled toggleSearchSuggestions UI test Sep 27, 2021
@AndiAJ AndiAJ marked this pull request as ready for review September 27, 2021 14:40
@AndiAJ AndiAJ requested review from a team as code owners September 27, 2021 14:40
Copy link
Contributor

@AaronMT AaronMT left a comment

Choose a reason for hiding this comment

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

See comment below. Can you also run this one a number of times after the change?

private fun assertSearchEngineSuggestionResults(rule: ComposeTestRule, searchResult: String) {
rule.waitForIdle()

mDevice.waitForAwesomeBarContent(
Copy link
Contributor

@AaronMT AaronMT Sep 27, 2021

Choose a reason for hiding this comment

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

I'm surprised to see this as part of an extension and not a robot. Can you refactor this out of WaitNotNull.kt? I don't think it makes sense to live in this extension https://github.com/mozilla-mobile/fenix/blob/main/app/src/androidTest/java/org/mozilla/fenix/helpers/ext/WaitNotNull.kt#L23 Or if you don't think it should be part of a robot it can become a generic function in https://github.com/mozilla-mobile/fenix/blob/main/app/src/androidTest/java/org/mozilla/fenix/helpers/TestHelper.kt since the function can handle other objects since it's merely waiting for them – it can be reused.

@AndiAJ AndiAJ requested a review from a team as a code owner September 27, 2021 15:24
@AndiAJ
Copy link
Collaborator Author

AndiAJ commented Sep 27, 2021

See comment below. Can you also run this one a number of times after the change?

Hi @AaronMT moved the function to TestHelper.kt

Successfully re-ran both toggleSearchSuggestions and shortcutButtonTest for 50x

Results

matrix result logs details
matrix-1dhte7f0xk2i6 success logs 2 test cases passed
matrix-okkxu1rrs7ufa success logs 2 test cases passed
matrix-pyxyg4hylacya success logs 2 test cases passed
matrix-hfv1o02pbuu7a success logs 2 test cases passed
matrix-5sx4sob6b6r3a success logs 2 test cases passed
matrix-27flxdesiibd1 success logs 2 test cases passed
matrix-2ebbs03sq6dsh success logs 2 test cases passed
matrix-1c5h1s67405v3 success logs 2 test cases passed
matrix-3igwc0mp5u8we success logs 2 test cases passed
matrix-2zuwsik1hpifc success logs 2 test cases passed
matrix-sxb664ibfvdsa success logs 2 test cases passed
matrix-1r6dxtvnnn3oa success logs 2 test cases passed
matrix-1tlf2tt8qo77f success logs 2 test cases passed
matrix-3ktpcto1v8a95 success logs 2 test cases passed
matrix-1zdcw54sqo5ls success logs 2 test cases passed
matrix-37s4n7slgkzn9 success logs 2 test cases passed
matrix-wq2fzjdcxfhqa success logs 2 test cases passed
matrix-1bokwhpezzav9 success logs 2 test cases passed
matrix-3ut2vn455pl78 success logs 2 test cases passed
matrix-31pexuu3f419g success logs 2 test cases passed
matrix-3g1bb2zmqdg7i success logs 2 test cases passed
matrix-1rk2do1uieto4 success logs 2 test cases passed
matrix-8e54kqkx9hrna success logs 2 test cases passed
matrix-13ma1e2fklzwr success logs 2 test cases passed
matrix-2umqslxyp66jr success logs 2 test cases passed
matrix-296wfgogkpine success logs 2 test cases passed
matrix-1r1t2iwe05ehx success logs 2 test cases passed
matrix-ub3xyp4s1swha success logs 2 test cases passed
matrix-3ft3mp2uo6u8x success logs 2 test cases passed
matrix-1qzzgco9a2o5g success logs 2 test cases passed
matrix-f4n6el8s5kmia success logs 2 test cases passed
matrix-adkgvdyw2tfoa success logs 2 test cases passed
matrix-3k36jt6y66z74 success logs 2 test cases passed
matrix-1dbocd9wa9i3r success logs 2 test cases passed
matrix-3cxirdp5l4gra success logs 2 test cases passed
matrix-2o4jgy1qeh9x0 success logs 2 test cases passed
matrix-2qbxgtusq2qj5 success logs 2 test cases passed
matrix-3148yilhszlia success logs 2 test cases passed
matrix-3gu8g571dvdy4 success logs 2 test cases passed
matrix-12d34oen0ueyd success logs 2 test cases passed
matrix-27y0h6elv7ik9 success logs 2 test cases passed
matrix-1wmey8edao1rz success logs 2 test cases passed
matrix-l6fhjmbwegxpa success logs 2 test cases passed
matrix-olej5qjb0asca success logs 2 test cases passed
matrix-3d97ecla0ysiv success logs 2 test cases passed
matrix-2rf9datg4uw69 success logs 2 test cases passed
matrix-1mltemqfgjr4l success logs 2 test cases passed
matrix-3v9trhzclcvyq success logs 2 test cases passed
matrix-6g09kukc8a3la success logs 2 test cases passed
matrix-md06anei29jva success logs 2 test cases passed

Copy link
Contributor

@AaronMT AaronMT left a comment

Choose a reason for hiding this comment

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

Looks OK to me. Let's try it.

@AaronMT
Copy link
Contributor

AaronMT commented Sep 27, 2021

@AndiAJ Oh this needs a rebase due to your other commit. Feel free to land after.

@AndiAJ AndiAJ removed the pr:needs-landing PRs that are ready to land [Will be merged by Mergify] label Sep 28, 2021
@AndiAJ AndiAJ added the pr:needs-landing PRs that are ready to land [Will be merged by Mergify] label Sep 28, 2021
@mergify mergify bot merged commit 06292ac into mozilla-mobile:main Sep 28, 2021
@AndiAJ AndiAJ deleted the toggleSearchSuggestions branch September 28, 2021 07:50
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
eng:ui-test UI Tests pr:needs-landing PRs that are ready to land [Will be merged by Mergify]
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants