Closed Bug 770456 Opened 12 years ago Closed 12 years ago

Intermittent Robocop | testHistoryTab | Exception caught - junit.framework.AssertionFailedError: View is null and can therefore not be clicked!

Categories

(Firefox for Android Graveyard :: General, defect)

Product:
Firefox for Android Graveyard
Component:
General
Platform:
ARM
Android
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(firefox16 fixed)

Status:
RESOLVED FIXED
Milestone:
Firefox 17
Tracking Flags:
Tracking Status
firefox16 --- fixed

People

(Reporter: emorley, Assigned: gbrown)

References

Details

(Keywords: intermittent-failure)

Attachments

(2 files, 1 obsolete file)

wait for child view to be populated
4.38 KB, patch
wesj
: review+
lsblakk
: approval-mozilla-aurora+
Details | Diff | Splinter Review
use more care when accessing history child view
5.92 KB, patch
gbrown
: review+
Details | Diff | Splinter Review 
Android Tegra 250 mozilla-inbound opt test robocop on 2012-07-02 15:05:00 PDT for push 8c50b2ddc155

slave: tegra-094

https://tbpl.mozilla.org/php/getParsedLog.php?id=13172075&tree=Mozilla-Inbound

{
0 INFO SimpleTest START
1 INFO TEST-START | testHistoryTab
2 INFO TEST-PASS | testHistoryTab | Awesomebar URL typed properly - http://mochi.test:8888/tests/robocop/robocop_big_link.html should equal http://mochi.test:8888/tests/robocop/robocop_big_link.html
3 INFO TEST-PASS | testHistoryTab | Awesomebar URL typed properly - http://mochi.test:8888/tests/robocop/robocop_blank_01.html should equal http://mochi.test:8888/tests/robocop/robocop_blank_01.html
4 INFO TEST-PASS | testHistoryTab | bookmark added sucessfully - true should equal true
5 INFO TEST-PASS | testHistoryTab | checking that history list exists - android.widget.ExpandableListView@443fdaf8
6 INFO TEST-PASS | testHistoryTab | history list has 3 children - 3 should equal 3
7 INFO TEST-PASS | testHistoryTab | TextView is filled in - Big Link
8 INFO TEST-PASS | testHistoryTab | TextView is filled in - http://mochi.test:8888/tests/robocop/robocop_big_link.html
9 INFO TEST-PASS | testHistoryTab | Correct number of ImageViews visible - 1 should equal 1
10 INFO TEST-PASS | testHistoryTab | TextView is filled in - Browser Blank Page 01
11 INFO TEST-PASS | testHistoryTab | TextView is filled in - http://mochi.test:8888/tests/robocop/robocop_blank_01.html
12 INFO TEST-PASS | testHistoryTab | Correct number of ImageViews visible - 2 should equal 2
13 INFO TEST-PASS | testHistoryTab | First row has Today header - Today
14 INFO TEST-PASS | testHistoryTab | Correct number of ImageViews visible - 0 should equal 0
15 INFO TEST-PASS | testHistoryTab | Header rows should not show a context menu - false should equal false
Exception caught during test!
junit.framework.AssertionFailedError: View is null and can therefore not be clicked!
	at junit.framework.Assert.fail(Assert.java:47)
	at junit.framework.Assert.assertTrue(Assert.java:20)
	at com.jayway.android.robotium.solo.Clicker.clickOnScreen(Clicker.java:144)
	at com.jayway.android.robotium.solo.Solo.clickLongOnView(Solo.java:797)
	at org.mozilla.fennec.tests.testHistoryTab.testContextMenu(testHistoryTab.java:125)
	at org.mozilla.fennec.tests.testHistoryTab.testHistoryTab(testHistoryTab.java:58)
	at java.lang.reflect.Method.invokeNative(Native Method)
	at java.lang.reflect.Method.invoke(Method.java:521)
	at android.test.InstrumentationTestCase.runMethod(InstrumentationTestCase.java:204)
	at android.test.InstrumentationTestCase.runTest(InstrumentationTestCase.java:194)
	at android.test.ActivityInstrumentationTestCase2.runTest(ActivityInstrumentationTestCase2.java:186)
	at org.mozilla.fennec.tests.BaseTest.runTest(BaseTest.java:93)
	at junit.framework.TestCase.runBare(TestCase.java:127)
	at junit.framework.TestResult$1.protect(TestResult.java:106)
	at junit.framework.TestResult.runProtected(TestResult.java:124)
	at junit.framework.TestResult.run(TestResult.java:109)
	at junit.framework.TestCase.run(TestCase.java:118)
	at android.test.AndroidTestRunner.runTest(AndroidTestRunner.java:169)
	at android.test.AndroidTestRunner.runTest(AndroidTestRunner.java:154)
	at android.test.InstrumentationTestRunner.onStart(InstrumentationTestRunner.java:520)
	at android.app.Instrumentation$InstrumentationThread.run(Instrumentation.java:1447)
16 INFO TEST-UNEXPECTED-FAIL | testHistoryTab | Exception caught - junit.framework.AssertionFailedError: View is null and can therefore not be clicked!
}
Assignee: nobody → gbrown
I tested this quite a bit and found that in all the occasional failures, the view was non-null if we simply checked again a little while later (say, 100 ms). The patch uses waitForTest to check for the problematic views periodically for up to 2000 ms. I also added code to send BACK to clear the VKB and ensure that the entire list is visible -- we are doing this in other tests now, as it has been a source of intermittent failures.
Attachment #641914 - Flags: review?(wjohnston)
Attachment #641914 - Flags: review?(wjohnston) → review+
https://hg.mozilla.org/mozilla-central/rev/c4f26b2a26c4
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 17
Please may this be backported to aurora? :-)
Comment on attachment 641914 [details] [diff] [review]
wait for child view to be populated

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Race condition in Robocop test.
User impact if declined: None -- only affects a test case. Expect intermittent Robocop orange on aurora if declined.
Testing completed (on m-c, etc.): On m-c since 2012-07-18.
Risk to taking this patch (and alternatives if risky): Minor risk of making the test behavior worse.
String or UUID changes made by this patch: none
Attachment #641914 - Flags: approval-mozilla-aurora?
Comment on attachment 641914 [details] [diff] [review]
wait for child view to be populated

test-only, fix intermittent orange - approving.
Attachment #641914 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
I was able to reproduce the problem on try, with additional diagnostics: https://tbpl.mozilla.org/?tree=Try&rev=0609ae830d45. Note that all the runs are green, but some would have been failures, except for the addition of automatic re-try code.

What I am seeing is that the history item view that we are trying to access is available in the waitForTest() call, but then sometimes cannot be retrieved a moment later, just after the waitForTest(); if the test waits just a little more and then re-tries, the view is available again -- and it's the same one we found during waitForTest. 

It seems to me that we ought to be able to simply click on the view we find during waitForTest -- I'm testing that approach now.
As discussed in comment 37, the failures seem to arise after finding the correct view in the waitForTest, because the subsequent re-retrieval fails. This patch avoids the re-retrieval -- we just use the view we find in the waitForTest. Try run looks good: https://tbpl.mozilla.org/?tree=Try&rev=7e67b2f8a3cc
Attachment #654647 - Flags: review?(jmaher)
Comment on attachment 654647 [details] [diff] [review]
use more care when accessing history child view

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

just the one concern below, otherwise this looks good.  Any chance other tests could benefit from this?

::: mobile/android/base/tests/testHistoryTab.java.in
@@ +159,5 @@
> +        if (!success) {
> +            mAsserter.ok(success != false, "waiting for history item", "history item available");
> +        }
> +        mAsserter.dumpLog("clickLongOnView: "+mFirstChild);
> +        mSolo.clickLongOnView(mFirstChild);

what happens if mFirstChild is still null?  Would this be better suited in an else clause?
Attachment #654647 - Flags: review?(jmaher) → review+
(In reply to Joel Maher (:jmaher) from comment #43)
> just the one concern below, otherwise this looks good.  Any chance other
> tests could benefit from this?

I checked all the other uses of waitForTest and couldn't find any instances where I could apply the same technique.
 
> what happens if mFirstChild is still null?  Would this be better suited in
> an else clause?

It's certainly easier to read as an else clause -- I've made this change.

r=jmaher carried.
Attachment #654647 - Attachment is obsolete: true
Attachment #655232 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/010b43686a5f
Status: REOPENED → RESOLVED
Closed: 12 years ago → 12 years ago
Resolution: --- → FIXED
Whiteboard: [orange]
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: