Closed Bug 770461 Opened 12 years ago Closed 12 years ago

Intermittent Robocop | testHistoryTab | Context menu has New Tab option (x2)

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

(firefox17 fixed)

Status:
RESOLVED FIXED
Milestone:
Firefox 18
Tracking Flags:
Tracking Status
firefox17 --- fixed

People

(Reporter: emorley, Assigned: gbrown)

References

Details

(Keywords: intermittent-failure)

Attachments

(3 files)

check that history list actually has text child before proceeding
2.34 KB, patch
jmaher
: review+
Details | Diff | Splinter Review
screenshot
43.08 KB, image/jpeg
Details
delay click on 'Today' until list population verified
2.29 KB, patch
jmaher
: review+
Details | Diff | Splinter Review 
Android Tegra 250 mozilla-inbound opt test robocop on 2012-07-02 15:26:18 PDT for push 64b4556e61f4

slave: tegra-066

https://tbpl.mozilla.org/php/getParsedLog.php?id=13172265&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@443bd6c8
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
16 INFO TEST-UNEXPECTED-FAIL | testHistoryTab | Context menu has New Tab option - Open in New Tab
Exception caught during test!
junit.framework.AssertionFailedError: 16 INFO TEST-UNEXPECTED-FAIL | testHistoryTab | Context menu has New Tab option - Open in New Tab
	at junit.framework.Assert.fail(Assert.java:47)
	at org.mozilla.fennec.FennecMochitestAssert._logMochitestResult(FennecMochitestAssert.java:107)
	at org.mozilla.fennec.FennecMochitestAssert.ok(FennecMochitestAssert.java:137)
	at org.mozilla.fennec.tests.testHistoryTab.testContextMenu(testHistoryTab.java:128)
	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)
17 INFO TEST-UNEXPECTED-FAIL | testHistoryTab | Exception caught - junit.framework.AssertionFailedError: 16 INFO TEST-UNEXPECTED-FAIL | testHistoryTab | Context menu has New Tab option - Open in New Tab
}
Assignee: nobody → gbrown
There is already code here waiting for the history list to be populated before proceeding to long-click on the first non-header item. I suppose the long-click could be failing for some reason, but it seems more likely that the long-click is applied to the wrong view, or that the view has not been fully populated yet.

The first child view (the one that we click on) is actually a ViewGroup - a RelativeLayout - that contains images and text. I suppose we could be clicking on the RelativeLayout before it is populated / ready.
I have expanded the existing check/wait to look deeper in the view hierarchy and ensure the ViewGroup has a child before proceeding. As well, if the ViewGroup contains a TextView (it should), this patches displays the text -- hopefully useful for debugging if this problem persists.
Attachment #651041 - Flags: review?(jmaher)
Comment on attachment 651041 [details] [diff] [review]
check that history list actually has text child before proceeding

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

::: mobile/android/base/tests/testHistoryTab.java.in
@@ +146,5 @@
> +                        if (grandChild instanceof android.widget.TextView) {
> +                            mAsserter.ok(true, "found TextView:", ((android.widget.TextView)grandChild).getText().toString());
> +                        }
> +                    }
> +                }

So this prints a list of history items?
Attachment #651041 - Flags: review?(jmaher) → review+
(In reply to Joel Maher (:jmaher) from comment #17)
> So this prints a list of history items?

It prints a list of text views (probably only 1 or 2 text views) associated with the first history item. Something like:

"Browser Blank Page 01"
"http://mochi.test:8888/ ..."
https://hg.mozilla.org/mozilla-central/rev/538c9b700037
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 17
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: Firefox 17 → ---
If anything, this seems to be occurring more frequently after the patch:
http://brasstacks.mozilla.com/orangefactor/?display=Bug&bugid=770461&entireHistory=true&tree=trunk

(Though we'd need to wait a bit longer to rule out noise entirely...)
The latest diagnostics do not indicate the cause of the problem. Consider:

16 INFO TEST-PASS | testHistoryTab | found TextView: - Browser Blank Page 01
17 INFO TEST-PASS | testHistoryTab | found TextView: - http://mochi.test:8888/tests/robocop/robocop_blank_01.html
clickLongOnView: android.widget.RelativeLayout@487948f0
18 INFO TEST-PASS | testHistoryTab | Context menu has New Tab option - Open in New Tab

vs.

16 INFO TEST-PASS | testHistoryTab | found TextView: - Browser Blank Page 01
17 INFO TEST-PASS | testHistoryTab | found TextView: - http://mochi.test:8888/tests/robocop/robocop_blank_01.html
clickLongOnView: android.widget.RelativeLayout@487385a8
18 INFO TEST-UNEXPECTED-FAIL | testHistoryTab | Context menu has New Tab option - Open in New Tab
Almost-identical code for testing the presence of "Open in New Tab" exists in testBookmarksTab and testAllPagesTab, yet neither of those tests is problematic, afaik. There are slightly different conditions for not showing a context menu from all-pages/bookmarks/history; consider http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/awesomebar/HistoryTab.java#435. (Still, nothing jumps out at me as a possible explanation for these failures.)
I reproduced this failure both on try and locally. The clickLongOnView appears to fail silently -- onCreateContextMenu is not called. I have checked the coordinates calculated by clickLongOnView, and they are correct (the same for pass and failure cases). 

I also tried increasing the click time -- eg. clickLongOnView(view, 0, 2000) -- but the failures persisted.

I also tried retrying -- clickLongOnView / waitForText / if failed, clickLongOnView -- but that fails too: Once a failure occurs, retries also fail.
I also found a possibly-related robotium issue described here: http://code.google.com/p/robotium/wiki/QuestionsAndAnswers. I tried the 2 solutions for "Why do text and button clicks go wrong?", but the failures persisted.
Attached image screenshot — 
I managed to collect a screenshot showing the device state immediately after the failure. It appears the clickLongOnView is bringing up a context menu -- just not the one we are expecting! This appears to be the same as the context menu displayed from a long-click on the awesome bar text...but I don't see how that can happen.
Next I collected screenshots *before* and after the failure. The "before" screenshot shows the expected screen state: the history tab is selected, 2 items in history under "Today" -- "Browser Blank Page 1" and "Big Link". I verified that the "before" screenshot for the failure case is identical to the before screenshot of the pass case.
Interesting! https://bugzilla.mozilla.org/show_bug.cgi?id=652168#c35 suggests the patch on that bug changes this problem from intermittent to perma-orange.
I have found that if the longClickOnView call *before* this failure is eliminated (long-click on "Today"), the entire test consistently succeeds. Screenshots show that the long-click on "Today" does not bring up a context menu, even in the failure case.
After much investigation, I still don't fully understand how this test goes wrong. The second long-click in the context menu function sometimes does not invoke the expected handlers in Fennec (onCreateContextMenu and the onTouch handler in onFinishInflate), even though all indications are that those handlers are registered, and are invoked by the first long-click ('Today') just a moment earlier. 

As mentioned earlier, the failures cease if the first long-click is eliminated. This patch seems equally effective and does not eliminate the first long-click or associated test: it just re-orders the test steps. In hindsight, the problem may have been caused by the first long-click being sent before the list is populated. Re-ordering ensures that neither long-click is sent until the list is populated.
Attachment #664589 - Flags: review?(jmaher)
Attachment #664589 - Flags: review?(jmaher) → review+
https://hg.mozilla.org/mozilla-central/rev/c5a7b7544f12
Status: REOPENED → RESOLVED
Closed: 12 years ago → 12 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → Firefox 18
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: