Closed Bug 900894 Opened 8 years ago Closed 8 years ago
[Email] Integration test for sending email via IMAP
Integration test for sending email via IMAP
Hi James and Gaye, It's my WIP patch. https://github.com/evanxd/gaia/commit/aaafc811f061e14edf2a6ab01ede55f6628d2848 And I have two problems for the patch. You could see that in the GitHub page. And we could discuss that in GitHub. Thanks. :)
I've commented on the commit. Since I expect the commit will eventually get rebased into oblivion, I'll include it here since this will probably be a relevant non-obvious issue for a while to come: === Check out https://github.com/mozilla-b2g/gaia/blob/master/apps/email/js/mail_common.js#L920 and _eatingEventsUntilNextCard. willkg's integration tests dealt with that here: https://github.com/mozilla-b2g/gaia/blob/master/apps/email/test/integration/email_integration.js#L74 That logic is still sound, but I don't think we expose 'Cards' directly on the global anymore. We should, however, expose require() globally, so require('mail_common').Cards should work. For our integration tests, we could probably do something like change the transition speed of our card animations by injecting some additional CSS. Right now the animations are, of course, neat, but they will needlessly slow down our tests. Right now we set the value to 0.3s. Correctness currently does require that some transition occurs, since we listen for ontransitionend. cc: @jrburke === In terms of structuring, I think James Lal was talking about breaking out the various methods you have in EmailUtils into separate card-specific objects or classes like the python test scripts do.
Hi James, Please help me review the patch. Thanks. :)
Attachment #791374 - Flags: review?(jlal)
And really thanks for Andrew's help in Comment 2.
Comment on attachment 791374 [details] Pull request I think we want to address the server problem before we land anything. We should hook-up/use our fake-servers from the back-end. The python-based gaia-ui-tests already test our app with real IMAP and ActivesSync (Exchange, not hotmail yet, see https://github.com/mozilla/gaia-ui-tests/issues/1251) accounts, so I don't think we're immediately gaining anything with realistic tests. My primary concern is that a test that accesses the network is very likely to intermittently fail, and that's before we factor in: - gmail has very aggressive account safety heuristics that will forbid connections on a geographic basis or if it thinks things are otherwise suspicious - gmail has a limit on 15 simultaneous IMAP connections per account; we could easily start failing because of that (https://support.google.com/mail/answer/97150?hl=en) - unless the credentials are 'device specific' credentials from a 2-factor enabled account, a jerk can use the credentials to steal the account/change the password/etc.
Attachment #791374 - Flags: feedback-
HI Andrew, Your concerns totally make sense. Could you give me some clues for using our fake IMAP server in the integration test? Could I image that we could just use the fake server with launching a application and just adding a email account? Then we could send the email to the fake server and receive the email from it?
From bug 906332 it sounds like James Lal is pursuing at least part of the fake-server integration. You should coordinate with him. In general, you would want to look at th_fake_imap_server.js and fake-server-support.js and branch out from there to understand how things work. But especially for integration tests that just do a loopback test, right now if you just run "make imap-server" in GELAM it should start an SMTP server and an IMAP server and you can run the tests against that. The only tricky thing is getting the client to use the right ports for IMAP and SMTP. th_fake_imap_server.js shows an example of how to clobber the right values into place in the back-end, but that's problematic for a UI integration test to get at because accountcommon.js lives in the back-end. It might be possible to use manual config, but we need to be careful about enabling the non-SSL connection to succeed for test purposes but not letting people configure non-SSL connections in the real world.
So- as I see it my work will land with Evan's (we need it for reliable travis tests) I will try to wrap up my side of things my day time so we can unblock this and jrburkes work.
Hey Evan- I reviewed your work and submitted a patch which wires up the fakeimap stuff to your branch. There is more to do though and I listed it in my PR.
Hi James, Thanks for your patch. And I already updated my patch for using the mail-fakeservers. But we couldn't login the server. You could see the code in: https://github.com/mozilla-b2g/gaia/pull/11578/files#L0R169 When we manually setup a email account, the screen always showed "Setting up account" just like the setting-up-account attachment. I don't why, and I'm checking on this. Could help me for that? Thanks.
When we manually setup a email account, the screen always showed "Setting up account" for a while just like the setting-up-account attachment. Then the test is just timeout.
Cool! I think you missed the step where you need to hack the socket type (I mentioned this in the pull request I sent to you) I will fix it and then life should be good.
Cool! James. Your patch works. We could continue to review the patch. Thanks. :)
Comment on attachment 791374 [details] Pull request Github has died (apocalypse) for the time being so I can't finish my review but my one general comment was to remove the use of the local "var client" variable in email.js as probably move email.js to lib/. Both of these are optional my r+ carries either way... Awesome work!
Attachment #791374 - Flags: review?(jlal) → review+
(followup from IRC) There was a regression for the last 20 hours where we where not running the integration tests per commit. This is fixed now so could you rebase and make sure the integration tests pass fully prior to landing?
Hi James, Thanks for your reminding. I don't know why the "send email via IMAP" test is always timeout. https://travis-ci.org/mozilla-b2g/gaia/builds/10488314 I'm checking that. Do you know why?
Hi all, The CI issues will be resolved, when Bug 908595 is fixed.
All integration tests are passed. We could see the CI result in the https://travis-ci.org/mozilla-b2g/gaia/builds/10528828, after we updated the marionette-helper.
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.