FNX-14440 ⁃ For #112,#145: Verify Search engine can be changed temporarily using Search engine #13259

TejaswiKarasani · 2020-08-04T14:21:22Z

This PR contains the UI test according these steps in Test Rail

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.

After merge

Milestone: Make sure issues finished by this pull request are added to the milestone of the version currently in development.

To download an APK when reviewing a PR:

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

codecov-commenter · 2020-08-04T14:36:59Z

Codecov Report

Merging #13259 into master will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master   #13259   +/-   ##
=========================================
  Coverage     29.57%   29.57%           
  Complexity     1111     1111           
=========================================
  Files           421      421           
  Lines         17163    17163           
  Branches       2229     2229           
=========================================
  Hits           5076     5076           
  Misses        11713    11713           
  Partials        374      374

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4b85f3e...ae78ecb. Read the comment docs.

isabelrios · 2020-08-17T11:10:49Z

app/src/androidTest/java/org/mozilla/fenix/ui/robots/SearchRobot.kt

+
+private fun assertSearchEngineList() {
+    scrollToElementByText("Google")
+    onView(withText("Google"))


Remove all these scrollToElementByText. Instead on line 233 add onView(withId(R.id.mozac_browser_toolbar_edit_icon)).click()
Thay way the keyboard will be dismissed and we can check all the elements on the screen without having to scroll.

isabelrios · 2020-08-17T11:12:29Z

app/src/androidTest/java/org/mozilla/fenix/ui/robots/SearchRobot.kt

+        .check(matches(withEffectiveVisibility(Visibility.VISIBLE)))
+
+private fun selectDefaultSearchEngine(searchEngine: String) {
+    onView(withText(searchEngine))


To be sure we can select each searchEngine in the list, let's dismiss the keyboard first by doing:
onView(withId(R.id.mozac_browser_toolbar_edit_icon)).click()
I have found many issues when running the test to be able to click on Twitter or the next search Engine in the list. With this line we are sure all the search engines in the list are visible

(This would go in the previous line to this one, just below the method definition)

isabelrios · 2020-08-17T11:22:04Z

app/src/androidTest/java/org/mozilla/fenix/ui/SmokeTest.kt

+        }.goToSearchEngine {
+        }.enterURLAndEnterToBrowser(defaultWebPage.url) {
+        }.openTabDrawer {
+


Nit: remove line

isabelrios · 2020-08-17T11:22:15Z

app/src/androidTest/java/org/mozilla/fenix/ui/SmokeTest.kt

+        }.goToSearchEngine {
+        }.enterURLAndEnterToBrowser(defaultWebPage.url) {
+        }.openTabDrawer {
+


Nit: same as above

isabelrios · 2020-08-17T11:23:11Z

app/src/androidTest/java/org/mozilla/fenix/ui/SmokeTest.kt

+        }.goToSearchEngine {
+        }.enterURLAndEnterToBrowser(defaultWebPage.url) {
+        }.openTabDrawer {
+        }.openHomeScreen {


I will add a comment here, Changing to default search engine, or something similar

isabelrios · 2020-08-17T11:25:44Z

app/src/androidTest/java/org/mozilla/fenix/ui/SmokeTest.kt

+        }.openHomeScreen {
+        }.openSearch {
+            clickSearchEngineButton()
+            scrollToElementByText("Wikipedia")


No need to this scroll if you do what I suggest in the search robot to avoid issues with the scroll view

isabelrios · 2020-08-17T11:37:27Z

app/src/androidTest/java/org/mozilla/fenix/ui/robots/SearchRobot.kt

+    fun verifyKeyboardVisibility() = assertKeyboardVisibility(isExpectedToBeVisible = true)
+    fun verifySearchEngineList() = assertSearchEngineList()
+    fun verifySearchEngineIcon(expectedText: String) = assertSearchEngineIconsList(expectedText)
+    fun verifySearchEngineIconsList() {


I would remove this check in favor or a modified verifySearchIcon. What we want to be sure is that when the search engine is changed, that the new search is going to use that one. So, when we want to check that would be at this point:

I would change the verifySearchEngineIcon like:

fun verifySearchEngineIcon(expectedText: String) { onView(withContentDescription(expectedText)) }

And I would add a call to that function after changing the search engine each time...as commented in the smoketest file

isabelrios

The test is looking really good now. There are a few changes needed in order to have the test working on firebase and with a cleaner code. Hope the changes suggested make sense, let me know otherwise or any question you may have.

isabelrios · 2020-08-17T11:38:23Z

app/src/androidTest/java/org/mozilla/fenix/ui/SmokeTest.kt

+            clickSearchEngineButton()
+            verifySearchEngineList()
+            verifySearchEngineIconsList()
+            changeDefaultSearchEngine("Amazon.com")


After this line we will need to check that the searc icon is the correct one for that search engine. It would be good to add this check:
verifySearchEngineIconsList()

after chaning the search engine each time.

…e changed temporarily using Search engine

isabelrios

LGTM! squash the commits and that would be it!
(Test passed all the 25 times it run)
Great work! thanks!

TejaswiKarasani requested review from isabelrios and rpappalax August 4, 2020 14:21

data-sync-user changed the title ~~For #112,#145: Verify Search engine can be changed temporarily using Search engine~~ FNX-14440 ⁃ For #112,#145: Verify Search engine can be changed temporarily using Search engine Aug 11, 2020

TejaswiKarasani force-pushed the temporaryEngine branch 2 times, most recently from 4ac84ba to a09fb02 Compare August 17, 2020 08:09

isabelrios reviewed Aug 17, 2020

View reviewed changes

isabelrios suggested changes Aug 17, 2020

View reviewed changes

TejaswiKarasani force-pushed the temporaryEngine branch from c6cbf91 to 0e1f1f4 Compare August 18, 2020 15:01

For mozilla-mobile#112,mozilla-mobile#145: Verify Search engine can b…

ae78ecb

…e changed temporarily using Search engine

isabelrios approved these changes Aug 18, 2020

View reviewed changes

TejaswiKarasani force-pushed the temporaryEngine branch from 0e1f1f4 to ae78ecb Compare August 18, 2020 15:40

TejaswiKarasani marked this pull request as ready for review August 18, 2020 16:22

rpappalax merged commit cd729b3 into mozilla-mobile:master Aug 18, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

FNX-14440 ⁃ For #112,#145: Verify Search engine can be changed temporarily using Search engine #13259

FNX-14440 ⁃ For #112,#145: Verify Search engine can be changed temporarily using Search engine #13259

TejaswiKarasani commented Aug 4, 2020 •

edited by data-sync-user

Loading

codecov-commenter commented Aug 4, 2020 •

edited

Loading

isabelrios Aug 17, 2020

isabelrios Aug 17, 2020

isabelrios Aug 17, 2020

isabelrios Aug 17, 2020

isabelrios Aug 17, 2020

isabelrios Aug 17, 2020

isabelrios Aug 17, 2020

isabelrios Aug 17, 2020

isabelrios left a comment

isabelrios Aug 17, 2020

isabelrios left a comment

FNX-14440 ⁃ For #112,#145: Verify Search engine can be changed temporarily using Search engine #13259

FNX-14440 ⁃ For #112,#145: Verify Search engine can be changed temporarily using Search engine #13259

Conversation

TejaswiKarasani commented Aug 4, 2020 • edited by data-sync-user Loading

Pull Request checklist

After merge

To download an APK when reviewing a PR:

codecov-commenter commented Aug 4, 2020 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

isabelrios left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

isabelrios left a comment

Choose a reason for hiding this comment

TejaswiKarasani commented Aug 4, 2020 •

edited by data-sync-user

Loading

codecov-commenter commented Aug 4, 2020 •

edited

Loading