Closed Bug 1079693 Opened 10 years ago Closed 10 years ago

Intermittent testAddonManager | Exception caught - junit.framework.AssertionFailedError: Text string: '^Add-ons$' is not found!

Categories

(Firefox for Android Graveyard :: Add-on Manager, defect)

x86
Android
defect
Not set
normal

Tracking

(firefox34 unaffected, firefox35 fixed, firefox36 fixed, firefox-esr31 unaffected)

RESOLVED FIXED
Firefox 36
Tracking Status
firefox34 --- unaffected
firefox35 --- fixed
firefox36 --- fixed
firefox-esr31 --- unaffected

People

(Reporter: cbook, Assigned: mcomella)

References

()

Details

(Keywords: intermittent-failure)

Attachments

(1 file)

Android 2.3 Emulator b2g-inbound opt test robocop-1

https://treeherder.mozilla.org/ui/logviewer.html#?job_id=604589&repo=b2g-inbound

22:37:45 WARNING - TEST-UNEXPECTED-FAIL | testAddonManager | Exception caught - junit.framework.AssertionFailedError: Text string: '^Add-ons$' is not found!
Mike, is this yours?
Flags: needinfo?(michael.l.comella)
Off the top of my head, I can't think of anything I did that might cause this.

I noticed all the failures seem to be on 2.3.
Assignee: nobody → michael.l.comella
Status: NEW → ASSIGNED
Flags: needinfo?(michael.l.comella)
Notably missing from the failure logs is:

INFO -  waitForText timeout on ^Add-ons$

Instead, the assertion is thrown:

INFO - 0 ERROR Exception caught during test! - junit.framework.AssertionFailedError: Text string: '^Add-ons$' is not found!

I don't know why we'd throw an assertion sometimes and other times log as it's supposed to (as I see when following the source).
`grep "Text string"` points to Clicker.clickOnText which throws the assertion, meaning:

if (waitForText(itemName)) {
    mSolo.clickOnText(itemName);
} else {
    ...

`waitForText` must be returning true (instead of false) [1], but the View is not found again in `clickOnText`. 

[1]: https://mxr.mozilla.org/mozilla-central/source/mobile/android/base/tests/BaseTest.java?rev=97cea18fba09#480
It looks like `waitForText` includes non-visible views, while `clickOnText` only includes visible views.
green try run for the patch in comment 68, which fixes the issue for testAddonManager (and other tests that could fail in the same place):

https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=f73ce1cf1275

However, I don't know who calls `waitForText` looking for non-visible views, so I threw together a try push where all the `waitForText` calls acted on visible views only:

https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=1da1974b0a0b

There is a failure (just one) so I think this may be an approach worth pursuing - I filed a followup: bug 1082171.
Comment on attachment 8504297 [details] [diff] [review]
When calling waitForText in testAddonManager, only look through visible views

Review of attachment 8504297 [details] [diff] [review]:
-----------------------------------------------------------------

::: mobile/android/base/tests/BaseTest.java
@@ +386,5 @@
>  
> +    public boolean waitForText(final String text) {
> +        // false is the default value for finding only
> +        // visible views in `Solo.waitForText(String)`.
> +        return waitForText(text, false);

I'm a bit confused. Isn't true the default? http://grepcode.com/file/repo1.maven.org/maven2/com.jayway.android.robotium/robotium-solo/1.6.0/com/jayway/android/robotium/solo/RobotiumUtils.java#104
Flags: needinfo?(michael.l.comella)
Comment on attachment 8504297 [details] [diff] [review]
When calling waitForText in testAddonManager, only look through visible views

Review of attachment 8504297 [details] [diff] [review]:
-----------------------------------------------------------------

Ah, I was mixing up the scroll boolean with the onlyVisible boolean. Here's the correct link: http://grepcode.com/file/repo1.maven.org/maven2/com.jayway.android.robotium/robotium-solo/5.2.1/com/robotium/solo/Waiter.java#442

::: mobile/android/base/tests/BaseTest.java
@@ +386,5 @@
>  
> +    public boolean waitForText(final String text) {
> +        // false is the default value for finding only
> +        // visible views in `Solo.waitForText(String)`.
> +        return waitForText(text, false);

So that surprises me -- I would have expected only visible views to be checked by default. Have you tried a test run with this changed to true? I'd be surprised if we have many tests relying on text in invisible views, so checking only visible by default seems like a good way to prevent/track down similar issues elsewhere.
Attachment #8504297 - Flags: review?(bnicholson) → review+
Flags: needinfo?(michael.l.comella)
(In reply to Brian Nicholson (:bnicholson) from comment #87)
> > +        return waitForText(text, false);
> 
> So that surprises me -- I would have expected only visible views to be
> checked by default. Have you tried a test run with this changed to true?

Yes - see comment 69 and followup bug 1082171.
(In reply to Michael Comella (:mcomella) from comment #88)
> > So that surprises me -- I would have expected only visible views to be
> > checked by default. Have you tried a test run with this changed to true?
> 
> Yes - see comment 69 and followup bug 1082171.

Cool, thanks for filing and checking :)
https://hg.mozilla.org/mozilla-central/rev/8637c4d76046
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → Firefox 36
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.