Closed Bug 843947 Opened 12 years ago Closed 11 years ago

Intermittent panda testSettingsMenuItems | Waiting for General option - The General option is present

Categories

(Firefox for Android Graveyard :: General, defect)

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

Tracking

(Not tracked)

Status:
RESOLVED INCOMPLETE

People

(Reporter: gbrown, Unassigned)

References

Details

(Keywords: intermittent-failure)

Attachments

(1 file, 2 obsolete files)

testSettingsMenuItems updated
(deleted), patch
jmaher
: review-
Details | Diff | Splinter Review
https://tbpl.mozilla.org/php/getParsedLog.php?id=19964381&tree=Mozilla-Inbound&full=1 0 INFO SimpleTest START 1 INFO TEST-START | testSettingsMenuItems 2 INFO TEST-PASS | testSettingsMenuItems | Awesomebar URL stayed the same - about:home should equal about:home 3 INFO TEST-UNEXPECTED-FAIL | testSettingsMenuItems | Waiting for General option - The General option is present Exception caught during test! junit.framework.AssertionFailedError: 3 INFO TEST-UNEXPECTED-FAIL | testSettingsMenuItems | Waiting for General option - The General option is present 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:136) at org.mozilla.fennec.tests.testSettingsMenuItems.checkMenuSections(testSettingsMenuItems.java:58) at org.mozilla.fennec.tests.testSettingsMenuItems.testSettingsMenuItems(testSettingsMenuItems.java:47) at java.lang.reflect.Method.invokeNative(Native Method) at java.lang.reflect.Method.invoke(Method.java:511) at android.test.InstrumentationTestCase.runMethod(InstrumentationTestCase.java:214) at android.test.InstrumentationTestCase.runTest(InstrumentationTestCase.java:199) at android.test.ActivityInstrumentationTestCase2.runTest(ActivityInstrumentationTestCase2.java:192) at org.mozilla.fennec.tests.BaseTest.runTest(BaseTest.java:130) 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:545) at android.app.Instrumentation$InstrumentationThread.run(Instrumentation.java:1551) 4 INFO TEST-UNEXPECTED-FAIL | testSettingsMenuItems | Exception caught - junit.framework.AssertionFailedError: 3 INFO TEST-UNEXPECTED-FAIL | testSettingsMenuItems | Waiting for General option - The General option is present 5 INFO TEST-END | testSettingsMenuItems | finished in 54564ms 6 INFO TEST-START | Shutdown 7 INFO Passed: 1 8 INFO Failed: 2 9 INFO Todo: 0 10 INFO SimpleTest FINISHED INFO | automation.py | Application ran for: 0:00:59.511449
Paul, please may you take a look at this?
Blocks: 830755
Flags: needinfo?(paul.feher)
This happens very occasionally on the tegras and very often on the pandas. On the Pandas, I wonder if this is a case-sensitivity issue. On my 4.0.3 Galaxy Nexus, I see "GENERAL" (all section headings are in all caps); on my 2.3 Galaxy S I see "General".
Attached patch testSettingsMenuItems fix (obsolete) (deleted) — — Splinter Review
I hope this fixes the problem, I’ve run several times the test on may panda board and it never failed. I'm sure that is not a case-sensitivity issue because I've run the test on different devices when I created the test. I think there is more of a problem with the repeated Back action which might causing problem now and then, so I replace it.
Attachment #719953 - Flags: review?(gbrown)
Flags: needinfo?(paul.feher)
Comment on attachment 719953 [details] [diff] [review] testSettingsMenuItems fix Review of attachment 719953 [details] [diff] [review]: ----------------------------------------------------------------- My case sensitivity patch failed the try run -- I agree that's not the issue. Your patch seems to work sometimes - most of the time? - but not always: The first panda run at https://tbpl.mozilla.org/?tree=Try&rev=482465d5dd37&noignore=1 fails. Calling loadUrl immediately after verifyUrl seems wrong to me. I think verifyUrl leaves the awesome bar selected. Won't clicking on the awesome bar again in loadUrl risk unselecting the existing url (about:home) before typing the new url?
Attachment #719953 - Flags: review?(gbrown) → review-
Attached patch testSettingsMenuItems fixV2 (obsolete) (deleted) — — Splinter Review
I’ve renounced to check the pages URL, it's sufficient to verify the page title so this will eliminate the risk of using loadUrl after verifyUrl. Tested on panda several times no fails encountered.
Attachment #719953 - Attachment is obsolete: true
Attachment #720591 - Flags: review?(gbrown)
Comment on attachment 720591 [details] [diff] [review] testSettingsMenuItems fixV2 Review of attachment 720591 [details] [diff] [review]: ----------------------------------------------------------------- Yes, it works fine on Panda...but now it fails on Tegras! https://tbpl.mozilla.org/?tree=Try&rev=8eef433e88f2&noignore=1
Attachment #720591 - Flags: review?(gbrown) → review-
(The increase in failures is due to the Panda robocop jobs now being unhidden due to bug 843107 comment 15, and the failure rate is much higher on Panda)
Per discussion with Joel, disabling this test for now, so we can leave the Panda robocop tests unhidden and have more overall test coverage until this is resolved: https://hg.mozilla.org/integration/mozilla-inbound/rev/ffc260a0f3fb
Whiteboard: [test disabled][leave open]
Attached patch testSettingsMenuItems updated (deleted) — — Splinter Review
This patch changes the testSettingsMenuItems to manage the tablet UI changes. All green on try server. https://tbpl.mozilla.org/?tree=Try&rev=0d7dcc0d5a1c
Attachment #720591 - Attachment is obsolete: true
Attachment #761399 - Flags: review?(jmaher)
Comment on attachment 761399 [details] [diff] [review] testSettingsMenuItems updated Review of attachment 761399 [details] [diff] [review]: ----------------------------------------------------------------- ::: mobile/android/base/tests/testSettingsMenuItems.java.in @@ +18,5 @@ > public void testSettingsMenuItems() { > blockForGeckoReady(); > midWidth = mDriver.getGeckoWidth()/2; > midHeight = mDriver.getGeckoHeight()/2; > + Device mDevice = new Device(); You can use BaseTest.mDevice -- no need to create a new one. @@ +41,4 @@ > }; > > selectMenuItem("Settings"); > + mSolo.waitForText("General"); Use BaseTest's waitForText instead. Indentation is off here. @@ +81,5 @@ > + mAsserter.ok(mSolo.waitForText(item), "Waiting for " + item + " option", "The " + item + " option is present"); > + if (item.equals("Content" )) { > + mSolo.clickOnText(item); > + } > + if (item.equals("Privacy & Security") || item.equals("Tiny")) { Why "Tiny"?
(In reply to Paul Feher from comment #50) > All green on try server. > https://tbpl.mozilla.org/?tree=Try&rev=0d7dcc0d5a1c Most of the intermittent failures reported here were on Android 4.0/panda -- I would run more panda tests before re-enabling.
Comment on attachment 761399 [details] [diff] [review] testSettingsMenuItems updated Review of attachment 761399 [details] [diff] [review]: ----------------------------------------------------------------- these changes look promising, can you update the patch with feedback provided so far and help answer some of the questions. ::: mobile/android/base/tests/testSettingsMenuItems.java.in @@ +28,2 @@ > // This string contains every menu section that have multiple selection options followed by the option that is selected by default > + String defaultSectionsValues [] = { "Character encoding", "Don't show menu", "Plugins", "Tap to play", "Text size", "Tiny", "Cookies", "Enable", "Tracking", "Do not tell sites anything", "Title bar", "Show page title"}; that is a lot of changes. @@ +104,5 @@ > + } > + } > + mAsserter.ok(mSolo.waitForText(item), "Waiting for " + item + " option", "The " + item + " option is present"); > + mSolo.clickOnText(item); > + for (j = 1; j < multipleSectionsOptions [i].length; j++) { please fix the whitespace here: for (j = 1; j < multipleSectionsOptions[i].length; j++) { @@ +118,5 @@ > } > } > + if (mSolo.searchText("^Cancel$")) { > + mSolo.clickOnText("^Cancel$"); > + mSolo.sleep(1500); I really don't like this sleep in here. Are there other options? Is this necessary?
Attachment #761399 - Flags: review?(jmaher) → review-
I retrigged a lot of try runs, all good except one crash on panda with this specific test case. I would vote for turning this on vs blocking on the 1 crash: https://tbpl.mozilla.org/?tree=Try&rev=0d7dcc0d5a1c :gbrown, thoughts?
That try run looks fine -- I would not worry about the one crash. However, I notice there are big plans to change Settings and re-enable this test in bug 872329 -- it might be better to wait for that bug to be resolved.
Geoff - what do you think about resolving this bug? I glanced through inbound and didn't see any robocop oranges relating to Settings, which was enabled with bug 872329.
Flags: needinfo?(gbrown)
Fine by me.
Flags: needinfo?(gbrown)
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Whiteboard: [test disabled][leave open]
Resolution: FIXED → INCOMPLETE
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: