885055 - [email/testing] Stand up a continuous integration travis setup for gaia-email-libs-and-more using the fake-servers

Status: RESOLVED FIXED

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

Description

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

gaia-email-libs-and-more pull requests should automatically have tests run against them.  Tests should also be run against the merged code, etc.

We will target running against mozilla-central builds at minimum.

Comment 1

[Dylan Oliver [:doliver]]

A Pivotal Tracker story has been created for this Bug: http://www.pivotaltracker.com/story/show/53249479

Comment 2

[Dylan Oliver [:doliver]]

Andrew Sutherland changed story state to started in Pivotal Tracker

Comment 3

[James Lal [:lightsofapollo]]

Andrew says he probably won't have time this week- considering this is one of our user stories for this sprint it would be awesome if Evan had time to work on it this week (so we can wrap it up early).

Comment 4

[Evan Tseng [:evanxd]]

Hi all,

I would like to make this done.
And I already learned Travis stuffs to fix the bug form Yuren yesterday.

But I have a presentation in Firefox OS training in Mozilla tomorrow.
I'm preparing that.

After I finish preparing the presentation, I will continue to work on this bug.
Thanks. :)

Comment 5

[Dylan Oliver [:doliver]]

Dylan Oliver changed story state to unstarted in Pivotal Tracker

Comment 6

[Evan Tseng [:evanxd]]

Hi Andrew,

It's my WIP patch.
https://github.com/evanxd/gaia-email-libs-and-more/commit/e5b663d015a7ea629c29beca5f696570ae48633f

And I have two problems.
1. How to stop the process of running unit tests when one unit test is failed? Then I could show the fail in Travis.
2. How to show the less log when run the unit test? The log is too much for showing in Travis.

Do you know how to fix it?

Comment 7

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

(In reply to Evan Tseng [:evanxd] from comment #6)
> 1. How to stop the process of running unit tests when one unit test is
> failed? Then I could show the fail in Travis.

Do we really want to stop when we experience the first failure for Travis?  It seems useful to still run all tests.

Having said that, a stop-at-first-failure mode could be useful; the relevant bits:

runTests() in loggest-chrome-runner.js is the logic that decides what tests to run and to keep running them:
https://github.com/mozilla-b2g/gaia-email-libs-and-more/blob/master/test-runner/chrome/content/loggest-chrome-runner.js#L1035

The runNextTest function in there gets a summary of the most recent run created by summaryFromLoggest() earlier in the file.  You could change that method to bubble up that any failure occurred to the top-level of the summary object and then check that and decide whether to be all done.


> 2. How to show the less log when run the unit test? The log is too much for
> showing in Travis.
> 
> Do you know how to fix it?

There's a number of sources of log spew:

- output from the chrome loggest runner; actual output is via https://github.com/mozilla-b2g/gaia-email-libs-and-more/blob/master/test-runner/chrome/content/console-helper.js although the place to control it would be from in: https://github.com/mozilla-b2g/gaia-email-libs-and-more/blob/master/test-runner/chrome/content/loggest-chrome-runner.js

- SUPER_DEBUG output from the testdriver in the worker thread, enabled by SUPER_DEBUG being non-null here (or more specifically, where it is passed into the driver down below): https://github.com/mozilla-b2g/gaia-email-libs-and-more/blob/master/test/loggest-runner-worker.js#L35

- console.log from the main thread in the test (LOG/ERR/INF/WAR): https://github.com/mozilla-b2g/gaia-email-libs-and-more/blob/master/test/loggest-runner.html

- console.log from the worker thread in the test (WLOG/WERR/WINF/WWAR): https://github.com/mozilla-b2g/gaia-email-libs-and-more/blob/master/test/worker-bootstrap.js


I think the ideal for me would be if we could control these output sources independently, ideally via command-line argument processed by loggest-chrome-runner which would then set things in its own scope or relay them into the content page which would then also relay them into the worker.  The 'testParams' value in _runTestFile in loggest-chrome-runner.js gets relayed everywhere, so that could be a good place to put it.  For example: "--test-framework-log-level", "--test-driver-log-level", and "--test-log-level" might be good.  They could take log/info/warn/error was the levels they support, etc.

By default, all of those logs would be turned off, since, as you say, they can get very verbose.

Comment 8

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

Per vidyo discussion with Evan, the key problem he was raising for point 1 was that we always return a zero error code.  Unfortunately, this is a real firefox/b2g-desktop platform issue that is unlikely to be fixed in platform.  Bug 692161 was about doing this and it got resolved WONTFIX for complexity reasons.

The summary of my investigation is that b2g's nsBrowserApp.cpp decides its return value by running nsAppRunner.cpp's XRE_main() function.  That function's return value ends up being the return value of nsIAppStartup::Run().  Unfortunately that gets hard-coded to be either NS_SUCCESS_RESTART_APP or NS_OK.
http://mxr.mozilla.org/mozilla-central/source/toolkit/xre/nsAppRunner.cpp#3858
http://mxr.mozilla.org/mozilla-central/source/toolkit/components/startup/nsAppStartup.cpp#274

The idiom adopted by jetpack was just to write its success value to a file and then the runner checks that:
http://mxr.mozilla.org/mozilla-central/source/addon-sdk/source/lib/sdk/system.js#52

I'm afraid we probably need to do that.  The other alternative is to use grep or something like that and just have us print out an unambiguous string that grep can trigger on for result code purposes.  For the writing case, before we call quitApp() we'd want to make sure we've written the file to disk.  If using something async like OS.File.writeAtomic we wwant to make sure to wait for the promise to be resolved before triggering shutdown.  The OS.File docs are at https://developer.mozilla.org/en-US/docs/JavaScript_OS.File/OS.File_for_the_main_thread if you want to check out other options.

Comment 9

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

Er, and for scripting purposes, I think we might be able to just do something like:

  exit `cat gelam-exit-code`

or something like that.  (We can bounce it through a variable too.)

Comment 10

[Evan Tseng [:evanxd]]

Attachment: pull-request.html

Hi Andrew,

Please help me review the patch.

And you could see the result on the Travis.
https://travis-ci.org/evanxd/gaia-email-libs-and-more

Thanks. :)

Comment 11

[Evan Tseng [:evanxd]]

Hi Andrew,

I have problem of enabling/disabling the log in https://github.com/mozilla-b2g/gaia-email-libs-and-more/blob/master/test/worker-bootstrap.js.

But for the spring 1, I think maybe we could skip that first. And make it done in next spring.

How do you think?

Comment 12

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

Comment on attachment 783722 [details]
pull-request.html

It's great to see this working! Woo!

So, controlling the log output is not the most important thing, but your re-ordering of console-helper.js to come after loggest-chrome-runner.js in test-runner.xul is incorrect.  loggest-chrome-runner.js uses console.*.  Things aren't breaking because the system is providing a 'console' global, but the net result is that all of our log data gets eaten and we can't do anything about it and I think that's too unintuitive.

As I mentioned on IRC, I think the right way to turn-off the console logging is to set a flag on the console instance.  That way we can avoid some re-ordering.

For example, if we chnaged console-helper.js to look more like the following (The _enabled bits are new).
===
function consoleHelper() {
  if (!this._enabled)
    return;
  var msg = arguments[0] + ':';
  for (var i = 1; i < arguments.length; i++) {
    msg += ' ' + arguments[i];
  }
  msg += '\x1b[0m\n';
  dump(msg);
}
window.console = {
  _enabled: true,
  log: consoleHelper.bind(null, '\x1b[32mLOG'),
  error: consoleHelper.bind(null, '\x1b[31mERR'),
  info: consoleHelper.bind(null, '\x1b[36mINF'),
  warn: consoleHelper.bind(null, '\x1b[33mWAR'),
  harness: consoleHelper.bind(null, '\x1b[36mRUN')
};
===

This has the advantage that it allows us to change the logging on the fly.

Adding lightsofapollo to review the mozilla-download usage in case he has any idiom preferences.

Comment 13

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

er, I was a little too quick with the changes to the window.console example.  the 'this' argument of our bind call needs to be "window.console".

Comment 14

[Evan Tseng [:evanxd]]

Hi Andrew,

I already updated my patch for your comments.
And I added the enable/disable function in worker-bootstrap.js.

Please review the patch.
Thanks. :)

Comment 15

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

Comment on attachment 783722 [details]
pull-request.html

conditional r=asuth granted with follow-ups that I did and have jrburke review on irc.  landed now, pasting those in a sec.

Comment 16

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

landed on gaia-email-libs-and-more/master:
https://github.com/mozilla-b2g/gaia-email-libs-and-more/pull/230
https://github.com/mozilla-b2g/gaia-email-libs-and-more/commit/4cf3717e20b5eec47305bff46efab53dbb7a54d3

Note that although we got a green build on the merge, we did get a failure in test_linkify's plaintext case that's weird.  So we already have a likely intermittent orange.  I have another pull request that will give us a little more details.