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)
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 | Splinter Review
The fix v2 (2.02 KB, patch)
2012-07-05 12:56 PDT, Mark Banner (:standard8)
mconley: review-
Details | Diff | Splinter Review
The fix v2 (4.51 KB, patch)
2012-07-05 13:32 PDT, Mark Banner (:standard8)
mconley: review+
mozilla: superreview+
Details | Diff | Splinter Review

Description User image Mike Conley (:mconley) 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 User image Mike Conley (:mconley) 2012-07-05 12:43:00 PDT
*** Bug 771226 has been marked as a duplicate of this bug. ***
Comment 2 User image Mike Conley (:mconley) 2012-07-05 12:43:24 PDT
*** Bug 771229 has been marked as a duplicate of this bug. ***
Comment 3 User image Mike Conley (:mconley) 2012-07-05 12:43:34 PDT
*** Bug 771230 has been marked as a duplicate of this bug. ***
Comment 4 User image 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 User image 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 User image 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 User image Mike Conley (:mconley) 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 User image Mark Banner (:standard8) 2012-07-05 13:32:10 PDT
Created attachment 639447 [details] [diff] [review]
The fix v2
Comment 9 User image David :Bienvenu 2012-07-05 13:41:29 PDT
looks reasonable, but it'll take me a while to rebuild and test...
Comment 10 User image 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 User image 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 User image Mike Conley (:mconley) 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 User image 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.