900894 - [Email] Integration test for sending email via IMAP

Status: RESOLVED FIXED

Description

[Evan Tseng [:evanxd]]

Integration test for sending email via IMAP

Comment 1

[Evan Tseng [:evanxd]]

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. :)

Comment 2

[Andrew Sutherland [:asuth]]

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.

Comment 3

[Evan Tseng [:evanxd]]

Created attachment 791374 [details]
Pull request

Hi James,

Please help me review the patch.
Thanks. :)

Comment 4

[Evan Tseng [:evanxd]]

And really thanks for Andrew's help in Comment 2.

Comment 5

[Andrew Sutherland [:asuth]]

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.

Comment 6

[Evan Tseng [:evanxd]]

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?

Comment 7

[Andrew Sutherland [:asuth]]

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.

Comment 8

[James Lal [:lightsofapollo]]

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.

Comment 9

[Evan Tseng [:evanxd]]

The patch is waiting for Bug 906332 and Bug 907061.

Comment 10

[James Lal [:lightsofapollo]]

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.

Comment 11

[Evan Tseng [:evanxd]]

Created attachment 793533 [details]
setting-up-account

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.

Comment 12

[Evan Tseng [:evanxd]]

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.

Comment 13

[James Lal [:lightsofapollo]]

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.

Comment 14

[Evan Tseng [:evanxd]]

Cool! James.
Your patch works.

We could continue to review the patch.
Thanks. :)

Comment 15

[James Lal [:lightsofapollo]]

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!

Comment 16

[James Lal [:lightsofapollo]]

(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?

Comment 17

[Evan Tseng [:evanxd]]

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?

Comment 18

[Evan Tseng [:evanxd]]

Hi all,

The CI issues will be resolved, when Bug 908595 is fixed.

Comment 19

[Evan Tseng [:evanxd]]

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.

Comment 20

[James Lal [:lightsofapollo]]

https://github.com/mozilla-b2g/gaia/commit/5910944bc3cd4a263299431a3832c5582325f393