976127 - Implement Loop client unit tests

Status: RESOLVED FIXED

(Hello (Loop) :: Client, defect)

Description

[Adam Roach [:abr]]

We need to implement client unit tests that are run as part of our normal CI process. The Talkilla team used mocha and chai for this.

Comment 1

[Dan Mosedale (:dmosedale, :dmose)]

Spent a bunch of time talking to various folks on the a-team about testing pieces yesterday.  

I talked to some folks from the a-team yesterday, and one of the things I picked up with this. If it makes sense for us to use mocha & chai (which I think is likely to depend on discussions with at least Gavin, and probably a few others) there is a way that should be easy, but hasn't been tried before, so we'll want to do a quick spike if we're considering it.

That is, to use Marionette to drive mocha and chai, much like we've done in the Talkilla tests.  The additional step is that we would need to cause the failure messages to propagate to the TBPL test-runner (we've already implemented something like this in the old repo).

jgriffen sez:

> Test harnesses have to output test results in a format "like mochitest" in 
> order for them to be parsed correctly by TBPL.
>
> Technically, the rules are here: https://wiki.mozilla.org/Sheriffing/Job_Visibility_Policy#6.29_Outputs_failures_in_a_TBPL-starrable_format.
>
> If you were to use the current Marionette test runner, for instance, the output
> is automatically formatted for you, but if you were to use Marionette with your
> own test runner, or use a different runner entirely, you'd have to make sure
> it conforms to this.

My understanding is that using the Marionette test runner is almost certainly preferable, because it's already in TBPL. 

The other options would be to set up a new test runner, which is likely to take 2-3 weeks lead time.  I've got more notes on how we might be able to optimize that if we happen to end up that far down this particular path.

jgriffin, please feel free to correct any misunderstandings or misquotes (and thanks so much for the help).

Comment 2

[Jonathan Griffin (:jgriffin)]

That's all correct.  To let the Marionette testrunner understand that you've failed a test, an exception must be thrown, e.g., on the return value from execute_script, using self.assert(), for example.

Comment 3

[Dan Mosedale (:dmosedale, :dmose)]

After more discussions, I think we want to move forward with mocha and chai (and SinonJS, too) (driven by Marionette), for a bunch of reasons:

* it's good at structuring large sets of tests for running quickly
* our team has lots of experience with these pieces and can move very fast
* well-documented, supported, and understood on the web
* there are harnesses (b2g has one, written by jlal) that can be used locally by developers so that whenever a file gets saved, the most relevant tests are run automagically, which speeds up to the code/test/fix feedback loop by (I'd guess) an order of magniture over something like mochitest.
* it's used in b2g and the loop server as well

I'd suggest the team just set up mocha/chai/Sinon in a Makefile alongside the standalone code that they hack on over the weekend.

Mark or I can work on gluing it into the build / Marionette framework perhaps sometime next week.

Comment 4

[Dan Mosedale (:dmosedale, :dmose)]

Attachment: Link to Github pull-request: https://github.com/adamroach/gecko-dev/pull/13

Comment 5

[Dan Mosedale (:dmosedale, :dmose)]

Attachment: Link to Github pull-request: https://github.com/adamroach/gecko-dev/pull/13

Together, these two pull requests hook up both the desktop-local and the shared unit tests to be run by:

./mach marionette-test browser/components/loop/test/manifest.ini

Note that there will be one more bug filed to really complete this work, which will be to get these tests wired up in such a way that they'll actually run on Tbpl and tested.

Comment 6

[Dan Mosedale (:dmosedale, :dmose)]

Attachment: Link to Github pull-request: https://github.com/mozilla/loop-client/pull/15

Comment 7

[Dan Mosedale (:dmosedale, :dmose)]

Attachment: Link to Github pull-request: https://github.com/adamroach/gecko-dev/pull/13

Comment 8

[Mark Banner (:standard8)]

Comment on attachment 8404434 [details] [review]
Link to Github pull-request: https://github.com/mozilla/loop-client/pull/15

Looks good r=Standard8, once the final comments about the debug are assessed.

Please squash commits before landing.

Comment 9

[Mark Banner (:standard8)]

Comment on attachment 8404435 [details] [review]
Link to Github pull-request: https://github.com/adamroach/gecko-dev/pull/13

r=Standard8, please address the comments before landing & squash the commits.

Comment 10

[Mark Banner (:standard8)]

https://hg.mozilla.org/projects/elm/rev/c0af40bf3f72
https://hg.mozilla.org/projects/elm/rev/d150667d31c2

Comment 11

[Mark Banner (:standard8)]

https://hg.mozilla.org/mozilla-central/rev/c0af40bf3f72
https://hg.mozilla.org/mozilla-central/rev/d150667d31c2

Comment 12

[u279076]

Looks like this doesn't need QA before release. Please needinfo me to request specific testing.