Closed Bug 1258055 Opened 4 years ago Closed 4 years ago

Enable a bunch of mochitest plain password manager tests not involving UI for e10s

Categories

(Toolkit :: Password Manager, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla48
Tracking Status
e10s + ---
firefox48 --- fixed

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.
* 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)
* 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)
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/
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/
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.
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 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/mozilla-central/rev/5585f64b747d
https://hg.mozilla.org/mozilla-central/rev/b903047d9572
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
You need to log in before you can comment on or make changes to this bug.