Closed
Bug 436847
Opened 17 years ago
Closed 17 years ago
Implement an SMTP fake server testing scheme for /mailnews
Categories
(MailNews Core :: Networking: SMTP, defect)
MailNews Core
Networking: SMTP
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: standard8, Assigned: standard8)
References
()
Details
Attachments
(2 files)
12.66 KB,
patch
|
Bienvenu
:
review+
Bienvenu
:
superreview+
|
Details | Diff | Splinter Review |
14.71 KB,
patch
|
Bienvenu
:
review+
Bienvenu
:
superreview+
|
Details | Diff | Splinter Review |
This bug is for extension of the mailnews fake servers for SMTP. This is based on the work originally done in bug 413077.
The attached patch implements a fake SMTP server and provides a test for nsISmtpService::sendMessageFile. This was the latest, most sane, place I could find before we actually start hooking into the message protocol.
I have added some documentation to nsISmtpServer and nsISmtpServer which were all things I had to find out whilst implementing the test.
In nsISmtpServer I changed the prototype from "SendMessageFile" to "sendMessageFile" to reflect how we normally do idl functions. I also changed requestDSN to aRequestDSN, and fixed some tabs.
The test checks that the messages received at the SMTP server are correct and in the right order. It currently doesn't check the content of the message is correct - I plan to do that later.
We will probably also want to provide more helper functions for various test actions, again I'll look at adding those later as I add more tests. The purpose of this patch is to get a baseline server in place.
Attachment #323349 -
Flags: superreview?(bienvenu)
Attachment #323349 -
Flags: review?(bienvenu)
Comment 1•17 years ago
|
||
Comment on attachment 323349 [details] [diff] [review]
The fix
very cool!
Is this copy+pasted from the fake news server? I don't think it applies to smtp servers.
+ this.group = null;
+ this.article = null;
super bonus points for adding a test to make sure the url listener really gets called - that's very important for extensions, and in general.
Attachment #323349 -
Flags: superreview?(bienvenu)
Attachment #323349 -
Flags: superreview+
Attachment #323349 -
Flags: review?(bienvenu)
Attachment #323349 -
Flags: review+
Assignee | ||
Comment 2•17 years ago
|
||
It turns out the first version wasn't actually working 100% correct. This was because the message1.eml file wasn't in CRLF format as per the spec. This was causing the fake server not to work right, so I've fixed the file format (hopefully!).
Also included is a bunch more documentation for nsISmtpServer.
I've extended the nsISmtpService::sendMessageFile documentation with a note about the file format.
I've moved some of the functions from the test file to the head (so that they can be shared) and implemented some slight more generic functions to make the test file cleaner and easier to set up overall.
There's a bunch of dumps removed as we don't really need them.
The fake server now doesn't return an code after each line (it shouldn't do).
On top of all that I've added a second test for the basic plain auth case. Doesn't really test much extra, but proves everything is working right now ;-)
Attachment #323724 -
Flags: superreview?(bienvenu)
Attachment #323724 -
Flags: review?(bienvenu)
Updated•17 years ago
|
Attachment #323724 -
Flags: superreview?(bienvenu)
Attachment #323724 -
Flags: superreview+
Attachment #323724 -
Flags: review?(bienvenu)
Attachment #323724 -
Flags: review+
Comment 3•17 years ago
|
||
(In reply to comment #2)
> It turns out the first version wasn't actually working 100% correct. This was
> because the message1.eml file wasn't in CRLF format as per the spec. This was
> causing the fake server not to work right, so I've fixed the file format
> (hopefully!).
In the maild test suites, the responses are sanitized to use CRLF using the following JS line:
response = response.replace(/([^\r])\n/,"$1\r\n");
However, looking at the SMTP APIs, it doesn't seem that this simple transformation is possible without creating a temporary file.
Assignee | ||
Comment 4•17 years ago
|
||
Both patches are checked in now. I'm declaring this fixed, I'll look at doing the listeners in another bug.
Status: NEW → RESOLVED
Closed: 17 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
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
•