Last Comment Bug 771277 - Fix XPCShell password authentication tests so that they don't pass around fake docShells
: Fix XPCShell password authentication tests so that they don't pass around fak...
Status: RESOLVED FIXED
: intermittent-failure
Product: Thunderbird
Classification: Client Software
Component: Testing Infrastructure (show other bugs)
: Trunk
: x86 All
: -- normal (vote)
: Thunderbird 16.0
Assigned To: Mark Banner (:standard8)
:
Mentors:
: 771226 771229 771230 (view as bug list)
Depends on:
Blocks: 669603
  Show dependency treegraph
 
Reported: 2012-07-05 12:42 PDT by Mike Conley (:mconley) - (needinfo me!)
Modified: 2012-11-25 19:31 PST (History)
2 users (show)
standard8: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
The fix (4.39 KB, patch)
2012-07-05 12:53 PDT, Mark Banner (:standard8)
no flags Details | Diff | Review
The fix v2 (2.02 KB, patch)
2012-07-05 12:56 PDT, Mark Banner (:standard8)
mconley: review-
Details | Diff | Review
The fix v2 (4.51 KB, patch)
2012-07-05 13:32 PDT, Mark Banner (:standard8)
mconley: review+
mozilla: superreview+
Details | Diff | Review

Description Mike Conley (:mconley) - (needinfo me!) 2012-07-05 12:42:35 PDT
Bug 669603 introduced a change that makes it impossible for us to mock docShells with JavaScript for our XPCShell tests.

We need to alter our authentication and testing infrastructure so that passing around fake docShells is no longer required.
Comment 1 Mike Conley (:mconley) - (needinfo me!) 2012-07-05 12:43:00 PDT
*** Bug 771226 has been marked as a duplicate of this bug. ***
Comment 2 Mike Conley (:mconley) - (needinfo me!) 2012-07-05 12:43:24 PDT
*** Bug 771229 has been marked as a duplicate of this bug. ***
Comment 3 Mike Conley (:mconley) - (needinfo me!) 2012-07-05 12:43:34 PDT
*** Bug 771230 has been marked as a duplicate of this bug. ***
Comment 4 Mark Banner (:standard8) 2012-07-05 12:53:14 PDT
Created attachment 639433 [details] [diff] [review]
The fix

This patch creates a function on nsIMsgWindow that will return the auth prompt.  This gives us the opportunity in xpcshell tests to not need to fake the docShell.

It also gives us a bit of a clearer function to call when we do really want the auth prompt.
Comment 5 Mark Banner (:standard8) 2012-07-05 12:56:36 PDT
Created attachment 639436 [details] [diff] [review]
The fix v2

Now without commented out sections.
Comment 6 Mark Banner (:standard8) 2012-07-05 13:22:57 PDT
Comment on attachment 639436 [details] [diff] [review]
The fix v2

Passes unit tests locally and still seems to work for the prompting users normally.
Comment 7 Mike Conley (:mconley) - (needinfo me!) 2012-07-05 13:31:03 PDT
Comment on attachment 639436 [details] [diff] [review]
The fix v2

This doesn't look like the right patch to me...
Comment 8 Mark Banner (:standard8) 2012-07-05 13:32:10 PDT
Created attachment 639447 [details] [diff] [review]
The fix v2
Comment 9 David :Bienvenu 2012-07-05 13:41:29 PDT
looks reasonable, but it'll take me a while to rebuild and test...
Comment 10 David :Bienvenu 2012-07-05 16:21:01 PDT
Comment on attachment 639447 [details] [diff] [review]
The fix v2

sr=me, based on running the mailnews/local tests, and verifying that imap password prompting still works.
Comment 11 Mark Banner (:standard8) 2012-07-06 01:57:13 PDT
I checked this in early pending Mike's review so that we could get the tree green again:

https://hg.mozilla.org/comm-central/rev/e61aeddc94ef
Comment 12 Mike Conley (:mconley) - (needinfo me!) 2012-07-06 06:13:45 PDT
Comment on attachment 639447 [details] [diff] [review]
The fix v2

Review of attachment 639447 [details] [diff] [review]:
-----------------------------------------------------------------

This looks good, everything seems to function normally, and I can't really argue with the results on tbpl.

Just two little nits, perhaps worthy of a followup patch.

Great job!

-Mike

::: mailnews/base/public/nsIMsgWindow.idl
@@ +45,5 @@
>     */
>    readonly attribute nsIDocShell messageWindowDocShell;
> +
> +    /**
> +     * Returns the auth prompt associtated with the window. This will only return

Nits:

This block is incorrectly indented, and there's a typo: "associtated" -> "associated".
Comment 13 Mark Banner (:standard8) 2012-08-16 14:20:59 PDT
I eventually got around to pushing the comment-only nits:

https://hg.mozilla.org/comm-central/rev/c1a5b1b4af57

Note You need to log in before you can comment on or make changes to this bug.