Closed
Bug 846340
Opened 11 years ago
Closed 11 years ago
Robocop: Add test for 'Clear Site settings', 'Clear Saved passwords' and 'Clear history' options
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
Firefox 29
People
(Reporter: csuciu, Assigned: csuciu)
References
()
Details
Attachments
(1 file, 6 obsolete files)
7.11 KB,
patch
|
gbrown
:
review+
|
Details | Diff | Splinter Review |
This bug will track the creation of on automated test for clearing the site settings. MozTrap testcase: https://moztrap.mozilla.org/manage/caseversion/35827/
Assignee | ||
Comment 1•11 years ago
|
||
Attachment #719947 -
Flags: review?(jmaher)
Comment 2•11 years ago
|
||
Comment on attachment 719947 [details] [diff] [review] Patch ClearSiteSettings v1 Review of attachment 719947 [details] [diff] [review]: ----------------------------------------------------------------- this fails for me locally and on tbpl: https://tbpl.mozilla.org/?tree=Try&rev=d7e4964d6a3d&noignore=1 specifically on tegra.
Attachment #719947 -
Flags: review?(jmaher) → review-
Assignee | ||
Comment 3•11 years ago
|
||
Attachment #719947 -
Attachment is obsolete: true
Attachment #720731 -
Flags: review?(jmaher)
Comment 4•11 years ago
|
||
Comment on attachment 720731 [details] [diff] [review] Patch ClearSiteSettings v2 Review of attachment 720731 [details] [diff] [review]: ----------------------------------------------------------------- this is failing on my local tegra: 01-19 13:31:39.266 I/Robocop (23782): 4 INFO TEST-PASS | testClearPrivateData | checking clear button - clear button exists 01-19 13:31:40.356 W/InputManagerService( 1022): Window already focused, ignoring focus gain of: com.android.internal.view.IInputMethodClient$Stub$Proxy@4454aa38 01-19 13:32:01.286 I/Robocop (23782): 5 INFO TEST-UNEXPECTED-FAIL | testClearPrivateData | private data cleared successfully - got false, expected true 01-19 13:32:01.306 I/Robocop (23782): Exception caught during test! 01-19 13:32:01.306 I/Robocop (23782): junit.framework.AssertionFailedError: 5 INFO TEST-UNEXPECTED-FAIL | testClearPrivateData | private data cleared successfully - got false, expected true 01-19 13:32:01.306 I/Robocop (23782): at junit.framework.Assert.fail(Assert.java:47) 01-19 13:32:01.306 I/Robocop (23782): at org.mozilla.fennec_jmaher.FennecMochitestAssert._logMochitestResult(FennecMochitestAssert.java:107) 01-19 13:32:01.306 I/Robocop (23782): at org.mozilla.fennec_jmaher.FennecMochitestAssert.ok(FennecMochitestAssert.java:136) 01-19 13:32:01.306 I/Robocop (23782): at org.mozilla.fennec_jmaher.FennecMochitestAssert.is(FennecMochitestAssert.java:142) 01-19 13:32:01.306 I/Robocop (23782): at org.mozilla.fennec_jmaher.tests.testClearPrivateData.clearHistory(testClearPrivateData.java:47) 01-19 13:32:01.306 I/Robocop (23782): at org.mozilla.fennec_jmaher.tests.testClearPrivateData.testClearPrivateData(testClearPrivateData.java:23) 01-19 13:32:01.306 I/Robocop (23782): at java.lang.reflect.Method.invokeNative(Native Method) 01-19 13:32:01.306 I/Robocop (23782): at java.lang.reflect.Method.invoke(Method.java:521) 01-19 13:32:01.306 I/Robocop (23782): at android.test.InstrumentationTestCase.runMethod(InstrumentationTestCase.java:204) 01-19 13:32:01.306 I/Robocop (23782): at android.test.InstrumentationTestCase.runTest(InstrumentationTestCase.java:194) 01-19 13:32:01.306 I/Robocop (23782): at android.test.ActivityInstrumentationTestCase2.runTest(ActivityInstrumentationTestCase2.java:186) 01-19 13:32:01.306 I/Robocop (23782): at org.mozilla.fennec_jmaher.tests.BaseTest.runTest(BaseTest.java:125) 01-19 13:32:01.306 I/Robocop (23782): at junit.framework.TestCase.runBare(TestCase.java:127) 01-19 13:32:01.306 I/Robocop (23782): at junit.framework.TestResult$1.protect(TestResult.java:106) 01-19 13:32:01.306 I/Robocop (23782): at junit.framework.TestResult.runProtected(TestResult.java:124) 01-19 13:32:01.306 I/Robocop (23782): at junit.framework.TestResult.run(TestResult.java:109) 01-19 13:32:01.306 I/Robocop (23782): at junit.framework.TestCase.run(TestCase.java:118) 01-19 13:32:01.306 I/Robocop (23782): at android.test.AndroidTestRunner.runTest(AndroidTestRunner.java:169) 01-19 13:32:01.306 I/Robocop (23782): at android.test.AndroidTestRunner.runTest(AndroidTestRunner.java:154) 01-19 13:32:01.306 I/Robocop (23782): at android.test.InstrumentationTestRunner.onStart(InstrumentationTestRunner.java:520) 01-19 13:32:01.306 I/Robocop (23782): at android.app.Instrumentation$InstrumentationThread.run(Instrumentation.java:1447) 01-19 13:32:01.306 I/Robocop (23782): 6 INFO TEST-UNEXPECTED-FAIL | testClearPrivateData | Exception caught - junit.framework.AssertionFailedError: 5 INFO TEST-UNEXPECTED-FAIL | testClearPrivateData | private data cleared successfully - got false, expected true 01-19 13:32:01.326 I/Robocop (23782): 7 INFO TEST-END | testClearPrivateData | finished in 122825ms 01-19 13:32:01.326 I/Robocop (23782): 8 INFO TEST-START | Shutdown 01-19 13:32:01.326 I/Robocop (23782): 9 INFO Passed: 3 01-19 13:32:01.336 I/Robocop (23782): 10 INFO Failed: 2 01-19 13:32:01.386 I/Robocop (23782): 11 INFO Todo: 0 01-19 13:32:01.396 I/Robocop (23782): 12 INFO SimpleTest FINISHED
Attachment #720731 -
Flags: review?(jmaher) → review-
Assignee | ||
Comment 5•11 years ago
|
||
Attachment #720731 -
Attachment is obsolete: true
Attachment #723899 -
Flags: review?(jmaher)
Comment 6•11 years ago
|
||
Comment on attachment 723899 [details] [diff] [review] Patch ClearSiteSettings v3 Review of attachment 723899 [details] [diff] [review]: ----------------------------------------------------------------- This is failing on try server on both tegra (android 2.3) and panda (android 4.0.4). https://tbpl.mozilla.org/?tree=Try&rev=abe784650d13
Attachment #723899 -
Flags: review?(jmaher) → review-
Assignee | ||
Comment 7•11 years ago
|
||
Try server run: https://tbpl.mozilla.org/?tree=Try&rev=f260110918f9
Attachment #723899 -
Attachment is obsolete: true
Attachment #762614 -
Flags: review?(jmaher)
Comment 8•11 years ago
|
||
Comment on attachment 762614 [details] [diff] [review] Patch ClearSiteSettings v4 Review of attachment 762614 [details] [diff] [review]: ----------------------------------------------------------------- a few nits, r- to ensure we cancel out the dialog at the end. The Try server run looks pretty good. ::: mobile/android/base/tests/testClearPrivateData.java.in @@ +13,5 @@ > > public void testClearPrivateData() { > blockForGeckoReady(); > clearHistory(); > + clearSiteSettigs(); s/Settigs/Settings/ @@ +36,4 @@ > mAsserter.ok(mSolo.searchButton("Clear data"),"checking clear button","clear button exists"); > mSolo.clickOnButton("Clear data"); > + ClearDataEventExpecter.blockForEvent(); > + mActions.sendSpecialKey(Actions.SpecialKey.BACK); we are removing the confirmation check here? is this intentional, accidental? @@ +69,5 @@ > + > + // Re-trigger geolocation notification in new tab > + addTab(url); // Using addTab(url) because loadAndPaint(url) seems to fail at this step > + mSolo.waitForText("Share your location"); > + mAsserter.is(mSolo.searchText("Share your location"), true, "Geolocation doorhanger has been displayed"); I would like to cancel this dialog out to make sure we don't leave things in a pending state.
Attachment #762614 -
Flags: review?(jmaher) → review-
Comment 9•11 years ago
|
||
Be aware that the Settings are being modified in bug 872329 -- that might affect this test.
Assignee | ||
Comment 10•11 years ago
|
||
(In reply to Joel Maher (:jmaher) from comment #8) > Comment on attachment 762614 [details] [diff] [review] > Patch ClearSiteSettings v4 > > Review of attachment 762614 [details] [diff] [review]: > ----------------------------------------------------------------- > > a few nits, r- to ensure we cancel out the dialog at the end. The Try > server run looks pretty good. > > ::: mobile/android/base/tests/testClearPrivateData.java.in > @@ +13,5 @@ > > > > public void testClearPrivateData() { > > blockForGeckoReady(); > > clearHistory(); > > + clearSiteSettigs(); > > s/Settigs/Settings/ Corrected. > > @@ +36,4 @@ > > mAsserter.ok(mSolo.searchButton("Clear data"),"checking clear button","clear button exists"); > > mSolo.clickOnButton("Clear data"); > > + ClearDataEventExpecter.blockForEvent(); > > + mActions.sendSpecialKey(Actions.SpecialKey.BACK); > > we are removing the confirmation check here? is this intentional, > accidental? This was intentional. > > @@ +69,5 @@ > > + > > + // Re-trigger geolocation notification in new tab > > + addTab(url); // Using addTab(url) because loadAndPaint(url) seems to fail at this step > > + mSolo.waitForText("Share your location"); > > + mAsserter.is(mSolo.searchText("Share your location"), true, "Geolocation doorhanger has been displayed"); > > I would like to cancel this dialog out to make sure we don't leave things in > a pending state. Added mActions.sendSpecialKey(Actions.SpecialKey.BACK); to dismiss the geolocation doorhanger
Attachment #762614 -
Attachment is obsolete: true
Attachment #763443 -
Flags: review?(jmaher)
Assignee | ||
Comment 11•11 years ago
|
||
(In reply to Geoff Brown [:gbrown] from comment #9) > Be aware that the Settings are being modified in bug 872329 -- that might > affect this test. I will update the test when this changes will land
Comment 12•11 years ago
|
||
Comment on attachment 763443 [details] [diff] [review] Patch ClearSiteSettings v4.1 Review of attachment 763443 [details] [diff] [review]: ----------------------------------------------------------------- thanks!
Attachment #763443 -
Flags: review?(jmaher) → review+
Updated•11 years ago
|
Keywords: checkin-needed
Comment 14•11 years ago
|
||
Backed out for frequent failures. https://hg.mozilla.org/integration/mozilla-inbound/rev/8bcccaf54ff1 https://tbpl.mozilla.org/php/getParsedLog.php?id=24250715&tree=Mozilla-Inbound
Updated•11 years ago
|
Summary: Robocop: Add test for 'Clear Site Settings' → Robocop: Add test for 'Clear Site settings', 'Clear Saved passwords' and 'Clear history' options
Comment 15•11 years ago
|
||
I have updated this test for clearSiteSettings option, which was backed out for frequent failures and I have integrated test for clearSavedPassword option. I have run the tests locally on multiple devices and on our Panda board and there were no failures.
Attachment #832224 -
Flags: review?(jmaher)
Attachment #832224 -
Flags: review?(gbrown)
Updated•11 years ago
|
Attachment #832224 -
Flags: review?(gbrown)
Comment 16•11 years ago
|
||
Comment on attachment 832224 [details] [diff] [review] Clear: Site settings, saved password, history Review of attachment 832224 [details] [diff] [review]: ----------------------------------------------------------------- a lot of style changes needed. I would also like to see this on try server with 20 retriggers and verify that this test case and subsequent test cases in the test run are stable. Paul is in the process of obtaining try server access. ::: mobile/android/base/tests/testClearPrivateData.java @@ +62,5 @@ > + > + public void clearSiteSettings() { > + String SHARE_STRINGS [] = {"Share your location with", "Share", "Don't share", "There are no settings to clear"}; > + String TITLE = StringHelper.ROBOCOP_GEOLOCATION_TITLE; > + String URL = getAbsoluteUrl(StringHelper.ROBOCOP_GEOLOCATION_URL); please make these variables lower case, i.e. shareStrings, title, url; @@ +64,5 @@ > + String SHARE_STRINGS [] = {"Share your location with", "Share", "Don't share", "There are no settings to clear"}; > + String TITLE = StringHelper.ROBOCOP_GEOLOCATION_TITLE; > + String URL = getAbsoluteUrl(StringHelper.ROBOCOP_GEOLOCATION_URL); > + loadCheckDismiss(SHARE_STRINGS [1], URL, SHARE_STRINGS [0]); > + checkOption(SHARE_STRINGS [1], "Clear"); nit: why is there a space between 'share_strings' and '[1]'? The existing style is to have SHARE_STRINGS[1], etc. @@ +75,5 @@ > + > + public void clearPassword(){ > + String PASSWORD_STRINGS [] = {"Save password", "Save", "Don't save"}; > + String TITLE = StringHelper.ROBOCOP_BLANK_PAGE_01_TITLE; > + String LOGIN_URL = getAbsoluteUrl(StringHelper.ROBOCOP_LOGIN_URL); please make these local variables lowercase.
Attachment #832224 -
Flags: review?(jmaher) → review-
Comment 17•11 years ago
|
||
Comment on attachment 832224 [details] [diff] [review] Clear: Site settings, saved password, history Review of attachment 832224 [details] [diff] [review]: ----------------------------------------------------------------- Hope you don't mind a little more feedback! ::: mobile/android/base/tests/testClearPrivateData.java @@ +33,5 @@ > String blank1 = getAbsoluteUrl(StringHelper.ROBOCOP_BLANK_PAGE_01_URL); > String blank2 = getAbsoluteUrl(StringHelper.ROBOCOP_BLANK_PAGE_02_URL); > + String title = StringHelper.ROBOCOP_BLANK_PAGE_01_TITLE; > + inputAndLoadUrl(blank1); > + waitForText(title); Consider using verifyPageTitle here instead of waitForText. @@ +41,5 @@ > verifyHistoryCount(1); > clearPrivateData(); > > + //checking for device > + checkDevice(title); I would combine checkDevice with clearPrivateData -- whenever you go into Settings and clear data, you probably want to exit Settings afterwards. @@ +76,5 @@ > + public void clearPassword(){ > + String PASSWORD_STRINGS [] = {"Save password", "Save", "Don't save"}; > + String TITLE = StringHelper.ROBOCOP_BLANK_PAGE_01_TITLE; > + String LOGIN_URL = getAbsoluteUrl(StringHelper.ROBOCOP_LOGIN_URL); > + blockForGeckoReady(); Why blockForGeckoReady now? 2 sub-tests have already been run -- surely the browser is up? @@ +90,5 @@ > + if (mDevice.type.equals("phone")) { > + mActions.sendSpecialKey(Actions.SpecialKey.BACK); > + mAsserter.ok(waitForText(StringHelper.PRIVACY_SECTION_LABEL), "waiting to perform one back", "one back"); > + mActions.sendSpecialKey(Actions.SpecialKey.BACK); > + waitForText(title); verifyPageTitle instead of waitForText here? @@ +102,5 @@ > + // Load a URL, verify that the doorhanger appears and dismiss it > + public void loadCheckDismiss(String option, String url, String message) { > + inputAndLoadUrl(url); > + waitForText(message); > + mAsserter.is(mSolo.searchText(message), true, "Doorhanger has been displayed"); If you use something like: ..., true, "Doorhanger "+message+" has been displayed"); it will be easier to follow the test progress from the output. @@ +114,5 @@ > + mSolo.clickLongOnView(toolbarView); > + mAsserter.ok(waitForText(StringHelper.CONTEXT_MENU_ITEMS_IN_URL_BAR [2]), "Waiting for the pop-up to open", "Pop up was openend"); > + mSolo.clickOnText(StringHelper.CONTEXT_MENU_ITEMS_IN_URL_BAR [2]); > + mAsserter.ok(waitForText(option), "Verify that the option: " + option + " is in the list", "The option is in the list. There are settings to clear"); > + mSolo.clickOnText(button); Can you use mSolo.clickOnButton here?
Comment 18•11 years ago
|
||
Made the necessary changes. The try-server is all green: https://tbpl.mozilla.org/?tree=Try&rev=586252ae1d19
Attachment #832224 -
Attachment is obsolete: true
Attachment #8336675 -
Flags: review?(jmaher)
Comment 19•11 years ago
|
||
Comment on attachment 8336675 [details] [diff] [review] Clear: Site settings, saved password, history v1.0 Review of attachment 8336675 [details] [diff] [review]: ----------------------------------------------------------------- gbrown is more up to speed on robocop than I am, my review would be basic syntax and a few random questions.
Attachment #8336675 -
Flags: review?(jmaher) → review?(gbrown)
Comment 20•11 years ago
|
||
Comment on attachment 8336675 [details] [diff] [review] Clear: Site settings, saved password, history v1.0 Review of attachment 8336675 [details] [diff] [review]: ----------------------------------------------------------------- ::: mobile/android/base/tests/testClearPrivateData.java @@ +62,5 @@ > + public void clearSiteSettings() { > + String shareStrings[] = {"Share your location with", "Share", "Don't share", "There are no settings to clear"}; > + String titleGeolocation = StringHelper.ROBOCOP_GEOLOCATION_TITLE; > + String url = getAbsoluteUrl(StringHelper.ROBOCOP_GEOLOCATION_URL); > + loadCheckDismiss(shareStrings[1], url, shareStrings[0]); nit - I find the array of strings less readable than alternatives: loadCheckDismiss("Share", url, "Share your location with") or String shareButton = "Share"; String doorhangerTitle = "Share your location with"; loadCheckDismiss(shareButton, url, doorhangerTitle); @@ +74,5 @@ > + public void clearPassword(){ > + String passwordStrings[] = {"Save password", "Save", "Don't save"}; > + String title = StringHelper.ROBOCOP_BLANK_PAGE_01_TITLE; > + String loginUrl = getAbsoluteUrl(StringHelper.ROBOCOP_LOGIN_URL); > + loadCheckDismiss(passwordStrings[1], loginUrl, passwordStrings[0]); Same here...but keep it as it is if you prefer this style.
Attachment #8336675 -
Flags: review?(gbrown) → review+
Updated•11 years ago
|
Keywords: checkin-needed
Updated•11 years ago
|
Attachment #763443 -
Attachment is obsolete: true
Comment 21•11 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/0aeb846dc2e7
Keywords: checkin-needed
Whiteboard: [fixed-in-fx-team]
Comment 22•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/0aeb846dc2e7
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 28
Comment 23•11 years ago
|
||
Backed out for causing frequent testMasterPassword failures. https://hg.mozilla.org/integration/mozilla-inbound/rev/9e38b4476027 https://hg.mozilla.org/releases/mozilla-aurora/rev/f9ae278dead7
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: Firefox 28 → ---
Comment 24•11 years ago
|
||
Merge of the backout: https://hg.mozilla.org/mozilla-central/rev/9e38b4476027 That said, the failure persists (I think the robocop chunking changed, which obscured the regression range), so I'll probably end up relanding this.
Flags: needinfo?(ryanvm)
Updated•11 years ago
|
Flags: needinfo?(ryanvm)
Keywords: checkin-needed
Comment 25•11 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/f28c346633a6
Keywords: checkin-needed
Whiteboard: [fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/f28c346633a6
Status: REOPENED → RESOLVED
Closed: 11 years ago → 11 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 29
Updated•3 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
•