437192 - Implement a POP fake server testing scheme for /mailnews

Status: RESOLVED FIXED

(MailNews Core :: Networking: POP, defect)

Description

[Mark Banner (:standard8)]

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.

Comment 1

[Mark Banner (:standard8)]

Attachment: The fix

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.

Comment 2

[David :Bienvenu]

         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;
+    }

Comment 3

[timeless]

it's actually harmless. NS_RELEASE will null the arg, and delete is null safe.

but yes, it's bad code :)

Comment 4

[Mark Banner (:standard8)]

Attachment: The fix v2

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.

Comment 5

[David :Bienvenu]

I believe an octet is 8 bits, one byte.

Comment 6

[Mark Banner (:standard8)]

Attachment: The fix v3

Dropped the divide by eight. Not needed, testing the messages are actually downloaded completely is a later job ;-)

Comment 7

[David :Bienvenu]

Comment on attachment 330042 [details] [diff] [review]
The fix v3

cool, this will be good to have.

Comment 8

[Mark Banner (:standard8)]

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.