Closed
Bug 1258055
Opened 8 years ago
Closed 8 years ago
Enable a bunch of mochitest plain password manager tests not involving UI for e10s
Categories
(Toolkit :: Password Manager, defect)
Toolkit
Password Manager
Tracking
()
RESOLVED
FIXED
mozilla48
People
(Reporter: MattN, Assigned: MattN)
References
(Blocks 1 open bug)
Details
Attachments
(2 files)
There are a bunch of password manager mochitest-plain tests which don't involve chrome UI but just need to manipulate pwmgr storage which are easy to fix.
Assignee | ||
Comment 1•8 years ago
|
||
* Support changing `selfFilling` in the `setupParent` message since these tests aren't self-filling like the ones already working in e10s. ** Make selfFilling = false work with e10s using a registerRunTests message. * Register a common cleanup function to remove all logins, remove all disabled hosts, and clear the HTTP auth state in the parent. ** Actually check there are no existing logins in commonInit (uncomment code) ** Delete some removeLogin calls at the end of tests now that we removeAllLogins during cleanup * Consolidate loadParentTestFile with runInParent Review commit: https://reviewboard.mozilla.org/r/41193/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/41193/
Attachment #8732460 -
Flags: review?(dolske)
Assignee | ||
Comment 2•8 years ago
|
||
* Delete test_zzz_finish.html now that we always cleanup logins, disabled hosts and auth state. * Move test_autofill_before_load.html to mochitest/ since it doesn't need to be fixed (skip-if=true) * Use runInParent to add custom logins for some tests * Remove some cleanup functions that were just cleaning up DOM attributes since it's not necessary. Review commit: https://reviewboard.mozilla.org/r/41195/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/41195/
Attachment #8732461 -
Flags: review?(dolske)
Assignee | ||
Comment 3•8 years ago
|
||
Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=a9b4d54c8a55
Assignee | ||
Comment 4•8 years ago
|
||
Comment on attachment 8732460 [details] MozReview Request: Bug 1258055 - E10S helpers for mochitest-plain password manager tests not involving UI. r=dolske Review request updated; see interdiff: https://reviewboard.mozilla.org/r/41193/diff/1-2/
Assignee | ||
Comment 5•8 years ago
|
||
Comment on attachment 8732461 [details] MozReview Request: Bug 1258055 - Enable a bunch of mochitest-plain password manager tests not involving UI for e10s. r=dolske Review request updated; see interdiff: https://reviewboard.mozilla.org/r/41195/diff/1-2/
Assignee | ||
Comment 6•8 years ago
|
||
I just made minor support-files changes to address the try push failures. The extension API stuff was a failure from the rev. I pulled, not my changes.
Updated•8 years ago
|
Comment 7•8 years ago
|
||
Comment on attachment 8732460 [details] MozReview Request: Bug 1258055 - E10S helpers for mochitest-plain password manager tests not involving UI. r=dolske https://reviewboard.mozilla.org/r/41193/#review38893 ::: toolkit/components/passwordmgr/test/pwmgr_common.js:138 (Diff revision 2) > - //todo(false, "Warning: wasn't expecting logins to be present."); > - pwmgr.removeAllLogins(); > - } > var disabledHosts = pwmgr.getAllDisabledHosts(); > if (disabledHosts.length) { > //todo(false, "Warning: wasn't expecting disabled hosts to be present."); Turn this commented-out todo into a is() now, since we have a cleanup function that should ensure it's good? Doesn't look like there's a way to check that the HTTP authmanager was cleared, oh well.
Attachment #8732460 -
Flags: review?(dolske) → review+
Comment 8•8 years ago
|
||
Comment on attachment 8732461 [details] MozReview Request: Bug 1258055 - Enable a bunch of mochitest-plain password manager tests not involving UI for e10s. r=dolske https://reviewboard.mozilla.org/r/41195/#review38897 Outstanding. Nice that so many of the tests ended up needing fairly minor changes. ::: toolkit/components/passwordmgr/test/mochitest.ini (Diff revision 2) > - auth2/authenticate.sjs > > -[test_autofill_before_load.html] > -# This test doesn't pass because we can't ensure a cross-platform event that > -# occurs between DOMContentLoaded and Pageload > -skip-if = true Let's just delete this test (test_autofill_before_load.html). It's been disabled since it landed 9 years ago, and is not really useful. ::: toolkit/components/passwordmgr/test/mochitest/test_basic_form_html5.html:17 (Diff revision 2) > -commonInit(); > +runChecksAfterCommonInit(() => startTest()); > -SimpleTest.waitForExplicitFinish(); > > -const Ci = SpecialPowers.Ci; > -const Cc = SpecialPowers.Cc; > -pwmgr = Cc["@mozilla.org/login-manager;1"]. > +runInParent(function setup() { > + const { classes: Cc, interfaces: Ci, results: Cr, utils: Cu } = Components; > + var pwmgr = Cc["@mozilla.org/login-manager;1"]. *gasp* Var? Not Let?!
Attachment #8732461 -
Flags: review?(dolske) → review+
https://hg.mozilla.org/integration/fx-team/rev/5585f64b747d https://hg.mozilla.org/integration/fx-team/rev/b903047d9572
Comment 10•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/5585f64b747d https://hg.mozilla.org/mozilla-central/rev/b903047d9572
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox48:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Comment 12•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/42c6ad3fc683
You need to log in
before you can comment on or make changes to this bug.
Description
•