Closed Bug 770456 Opened 13 years ago Closed 13 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: 13 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: 13 years ago → 13 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: