Closed
Bug 437192
Opened 17 years ago
Closed 17 years ago
Implement a POP fake server testing scheme for /mailnews
Categories
(MailNews Core :: Networking: POP, defect)
MailNews Core
Networking: POP
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla1.9.1a1
People
(Reporter: standard8, Assigned: standard8)
References
()
Details
Attachments
(1 file, 2 obsolete files)
11.85 KB,
patch
|
Bienvenu
:
review+
|
Details | Diff | Splinter Review |
This bug is for implementing a POP fake server testing scheme for /mailnews.
See URL above for background information on xpcshell tests.
Bug 413077 implemented the original fake server for news, Bug 436847 extended it for SMTP and is therefore a good reference for this bug.
Assignee | ||
Comment 1•17 years ago
|
||
Here's a basic pop3 fakeserver implementation and some tests.
It performs very basic authentication, and some checks for new mail, including downloading a message.
I had to change one processNextEvent call in maild.js not to wait for events, due to an occasional issue where the test would hang on exit - the timing sometimes means that we don't have any events left to process when we get to that stage.
Assignee | ||
Updated•17 years ago
|
Keywords: helpwanted
Comment 2•17 years ago
|
||
if(NS_FAILED(rv))
{
+ NS_RELEASE(protocol);
delete protocol;
return rv;
this seems wrong - the NS_RELEASE looks like it would force the ref count to 0, which would cause the protocol to delete itself, so the second delete would probably crash. You could lose the "delete protocol" line, I think.
Indentation looks funny here:
+ var thread = gThreadManager.currentThread;
+ while (thread.hasPendingEvents())
+ thread.processNextEvent(true);
+
+ // Let OnStopRunningUrl return cleanly before doing anything else.
+ do_timeout(0, "checkBusy();");
and here:
+ this.expectingData = false;
+ return "250 Wonderful article, your style is gorgeous!";
+ }
+ }
+
+ if (this.data) {
+ if (line[0] == '.')
+ line = line.substring(1);
+ this.post += line+'\n';
+ }
why divide the length by 8? I think the size is the same as length, as long as the lines are CRLF terminated...
+ for (var i = 0; i < this._messages.length; ++i) {
+ this._messages[i].size = this._messages[i].length / 8;
+ this._totalMessageSize += this._messages[i].size;
+ }
it's actually harmless. NS_RELEASE will null the arg, and delete is null safe.
but yes, it's bad code :)
Assignee | ||
Comment 4•17 years ago
|
||
Updated patch.
The size of the message is divided by eight to put it into octets, as required by RFC 1939 in the STAT reponse.
Attachment #329722 -
Attachment is obsolete: true
Attachment #330012 -
Flags: review?(bienvenu)
Attachment #329722 -
Flags: review?(bienvenu)
Comment 5•17 years ago
|
||
I believe an octet is 8 bits, one byte.
Assignee | ||
Comment 6•17 years ago
|
||
Dropped the divide by eight. Not needed, testing the messages are actually downloaded completely is a later job ;-)
Attachment #330012 -
Attachment is obsolete: true
Attachment #330042 -
Flags: review?(bienvenu)
Attachment #330012 -
Flags: review?(bienvenu)
Comment 7•17 years ago
|
||
Comment on attachment 330042 [details] [diff] [review]
The fix v3
cool, this will be good to have.
Attachment #330042 -
Flags: review?(bienvenu) → review+
Assignee | ||
Comment 8•17 years ago
|
||
When I checked this in, the tests were initially failing on Linux. This is because the xpcshell was trying to create the new mail windows, and failing. So I've disabled the following prefs in the test, and I've now re-enabled the test as well.
mail.biff.play_sound
mail.biff.show_alert
mail.biff.show_tray_icon
mail.biff.animate_dock_icon
I probably didn't need to disable all of them, but at least we should be safe.
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Summary: Implement an POP fake server testing scheme for /mailnews → Implement a POP fake server testing scheme for /mailnews
Target Milestone: --- → mozilla1.9.1a1
Updated•16 years ago
|
Product: Core → MailNews Core
You need to log in
before you can comment on or make changes to this bug.
Description
•