Closes #6031 - Wrap waits with assert check; adjust timer check

[AaronMT]

See #6031

• Adjust waits with assert check
• Adjusts timer to speed things up
• Test fix and cleanup

[codecov-io]

Codecov Report

❗ No coverage uploaded for pull request base (master@40cda1d). Click here to learn what that means.
The diff coverage is n/a.

@@           Coverage Diff            @@
##             master   #6084   +/-   ##
========================================
  Coverage          ?   14.3%           
  Complexity        ?     326           
========================================
  Files             ?     261           
  Lines             ?   10756           
  Branches          ?    1572           
========================================
  Hits              ?    1539           
  Misses            ?    9090           
  Partials          ?     127

---

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 40cda1d...1cbf1a4. Read the comment docs.

[rpappalax]

@AaronMT is there a reason why the mDevice.waits were removed? In general, I don't think any of these were added randomly. Each one of them was originally added afaik to address timing issues

[AaronMT]

@AaronMT is there a reason why the mDevice.waits were removed? In general, I don't think any of these were added randomly. Each one of them was originally added afaik to address timing issues

I didn't think they were necessary (one in particular above was failing a test) (aside it doesn't make sense to have them in the test but rather should be in the robot)

[nojunpark]

@AaronMT So let's work on fixing intermittents on this PR until we make a one clean pass. In this run, only the shortcutButtonTest failed, and when I looked at it, it has a good reason why it failed.
after clickDuckDuckGoResult(), we don't wait for anything before we call verifyDuckDuckGoURL(). The DOM tree at the time of the failure shows that it was still in the search view, it didn't even open the web view at the time. I think within verifyDuckDuckGoURL(), you have to put a wait.Until() to wait for the url text.

[severinrudie]

It occurs to me that we could make this easier to reuse with an extension function. Something like the following:

    /**
     * Blocks the test for [waitTime] miliseconds before continuing.
     *
     * Will cause the test to fail is the condition is not met before the timeout.
     */
    fun UiDevice.waitNotNull(
        searchCondition: SearchCondition<*>,
        waitTime: Long = TestAssetHelper.waitingTimeShort
    ) = assertNotNull(wait(searchCondition, waitTime))

Which would update L113 to look like mDevice.waitNotNull(Until.findObjects(By.text("Sign in"))), or (if we wanted to use a different timeout) mDevice.waitNotNull(Until.findObjects(By.text("Sign in")), TestAssetHelper.waitingTimeShort)

[nojunpark]

^ @rpappalax fyi

[AaronMT]

We can open a new issue.

[AaronMT]

🍏 green @npark-mozilla (quick re-review)

[nojunpark]

LGTM!

[nojunpark]

@AaronMT let's see how this one goes. :) and also let's create an issue based on Severin's suggestion and see how that goes as well, this might refactor our tests better.