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

Status: RESOLVED FIXED

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

Description

[Andrew Sutherland [:asuth] (he/him)]

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:
http://mxr.mozilla.org/comm-central/source/mailnews/test/fakeserver/

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.

Comment 1

[Andrew Sutherland [:asuth] (he/him)]

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

Comment 2

[Andrew Sutherland [:asuth] (he/him)]

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 3

[Andrew Sutherland [:asuth] (he/him)]

Attachment: Pointer to Github pull request: https://github.com/mozilla-b2g/gaia-email-libs-and-more/pull/224

Pointer to Github pull-request

Comment 4

[Andrew Sutherland [:asuth] (he/him)]

Comment on attachment 778816 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia-email-libs-and-more/pull/224

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: https://github.com/asutherland/gaia-email-libs-and-more/tree/thunderbird-fakeserver-vendor .  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.)

Comment 5

[Andrew Sutherland [:asuth] (he/him)]

: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 6

[James Burke [:jrburke]]

Comment on attachment 778816 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia-email-libs-and-more/pull/224

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.

Comment 7

[Andrew Sutherland [:asuth] (he/him)]

landed in gaia-email-libs-and-more/master:
https://github.com/mozilla-b2g/gaia-email-libs-and-more/pull/224
https://github.com/mozilla-b2g/gaia-email-libs-and-more/commit/edec2e82ba9c642503826a62a3802555215f8ce3

landed in gaia/master:
https://github.com/mozilla-b2g/gaia/pull/11139
https://github.com/mozilla-b2g/gaia/commit/cd5a250e64f451d074ada4b9a1b1d773d1b2d4bd

Comment 8

[Dylan Oliver [:doliver]]

Dylan Oliver changed story state to accepted in Pivotal Tracker