Closed Bug 813411 Opened 9 years ago Closed 8 years ago

[email/IMAP] port/import Thunderbird's IMAP and SMTP fake-servers


(Firefox OS Graveyard :: Gaia::E-Mail, defect)

Not set


(Not tracked)



(Reporter: asuth, Assigned: asuth)



(Whiteboard: [FT: Productivity], [Sprint: 1])


(1 file)

Currently IMAP unit tests are expected to be run against real IMAP servers.  I use dovecot and postfix on localhost most of the time, running specific tests against other servers as needed.  This is good, but there are a few deficiencies:

- It's hard to write unit tests involving behaviour not (normally) exhibited by the server.  For example, localized folder names, extensions only supported by gmail, UID rolls, etc.  fault_injecting_socket.js does let us fake some things, but it's labor intensive and undesirable to do that for more than a few canned lines.

- It can be hard to setup a local IMAP+SMTP setup, both for developers and for tinderbox-type machines.  While we could stand up a test server somewhere and try and point people at that, it's not the same thing as something on localhost.

So we should add an IMAP fake-server.  But since it's not real, before landing any non-trivial IMAP changes we should still run tests against one or more IMAP servers used in the real world.

Thunderbird fake-servers live here:

The Thunderbird fake-servers have various methods that will spin nested event loops (characterized by calls to "processNextEvent").  In general, we really want to avoid spinning nested event loops.
Assignee: nobody → bugmail
Blocks: 885049
Blocks: 871897
metrics for comparison:

make imap-tests (with deps):
47.28user 16.46system 1:02.18elapsed 102%CPU (0avgtext+0avgdata 189036maxresident)k
0inputs+58240outputs (0major+462718minor)pagefaults 0swaps

make activesync-tests (no deps, activesync server manually run as required):
17.65user 6.28system 0:23.44elapsed 102%CPU (0avgtext+0avgdata 178764maxresident)k
80inputs+15464outputs (0major+193437minor)pagefaults 0swaps

So our target time for all tests is a wall-clock time of no more than 1m 25.5s
All tests are green except for test_compose (which also includes some previously permanently broken activesync tests), although test_compose is now IMAP-only since it just didn't work sufficiently well on activesync.  Many character-set bugs were slain and a lot of normalization between activesync and IMAP occurred, especially in terms of real and fake server control normalization.

Current runtime (which includes some delay timeouts in the compose tests) is 1m 13.7s.

I should be able to resolve the compose tests and the ever-important SMTP-to-IMAP hookup (for the streaming tests) tomorrow.
Comment on attachment 778816 [details]
Pointer to Github pull request:

Okay, all tests are green and this includes some improved test coverage.  Most happy is that test_compose now runs under activesync after implementing sending in the activesync fake-server and bolting the mime parser onto it.  If there are any intermittent failures, they are less rare than they used to be.  (I did squash several, so it's not like I'm just crossing my fingers.)

Run-time for "make tests" is now 54 seconds, so we've improved our run-time by about 30 seconds.  "make post-tests" is the posty thing.

The list of tests and variants to run can be found in test/test-file.json where variants are listed.

While I made a serious effort to keep th_real_imap_server.js up-to-date with all the other changes I made, this patch-set currently *does not* support running against an actual IMAP server.  The main hoops for that are probably to:
- fix the inevitable glitches in th_real_imap_server
- modify loggest-chrome-runner.js and our Makefile to have a sane-ish/easy-ish way to let us specify real credentials and cause the test to run the "imap:real" variants.  This would only get more complex if we tried to have it so you could specify multiple real IMAP servers in a single go.  I don't think we want that because of how the log files get to go where they go.

The patch includes fakeserver code from Thunderbird.  I tried to do a vendor-branch thing so we can track upstream.  The vendor branch currently is this guy: .  More specifically, everything under test-runner/chrome/fakeserver is pretty much just existing comm-central or mozilla-central (httpd.js) code (or outstanding patches that have been reviewed but not yet landed), so I don't believe further review is required.  Or at least, there isn't much I can do about the code directly... Joshua Cranmer of Thunderbird fame is the person to talk to about most of that code.  Note that I have made some minor fixes that I will endeavor to upstream.

For changes to test files, in most cases these are simple mechanical-ish changes.  The most obvious ones are that I've created a 'changed' field for sync expectations for ActiveSync.  We were previously abusing 'flags' to mean one thing for IMAP (number of message flags updated; we always update them for present messages) and another for ActiveSync (the number of changed messages, since ActiveSync just tells us deltas across the whole synced folder).

I think my point there, and maybe about some of the test infrastructure changes is that I think we can get away with doing a less thorough review on this since we're changing unit tests that already worked to unit tests that still work, and the arguably overkill nature of the logging strategy means that breakage is less likely than in a sparse assertion model.

I'm asking :jrburke to review mainly because of team availability and he has had some exposure to the testing infrastructure from deuxdrop.  If others want to hop on, that would be great.  (:lightsofapollo or :squib would be the most qualified to do a deep-dive at this time, but I think they are probably particularly busy.)
Attachment #778816 - Flags: review?(jrburke)
:squib, as a heads-up I implemented sending in the activesync fake-server here.  If you want to skim to see if I did anything dumb/unrealistic, it would be appreciated.  Thanks!
Comment on attachment 778816 [details]
Pointer to Github pull request:

I added some small changes directly to the pull request in consultation with :asuth in IRC. After those changes, the tests work for me, r+ for merging.
Attachment #778816 - Flags: review?(jrburke) → review+
Blocks: 897323
Dylan Oliver changed story state to accepted in Pivotal Tracker
Whiteboard: [FT: Productivity], [Sprint: 1]
You need to log in before you can comment on or make changes to this bug.