Closed
Bug 1018624
Opened 11 years ago
Closed 11 years ago
TEST-UNEXPECTED-FAIL | /builds/slave/test/build/xpcshell/tests/mailnews/local/test/unit/test_pop3PasswordFailure2.js | test failed (with xpcshell return code: 0), see following log:
Categories
(MailNews Core :: Testing Infrastructure, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
Thunderbird 32.0
People
(Reporter: jcranmer, Assigned: jcranmer)
References
Details
(Keywords: intermittent-failure)
Attachments
(5 files)
24.58 KB,
patch
|
standard8
:
review+
|
Details | Diff | Splinter Review |
2.90 KB,
text/plain
|
Details | |
27.64 KB,
patch
|
standard8
:
review+
|
Details | Diff | Splinter Review |
8.75 KB,
patch
|
neil
:
review+
|
Details | Diff | Splinter Review |
2.10 KB,
patch
|
standard8
:
review+
|
Details | Diff | Splinter Review |
Bug 853549 makes initializing the password storage happen asynchronously. This hurts our tests very badly, because we've been assuming that this happens synchronously.
The solution is to start using promises. I have a WIP patch that fixes all of the compose test; it's completely mechanical changes once you know what the problem is.
Assignee | ||
Comment 1•11 years ago
|
||
I spoke too soon... there's some funky edge cases going on when async tests or alert test utils is brought in.
Assignee | ||
Comment 2•11 years ago
|
||
Basically every test that ain't using asyncTestUtils.js
Attachment #8432181 -
Flags: review?(standard8)
Comment 3•11 years ago
|
||
Comment on attachment 8432181 [details] [diff] [review]
Fix the simple failures [checkin: comment 4]
Review of attachment 8432181 [details] [diff] [review]:
-----------------------------------------------------------------
Looks good, r=Standand8 once you've considered the debug comments below.
::: mailnews/compose/test/unit/test_bug155172.js
@@ +3,5 @@
> * Authentication tests for SMTP.
> */
>
> load("../../../resources/alertTestUtils.js");
> +Services.prefs.setBoolPref("signon.debug", true);
Not sure we need this...
@@ +46,5 @@
> return handler;
> }
>
> server = setupServerDaemon(createHandler);
> + //server.setDebugLevel(fsDebugAll);
This is generally useful for debugging tests, unintentional removal?
Attachment #8432181 -
Flags: review?(standard8) → review+
Assignee | ||
Comment 4•11 years ago
|
||
Part 1 checked in:
https://hg.mozilla.org/comm-central/rev/30b5a6ab6780
Assignee | ||
Comment 5•11 years ago
|
||
An off-hand comment from rkent in the IRC channel asked if this is a test-specific bustage or if our underlying code is broken. After a bit of playing around, poking the async auth prompter to delay all prompts until after Services.logins.initializationPromise managed to get the tests to pass much more easily... except they've exposed timing issues in our tests.
The biggest issue in the tests is the NNTP legacy auth credentials migrator, which ends up trying to run on shutdown for some tests. Given that this was added in TB 13 (bug 201750), I think we can safely remove it now. Except that all of our password stores for the tests use the legacy credentials. :-(
[This may be a good time to regenerate them to use the JSON storage format as well, precluding the inevitable future migration issues bug.]
Assignee | ||
Comment 6•11 years ago
|
||
Based on the script in bug 925489.
How to run:
1. Drop in mailnews/base/test/unit
2. Run as an xpcshell test.
3. Copy the resulting .json files from $OBJDIR/mozilla/_tests/xpcshell/mailnews/data to mailnews/test/data.
4. Copy the resulting key3.db from /tmp/xpcshell/xpcshellprofile to mailnews/test/data.
[Note that I goofed on the name of the -alt database due to bad copy-paste. It's fixed in the actual patches I made.]
Assignee | ||
Comment 7•11 years ago
|
||
There is a slight change from the current databases in that the NNTP passwords are now encoded in their TB 13+ form instead of the original TB 12- form. That's really the only reason I need this patch.
Attachment #8433819 -
Flags: review?(standard8)
Assignee | ||
Comment 8•11 years ago
|
||
I think we can safely stop supporting migration from TB 12 or older profiles. This is necessary for part 4, since without it, we try to migrate passwords during shutdown. Which turns out to work extremely poorly.
Attachment #8433822 -
Flags: review?(neil)
Assignee | ||
Comment 9•11 years ago
|
||
UGH.
I haven't uploaded part 4 yet because I get a few test failures if I don't disable the error console tunnel. Here is what happens:
The new code uses OS.File. OS.File uses a worker thread. Worker threads require loading a file channel for the URL. The file channel requires loading a mime type conversion for the .js extension. This mapping isn't built in, so it asks the external handler service for the mapping. This tries loading UMimType, which isn't enabled by default in the handler service, throwing an error to the error console. Thus causing the tests to fail.
...
Lovely.
Assignee | ||
Updated•11 years ago
|
Summary: TEST-UNEXPECTED-FAIL | C:/slave/test/build/xpcshell/tests/mailnews/compose/test/unit/head_compose.js | "EHLO test,AUTH PLAIN AHRlc3Quc210cEBmYWtlc2VydmVyAHdyb25n,MAIL FROM:<from@foo.invalid> SIZE=155,RCPT TO:<to@foo.invalid>,DATA" == "EHLO test,AUTH PLAI → TEST-UNEXPECTED-FAIL | /builds/slave/test/build/xpcshell/tests/mailnews/local/test/unit/test_pop3PasswordFailure2.js | test failed (with xpcshell return code: 0), see following log:
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment 16•11 years ago
|
||
Comment on attachment 8433819 [details] [diff] [review]
Part 2: modify the passwords database
I'm a bit tentative about swapping the format this early, but I think it should be fine. r=Standard8
Attachment #8433819 -
Flags: review?(standard8) → review+
Updated•11 years ago
|
Attachment #8433822 -
Flags: review?(neil) → review+
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Assignee | ||
Comment 25•11 years ago
|
||
msgAsyncPrompter is only ever used to trigger the authentication prompts, which will first need to grab the login manager anyways.
Attachment #8435440 -
Flags: review?(standard8)
Assignee | ||
Updated•11 years ago
|
Attachment #8432181 -
Attachment description: Fix the simple failures → Fix the simple failures [checkin: comment 4]
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment 27•11 years ago
|
||
Comment on attachment 8435440 [details] [diff] [review]
Part 4: Wait for Services.logins initialization
Review of attachment 8435440 [details] [diff] [review]:
-----------------------------------------------------------------
Looks reasonable, r=Standard8
Attachment #8435440 -
Flags: review?(standard8) → review+
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Assignee | ||
Comment 30•11 years ago
|
||
https://hg.mozilla.org/comm-central/rev/9f96a0305c9f
https://hg.mozilla.org/comm-central/rev/24156b0458cd
https://hg.mozilla.org/comm-central/rev/321e89baf21e
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 32.0
You need to log in
before you can comment on or make changes to this bug.
Description
•