Closed
Bug 769919
Opened 11 years ago
Closed 10 years ago
Intermittent testBookmark | bookmarks list has 4 children (the default bookmarks) - got 5 (or 0), expected 4
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
Firefox 16
People
(Reporter: RyanVM, Assigned: gbrown)
References
Details
(Keywords: intermittent-failure)
Attachments
(5 files, 3 obsolete files)
3.99 KB,
patch
|
jmaher
:
review+
|
Details | Diff | Splinter Review |
1.62 KB,
patch
|
jmaher
:
review+
|
Details | Diff | Splinter Review |
2.41 KB,
patch
|
gbrown
:
review+
|
Details | Diff | Splinter Review |
15.42 KB,
patch
|
gbrown
:
review+
akeybl
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
23.33 KB,
patch
|
jmaher
:
review+
|
Details | Diff | Splinter Review |
+++ This bug was initially created as a clone of Bug #730129 +++ https://tbpl.mozilla.org/php/getParsedLog.php?id=13126997&tree=Mozilla-Inbound Android Tegra 250 mozilla-inbound opt test robocop on 2012-06-30 00:49:58 PDT for push 37776fc33437 0 INFO SimpleTest START 1 INFO TEST-START | testBookmark 2 INFO TEST-PASS | testBookmark | checking that bookmarks list exists - bookmarks list exists 3 INFO TEST-UNEXPECTED-FAIL | testBookmark | bookmarks list has 4 children (the default bookmarks) - got 5, expected 4 Exception caught during test! junit.framework.AssertionFailedError: 3 INFO TEST-UNEXPECTED-FAIL | testBookmark | bookmarks list has 4 children (the default bookmarks) - got 5, expected 4 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.FennecMochitestAssert.is(FennecMochitestAssert.java:143) at org.mozilla.fennec.tests.testBookmark.runAwesomeScreenTest(testBookmark.java:86) at org.mozilla.fennec.tests.testBookmark.testBookmark(testBookmark.java:54) 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) 4 INFO TEST-UNEXPECTED-FAIL | testBookmark | Exception caught - junit.framework.AssertionFailedError: 3 INFO TEST-UNEXPECTED-FAIL | testBookmark | bookmarks list has 4 children (the default bookmarks) - got 5, expected 4 5 INFO TEST-END | testBookmark | finished in 16501ms 6 INFO TEST-START | Shutdown 7 INFO Passed: 1 8 INFO Failed: 2 9 INFO Todo: 0 10 INFO SimpleTest FINISHED
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment 8•11 years ago
|
||
Is this intermittent? I'm worried it was caused by bug 759041, but I thought I fixed it (on try) before I pushed.
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment 10•11 years ago
|
||
It's intermittent, maybe only 50% of the time that robocop actually manages to run, though that can be more than 50% of the time that this test runs since robocop will pretend green while skipping tests.
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
![]() |
Assignee | |
Comment 27•11 years ago
|
||
(In reply to Phil Ringnalda (:philor) from comment #10) > It's intermittent, maybe only 50% of the time that robocop actually manages > to run, though that can be more than 50% of the time that this test runs > since robocop will pretend green while skipping tests. Agreed - it is intermittent. There are many failures, and some passes where testBookmark did not run, or failed silently. There are also some real passes, such as: https://tbpl.mozilla.org/php/getParsedLog.php?id=13196066&tree=Mozilla-Inbound&full=1
![]() |
Assignee | |
Comment 28•11 years ago
|
||
Running locally on a Galaxy S, the test fails consistently - got 4, expected 5 children. Of note, after the test runs, if I start Fennec and check the bookmarks, the new (5th) bookmark exists. I tried inserting a long sleep between the bookmark creation and the check -- test continues to fail. Also, the new bookmark is clearly visible on screen before the check...very strange.
Comment hidden (Legacy TBPL/Treeherder Robot) |
![]() |
Assignee | |
Updated•11 years ago
|
Assignee: nobody → gbrown
![]() |
Assignee | |
Comment 30•11 years ago
|
||
The failing test is calling ListView.getChildCount() (ViewGroup.getChildCount()); there are some Android q/a articles that caution that this returns the number of *visible* items in the list. Sure enough, on my small-screen phone, only 4 items are visible, the 5th being (just barely!) obscured by the VKB. If I clear the VKB with a Back key before testing, getChildCount() returns 5.
![]() |
Assignee | |
Comment 31•11 years ago
|
||
The new testBookmarksTab has a very similar test, and it also fails consistently on my Galaxy S. On tbpl, testBookmarksTab usually (always?) passes when testBookmark passes and usually (always?) fails silently when testBookmark fails.
Comment hidden (Legacy TBPL/Treeherder Robot) |
![]() |
Assignee | |
Comment 33•11 years ago
|
||
5 robocop runs at https://tbpl.mozilla.org/?tree=Try&rev=b8006c6ddbd3 - 4 successful, complete runs of testBookmark - 4 successful, complete runs of testBookmarksTab - 1 unexplained short run of testBookmarksTab (crash, hang?) which left a bookmark which caused a failure in the subsequent testBookmark
Attachment #639070 -
Flags: review?(jmaher)
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment 37•11 years ago
|
||
Comment on attachment 639070 [details] [diff] [review] use BACK key to clear VKB so that bookmark list is not obscured Review of attachment 639070 [details] [diff] [review]: ----------------------------------------------------------------- This looks harmless and probably the right thing to do. I wonder if we should do this in other tests as well?
Attachment #639070 -
Flags: review?(jmaher) → review+
![]() |
Assignee | |
Comment 38•11 years ago
|
||
There are getChildCount() calls in testAllPagesTab and testHistoryTab as well, but I don't see those failing (oddly)...I'll keep an eye on those. https://hg.mozilla.org/integration/mozilla-inbound/rev/05500de40104
Reporter | ||
Comment 39•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/05500de40104
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Updated•11 years ago
|
Target Milestone: --- → Firefox 16
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Updated•11 years ago
|
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
![]() |
Assignee | |
Comment 47•11 years ago
|
||
In some of these, at least: https://tbpl.mozilla.org/php/getParsedLog.php?id=13776399&tree=Mozilla-Inbound https://tbpl.mozilla.org/php/getParsedLog.php?id=13787859&tree=Profiling https://tbpl.mozilla.org/php/getParsedLog.php?id=13811155&tree=Firefox there is an undetected failure in testBookmarksTab, which runs earlier. I think testBookmarksTab is failing to clean up, resulting in the testBookmark failure. 42 INFO TEST-PASS | testBookmarksTab | TextView is filled in - Bookmarks Toolbar 43 INFO TEST-PASS | testBookmarksTab | Correct number of ImageViews visible - 0 should equal 0 44 INFO TEST-PASS | testBookmarksTab | Correct number of TextViews visible - 1 should equal 1 45 INFO TEST-PASS | testBookmarksTab | Back moved up one level - 46 INFO TEST-PASS | testBookmarksTab | Back moved up one level - INFO | automation.py | Application ran for: 0:17:29.931003 INFO | automation.py | Reading PID log: /tmp/tmpV6zDIBpidlog getting files in '/mnt/sdcard/tests/profile/minidumps/' WARNING | automationutils.processLeakLog() | refcount logging is off, so leaks can't be detected!
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
![]() |
Assignee | |
Comment 50•11 years ago
|
||
I cannot think of what might be going wrong here, and I have had no luck reproducing this problem locally or on try. I am adding diagnostics in this patch in hopes that seeing the extra bookmark URL will point to a specific problem/solution. I intend to [leave open] and remove the extra logging once a solution is in place.
Attachment #650276 -
Flags: review?(jmaher)
Comment 51•11 years ago
|
||
Comment on attachment 650276 [details] [diff] [review] diagnostic patch Review of attachment 650276 [details] [diff] [review]: ----------------------------------------------------------------- cool!
Attachment #650276 -
Flags: review?(jmaher) → review+
Reporter | ||
Updated•11 years ago
|
Whiteboard: [orange] → [orange][leave open]
![]() |
Assignee | |
Comment 52•11 years ago
|
||
diagnostic patch: https://hg.mozilla.org/integration/mozilla-inbound/rev/aaac2eaf07f2
Comment hidden (Legacy TBPL/Treeherder Robot) |
Reporter | ||
Comment 54•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/aaac2eaf07f2
Updated•10 years ago
|
Keywords: intermittent-failure
Comment hidden (Legacy TBPL/Treeherder Robot) |
Updated•10 years ago
|
Whiteboard: [orange][leave open] → [leave open]
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment 57•10 years ago
|
||
I can reproduce this consistently. getChildCount() returns the number of visible views in the list, but we actually want to check the number of total views in the adapter.
Blocks: 798683
Comment 58•10 years ago
|
||
Assignee: gbrown → bnicholson
Attachment #689379 -
Flags: review?(gbrown)
Comment 59•10 years ago
|
||
Comment on attachment 689379 [details] [diff] [review] Check adapter count instead of visible list child count in testAllPagesTab Whoops, wrong bug - this one's about testBookmark.
Attachment #689379 -
Attachment is obsolete: true
Attachment #689379 -
Flags: review?(gbrown)
Comment 60•10 years ago
|
||
I can reproduce this one easily too if I run testBookmark with my phone in landscape orientation. This should fix the tests by using the adapter count rather than visible item count. This still uses the visible item count when iterating through the list, so we only check items that are visible. Ideally, we'd scroll the lists to verify every view rather than just the ones that are visible, but this should be enough to just fix the failed tests for now.
Attachment #689398 -
Flags: review?(gbrown)
![]() |
Assignee | |
Comment 61•10 years ago
|
||
Comment on attachment 689398 [details] [diff] [review] Check adapter count instead of visible list child count in testBookmark Review of attachment 689398 [details] [diff] [review]: ----------------------------------------------------------------- I think this is a good change, and will help us to run reliably on a wider range of devices / orientations - thanks! I am not sure that this will reduce the intermittent failures on tegras. I ran many testBookmark tests on try with and without this patch; unfortunately, I couldn't repro the failure in either case! https://tbpl.mozilla.org/?tree=Try&rev=7f3e3c166fa2 https://tbpl.mozilla.org/?tree=Try&rev=02e59f1b29a5
Attachment #689398 -
Flags: review?(gbrown) → review+
Comment 62•10 years ago
|
||
Let's see what happens! http://hg.mozilla.org/integration/mozilla-inbound/rev/7686d9b787e1
![]() |
Assignee | |
Comment 64•10 years ago
|
||
testBookmark was disabled and fails almost consistently when enabled now. The main problem is that the BACK key sent to clear the VKB is no longer necessary - see bug 808408. I have a patch under test.
See Also: → 808408
![]() |
Assignee | |
Updated•10 years ago
|
Assignee: bnicholson → gbrown
![]() |
Assignee | |
Comment 65•10 years ago
|
||
The BACK key is no longer necessary; it was causing all the failures. I also made some additional changes to make this test more like the now-reliable testBookmarksTab. Multiple try runs show testBookmark passing consistently. I will mark bug 817339 as a dup of this one as it seems to be addressed as well.
Attachment #696156 -
Flags: review?(jmaher)
![]() |
Assignee | |
Comment 66•10 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=d32c17f3516d
Comment 67•10 years ago
|
||
Comment on attachment 696156 [details] [diff] [review] do not send BACK to clear VKB and check for list before continuing Review of attachment 696156 [details] [diff] [review]: ----------------------------------------------------------------- Thanks! just a couple nits. ::: mobile/android/base/tests/testBookmark.java.in @@ -80,5 @@ > // Open the bookmark list and check the root folder view > ListView bookmarksList = openBookmarksList(); > > - // Clear VKB so that list is not obscured > - mActions.sendSpecialKey(Actions.SpecialKey.BACK); we need to check other test cases that do this. I recall reviewing one recently. @@ +156,5 @@ > } > } > > private ListView openBookmarksList() { > + final int WAIT_FOR_CHILD_TIMEOUT = 2000; I thought this was at the baseTest level?
Attachment #696156 -
Flags: review?(jmaher) → review+
![]() |
Assignee | |
Comment 68•10 years ago
|
||
Updated for review comments. r+ carried. >::: mobile/android/base/tests/testBookmark.java.in >@@ -80,5 @@ >> // Open the bookmark list and check the root folder view >> ListView bookmarksList = openBookmarksList(); >> >> - // Clear VKB so that list is not obscured >> - mActions.sendSpecialKey(Actions.SpecialKey.BACK); > we need to check other test cases that do this. I recall reviewing one > recently. I only found testHistoryTab. BACK is used in various other places, but generally not to clear the VKB. >> + final int WAIT_FOR_CHILD_TIMEOUT = 2000; > > I thought this was at the baseTest level? Not really...but I thought that was a good idea, so I combined WAIT_FOR_CHILD_TIMEOUT and MAX_WAIT_MS, and made MAX_WAIT_MS public in BaseTest -- much neater. Thanks.
Attachment #696156 -
Attachment is obsolete: true
Attachment #696421 -
Flags: review+
![]() |
Assignee | |
Comment 69•10 years ago
|
||
New try run: https://tbpl.mozilla.org/?tree=Try&rev=abdc5fccf45a https://hg.mozilla.org/integration/mozilla-inbound/rev/9fbba2a7caa1
Whiteboard: [leave open]
Comment 71•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/9fbba2a7caa1
Status: REOPENED → RESOLVED
Closed: 11 years ago → 10 years ago
Resolution: --- → FIXED
Comment 72•10 years ago
|
||
Comment on attachment 696421 [details] [diff] [review] do not send BACK to clear VKB and check for list before continuing [Approval Request Comment] Bug caused by (feature/regressing bug #): this is required to push bug 808408, which is a regression. Sorry I forgot about need it. User impact if declined: Keyboard doesn't disappear when scrolling through panes in awesomescreen Testing completed (on m-c, etc.): landed on mc awhile ago Risk to taking this patch (and alternatives if risky): Low risk, test only String or UUID changes made by this patch: none
Attachment #696421 -
Flags: approval-mozilla-aurora?
Comment 73•10 years ago
|
||
Comment on attachment 696421 [details] [diff] [review] do not send BACK to clear VKB and check for list before continuing [Triage Comment] FF19 is now on beta, so approving for that channel.
Attachment #696421 -
Flags: approval-mozilla-aurora? → approval-mozilla-beta+
Updated•10 years ago
|
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment hidden (Legacy TBPL/Treeherder Robot) |
![]() |
Assignee | |
Comment 75•10 years ago
|
||
From Comment 74: 01-29 22:22:17.117 D/GeckoApp( 3137): State - 983041 01-29 22:22:17.117 D/GeckoApp( 3137): Got a document start event. 01-29 22:22:17.207 I/GeckoApp( 3137): Security Mode - unknown 01-29 22:22:17.267 I/ActivityManager( 1020): Displayed activity org.mozilla.fennec/org.mozilla.gecko.AwesomeBar: 432 ms (total 432 ms) 01-29 22:22:17.297 D/GeckoApp( 3137): State - 786448 01-29 22:22:17.297 D/GeckoApp( 3137): Got a document stop event. 01-29 22:22:17.297 I/ActivityManager( 1020): Start proc org.mozilla.f3nn3c.UpdateService for service org.mozilla.fennec/org.mozilla.gecko.updater.UpdateService: pid=3219 uid=10033 gids={3003, 1015, 1006} 01-29 22:22:17.307 I/CompositorParent( 3137): Unable to renew compositor surface; remaining in paused state 01-29 22:22:17.317 I/GeckoDisplayPortCalculator( 3137): Set strategy VelocityBiasStrategy mult=2.0, threshold=5.1200004, reverse=0.2, dangerBaseX=1.0, dangerBaseY=1.0, dangerIncrX=0.0, dangerIncrY=0.0 01-29 22:22:17.447 D/GeckoAppShell( 3137): Gecko event sync took 292 ms 01-29 22:22:17.457 I/GeckoToolbar( 3137): zerdatime 875068 - Throbber stop 01-29 22:22:17.467 D/GeckoFavicons( 3137): Requesting cancelation of favicon load (3) 01-29 22:22:17.467 D/GeckoFavicons( 3137): Cancelling favicon load (3) 01-29 22:22:17.477 I/GeckoToolbar( 3137): zerdatime 875080 - Throbber stop 01-29 22:22:17.637 I/UpdateService( 3219): next update will be at: Wed Jan 30 22:14:31 GMT+00:00 2013 01-29 22:22:18.127 I/ALL_PAGES( 3137): Got cursor in 1118ms 01-29 22:22:19.629 D/GeckoJarReader( 3137): Jar reader reached end of stack 01-29 22:22:22.967 I/Robocop ( 3137): 2 INFO TEST-PASS | testBookmark | checking that bookmarks list exists - bookmarks list exists 01-29 22:22:22.977 I/Robocop ( 3137): 3 INFO TEST-UNEXPECTED-FAIL | testBookmark | bookmarks list has 4 children (the default bookmarks) - got 0, expected 4
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Reporter | ||
Updated•10 years ago
|
Summary: Intermittent testBookmark | bookmarks list has 4 children (the default bookmarks) - got 5, expected 4 → Intermittent testBookmark | bookmarks list has 4 children (the default bookmarks) - got 5 (or 0), expected 4
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
![]() |
Assignee | |
Comment 97•10 years ago
|
||
Diagnostic try run at https://tbpl.mozilla.org/?tree=Try&rev=435a512ed89f
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
![]() |
Assignee | |
Comment 147•10 years ago
|
||
These intermittent failures on top sites/bookmarks/history lists have plagued us for too long! I cannot reproduce this particular bug locally, and on try it seems to get harder and harder to reproduce as I add more useful logging. I have found though that most (if not all) errors are transient: If we query the list again a moment later, all is well. My approach in this patch then is to retry -- up to 3 times -- if the list we get is not what we expect. Sometimes it's not clear what to expect -- how many items should be in the list at that point in time. In those cases I pass -1 as the expected number of items, and that check is skipped. I may be able to reduce the number of these cases by analyzing those tests better -- I'm trying not to get bogged down by details here. Try run is promising: https://tbpl.mozilla.org/?tree=Try&rev=332c74f4652d&noignore=1 -- and that is with testAllPagesTab enabled as well!
Attachment #719511 -
Flags: review?(jmaher)
Comment 148•10 years ago
|
||
Comment on attachment 719511 [details] [diff] [review] add retry logic to getHistory/Bookmark/AllPagesList Review of attachment 719511 [details] [diff] [review]: ----------------------------------------------------------------- seems like this is a major change, but mostly due to the number of files. I think we can clean up more around what this touches as indicated in my detailed comments below. ::: mobile/android/base/tests/BaseTest.java.in @@ +413,2 @@ > */ > + protected ListView getAllPagesList(String waitText, int expectedChildCount) { can we default this to -1, that is the majority of the values we pass in for this value (on all 3 functions) @@ +509,2 @@ > return null; > } can we make a common function to do this: protected ListView getList(String waitText, int exptectedChildCount, String clickText, String tagText, String listName) { <insert getHistoryList with hard coded strings matching variables;> } protected ListView getHistoryList(String waitText, int exptectedChildCount) { return getList(waitText, expectedChildCount, "History", "history", "getHistoryList") } ::: mobile/android/base/tests/testBookmarklets.java.in @@ +47,5 @@ > > Boolean found = false; > if (bookmarks == null) { > mAsserter.is(true, true, "Did not find the bookmarks section in the awesomebar"); > } else if (mSolo.waitForText(title)) { do we still need this waitForText? ::: mobile/android/base/tests/testBookmarksTab.java.in @@ +52,4 @@ > mAsserter.isnot(list, null, "checking that bookmarks list exists"); > > int count = list.getAdapter().getCount(); > mAsserter.is(count, 5, "bookmarks list has 5 children (defaults + a folder)"); do we need this assertion anymore? We sort of do that by default in the new getBookmarksList() ::: mobile/android/base/tests/testClearPrivateData.java.in @@ +50,1 @@ > mAsserter.ok(hList.getAdapter().getCount() == 0,"checking history is cleared ","history is cleared"); do we need the assertion for getCount() == 0 still? ::: mobile/android/base/tests/testHistoryTab.java.in @@ +103,2 @@ > final int count = listview.getAdapter().getCount(); > mAsserter.is(count, 3, "history list has 3 children"); do we need this assertion for count == 3 still?
Attachment #719511 -
Flags: review?(jmaher) → review-
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
![]() |
Assignee | |
Comment 161•10 years ago
|
||
Updated to address review comments.
Attachment #719511 -
Attachment is obsolete: true
Attachment #719981 -
Flags: review?(jmaher)
Comment 162•10 years ago
|
||
Comment on attachment 719981 [details] [diff] [review] add retry logic to getHistory/Bookmark/AllPagesList Review of attachment 719981 [details] [diff] [review]: ----------------------------------------------------------------- thanks!
Attachment #719981 -
Flags: review?(jmaher) → review+
Comment hidden (Legacy TBPL/Treeherder Robot) |
![]() |
Assignee | |
Comment 164•10 years ago
|
||
New try run: https://tbpl.mozilla.org/?tree=Try&rev=e50d62f8de40&noignore=1
![]() |
Assignee | |
Comment 165•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/7f88297be027
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment 168•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/7f88297be027
Status: REOPENED → RESOLVED
Closed: 10 years ago → 10 years ago
Resolution: --- → FIXED
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Updated•2 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•