Closed
Bug 900894
Opened 12 years ago
Closed 12 years ago
[Email] Integration test for sending email via IMAP
Categories
(Firefox OS Graveyard :: Gaia::E-Mail, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: evanxd, Assigned: evanxd)
References
Details
Attachments
(2 files)
Integration test for sending email via IMAP
Assignee | ||
Comment 1•12 years ago
|
||
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. :)
Flags: needinfo?(jlal)
Assignee | ||
Updated•12 years ago
|
Flags: needinfo?(jlal)
Flags: needinfo?(gaye)
Flags: needinfo?
Assignee | ||
Updated•12 years ago
|
Flags: needinfo?
Assignee | ||
Updated•12 years ago
|
Flags: needinfo?(gaye)
Comment 2•12 years ago
|
||
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.
Assignee | ||
Comment 3•12 years ago
|
||
Hi James,
Please help me review the patch.
Thanks. :)
Attachment #791374 -
Flags: review?(jlal)
Comment 5•12 years ago
|
||
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-
Assignee | ||
Comment 6•12 years ago
|
||
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•12 years ago
|
||
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•12 years ago
|
||
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.
Assignee | ||
Updated•12 years ago
|
Assignee | ||
Comment 9•12 years ago
|
||
The patch is waiting for Bug 906332 and Bug 907061.
Comment 10•12 years ago
|
||
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.
Assignee | ||
Comment 11•12 years ago
|
||
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.
Assignee | ||
Comment 12•12 years ago
|
||
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•12 years ago
|
||
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.
Assignee | ||
Comment 14•12 years ago
|
||
Cool! James.
Your patch works.
We could continue to review the patch.
Thanks. :)
Comment 15•12 years ago
|
||
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+
Comment 16•12 years ago
|
||
(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?
Assignee | ||
Comment 17•12 years ago
|
||
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?
Assignee | ||
Comment 18•12 years ago
|
||
Hi all,
The CI issues will be resolved, when Bug 908595 is fixed.
Assignee | ||
Comment 19•12 years ago
|
||
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•12 years ago
|
||
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•