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:

RESOLVED FIXED in Thunderbird 32.0

Status

defect
--
critical
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: jcranmer, Assigned: jcranmer)

Tracking

({intermittent-failure})

Trunk
Thunderbird 32.0
x86_64
Linux
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(5 attachments)

Assignee

Description

5 years ago
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

5 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

5 years ago
Basically every test that ain't using asyncTestUtils.js
Attachment #8432181 - Flags: review?(standard8)
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 5

5 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

5 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

5 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

5 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

5 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

5 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 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+
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

5 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

5 years ago
Attachment #8432181 - Attachment description: Fix the simple failures → Fix the simple failures [checkin: comment 4]
Comment hidden (Legacy TBPL/Treeherder Robot)
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

5 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: 5 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.