Simplify async code-flows and unify unit tests

[Mike de Boer]

The thing I always do when I encounter a large code base, is roam though it and make a mental map of the structure; the good parts and ones that would benefit from some TLC.

A part I noticed did not yet receive the amount of attention it deserves are unit tests and asynchronous code handling.

I'll be blunt about it, here's what I did: I ported the async NodeJS library to a Mozilla-style module and on top of that I built a unit testing framework, inspired by Mocha (among others).

I can imagine that the fellas I just mentioned don't make you recall anything, so here are two links:

• https://github.com/caolan/async
• https://github.com/visionmedia/mocha

First of all, the Github numbers will show you that they are crazy popular NodeJS libraries. And well-documented. And well-written. And awesome.

Here's the thing I ended up with: https://github.com/mikedeboer/mozAsync

The lib folder therein contains two files:

• Async.jsm - the port of the async library
• AsyncTest.jsm - the unit testing library

The test folder contains the unit tests (erm, duh) and also serve as great examples of how to use the two modules!

Ok, example time! I really like this one from the async module:

// Sorts a list by the results of running each value through an async iterator.

Async.sortBy(["file1", "file2", "file3"], function(file, callback){

  fs.stat(file, function(err, stats){

    callback(err, stats.mtime);

  });

}, function(err, results){

  // results is now the original array of files sorted by

  // modified date

});

Note: this one uses a NodeJS API, but you can mentally replace that with an async MozFile one.

I've told some of you before that I'm not a big fan of Promise libraries (if not, please read the 10 reasons to NOT use a Promise library: https://gist.github.com/mikedeboer/5305020). However, this library is not meant to replace them, but to augment the ecosystem with another option. There are many developers out there that feel uneasy about using Promises as long as they're not a first-class primitive in SpiderMonkey.

If that's you, you can use this: a minimalistic, functional utility library.

What about them unit tests?

Design goal: create a simple, minimalistic framework that provides a clear, unified API to create unit tests that is as easy to use for sync code flows as for async ones.

Rationale: unit tests are 'hot' code. They are modified as often - perhaps even more - as functionality it covers changes, especially in TDD environments. They serve as documentation for the code they cover. When tests fail, they usually don't on 'your computer', but somewhere else, like on the build infrastructure. When that happens, someone will open the unit test and try to understand what is going on. In all these cases it is hugely beneficial to a) know that most of the unit tests are written in the same structure and b) are structured in such a way that they're easy to read by someone other than you.

This is an example of a minimal test:

AsyncTest([

{

  name: "Minimal Test",

  reporter: "tap",

  tests: {

    "it should execute this test": function(next) {

      Assert.equal(typeof next, "function", "'next' should be a callback function");

      next();

    },

    "! it should NOT execute this test": function(next) {

      Assert.ok(false, "BAM!");

      next();

    },

    "it should be aware of the correct context": function() {

      Assert.ok(this["it should execute this test"], "The function ought to be accessible");

    }

  }

}

]);

There are a couple of interesting things going on here:

• You can pass an Array of test suites or just one Object
• Suites can have names
• Tests can be described freely
• It mixes fine with ANY assertion style (Mochi or XPCShell)
• Prefix a test with ! to exclude it from the suite
• Prefix a test with > to only run that test and ignore the others
• If the test is sync, you can forget about the next function (callback) completely - the framework takes care of it; you can even mix async and non-async tests
• You can choose the style of reporting by setting the reporter property to dot or tap. The reporters progress and spec are under development. More information about what 'TAP' is can be found at: http://testanything.org

But more importantly, there are several things that usually need to happen before a test can be run, like opening a tab, load a page and wait for it to load, etc. AsyncTest unifies scenarios like this in the following way:

• Each suite may have one or more of the following functions:
• setUpSuite - run only once before any test is executed
• setUp - run once before each test
• tearDownSuite - run only once when all tests are done
• tearDown - run once after each test
• These functions are executed in the context of the tests, so the this is the same as the this in test functions

This takes care of all the flows a test suite might need to implement.

To top this off, you can set a flag in each suite: notify: true to present you with a Growl notification once the suite is done to report the results!

That shows that this library is meant to bring back the fun into creating unit tests.

Have fun and please let me know what you think!

Mike.

[Jared Wein]

Hi Mike,

Thanks for writing this up. Please see my comments inline.

----- Original Message -----
> From: "Mike de Boer" <mde...@mozilla.com>
>
> What about them unit tests?
>
> Design goal: create a simple, minimalistic framework that provides a
> clear, unified API to create unit tests that is as easy to use for
> sync code flows as for async ones.
>
> Rationale: unit tests are 'hot' code. They are modified as often -
> perhaps even more - as functionality it covers changes, especially
> in TDD environments. They serve as documentation for the code they
> cover. When tests fail, they usually don't on 'your computer', but
> somewhere else, like on the build infrastructure. When that happens,
> someone will open the unit test and try to understand what is going
> on.
>
> In all these cases it is hugely beneficial to a) know that most
> of the unit tests are written in the same structure and b) are
> structured in such a way that they're easy to read by someone other
> than you.

It seems to me that adding another testing framework exacerbates the problem of unit tests not being written in the same structure.

I believe that some of the features of this unit testing framework actually make it quite hard to read by someone else. For example, below you mention that tests can be commented out by placing an exclamation point at the beginning of the test description. This seems like a very obscure and non-obvious trick that won't be caught by many people without extensive documentation reading.

>
> This is an example of a minimal test:
>
>
> AsyncTest([
> {
> name: "Minimal Test",
> reporter: "tap",
> tests: {
> "it should execute this test": function(next) {
> Assert.equal(typeof next, "function", "'next' should be a callback
> function");
> next();
> },
> "! it should NOT execute this test": function(next) {
> Assert.ok(false, "BAM!");
> next();
> },
> "it should be aware of the correct context": function() {
> Assert.ok(this["it should execute this test"], "The function ought to
> be accessible");
> }
> }
> }
> ]);
>

Have you tried coupling in the Mochitest asserts? As it currently stands, the example of using the NodeJS Assert's aren't that useful for seeing how hard it might be to get this integrated in our setup.

It's unclear to me how AsyncTest provides something that isn't available today with Mochitest. What I do see though is that adopting another testing framework will create more work for the people who work on TBPL as well as developers who need to learn the special rules and syntax of another testing framework.

Thanks,
Jared
_______________________________________________
firefox-dev mailing list
firef...@mozilla.org
https://mail.mozilla.org/listinfo/firefox-dev

[Paolo Amadini]

On 22/04/2013 18.11, Jared Wein wrote:
> What I do see though is that adopting another testing framework will create more work for the people who work on TBPL as well as developers who need to learn the special rules and syntax of another testing framework.

I agree. And I think an alternative good first step would be to unify
our Assert syntax as much as possible across all our frameworks.
(That's easier than unifying how the suite calls into tests.)

It might even be the NodeJS Assert syntax - you could start a proposal
on a wiki page mapping old asserts to new ones, and discuss from there.

Cheers,
Paolo

[Mike de Boer]

Hi Jared,

Thanks for the feedback! Please read my comments inline too...

On Apr 22, 2013, at 6:11 PM, Jared Wein <jw...@mozilla.com> wrote:

> Hi Mike,
>
> Thanks for writing this up. Please see my comments inline.
>
> ----- Original Message -----
>> From: "Mike de Boer" <mde...@mozilla.com>
>>
>> What about them unit tests?
>>
>> Design goal: create a simple, minimalistic framework that provides a
>> clear, unified API to create unit tests that is as easy to use for
>> sync code flows as for async ones.
>>
>> Rationale: unit tests are 'hot' code. They are modified as often -
>> perhaps even more - as functionality it covers changes, especially
>> in TDD environments. They serve as documentation for the code they
>> cover. When tests fail, they usually don't on 'your computer', but
>> somewhere else, like on the build infrastructure. When that happens,
>> someone will open the unit test and try to understand what is going
>> on.
>>
>> In all these cases it is hugely beneficial to a) know that most
>> of the unit tests are written in the same structure and b) are
>> structured in such a way that they're easy to read by someone other
>> than you.
>
> It seems to me that adding another testing framework exacerbates the problem of unit tests not being written in the same structure.
>
> I believe that some of the features of this unit testing framework actually make it quite hard to read by someone else. For example, below you mention that tests can be commented out by placing an exclamation point at the beginning of the test description. This seems like a very obscure and non-obvious trick that won't be caught by many people without extensive documentation reading.

I'm afraid you misunderstood me: this is not a unit testing framework, but a library that will enable us to write unit tests in a uniform format in *together* with the existing test frameworks: Mochitest and XPCShell. Please note that I'm using NodeJS style assertions here as an example. Perhaps I picked that one poorly, but I hope that you can fill in the gap.

The most important thing about commenting out a unit test with a "!" is to allow more flexibility during a TDD workflow. This whole thing is meant as a tool for developers, not TBPL maintainers. Another example is the Growl notifications; they would not work and not make any sense on our build servers, it's just there to make your life a tad easier when doing TDD.

The "<" in the test name is meant to signal out a test case and run only that specific one. This is something that is practically useful when doing TDD.

>
>>
>> This is an example of a minimal test:
>>
>>
>> AsyncTest([
>> {
>> name: "Minimal Test",
>> reporter: "tap",
>> tests: {
>> "it should execute this test": function(next) {
>> Assert.equal(typeof next, "function", "'next' should be a callback
>> function");
>> next();
>> },
>> "! it should NOT execute this test": function(next) {
>> Assert.ok(false, "BAM!");
>> next();
>> },
>> "it should be aware of the correct context": function() {
>> Assert.ok(this["it should execute this test"], "The function ought to
>> be accessible");
>> }
>> }
>> }
>> ]);
>>
>
> Have you tried coupling in the Mochitest asserts? As it currently stands, the example of using the NodeJS Assert's aren't that useful for seeing how hard it might be to get this integrated in our setup.

Looks like I already spilled the beans above :) Like I said… the assertion style in a matter of choice; AsyncTest conforms to whatever (as it's not a framework)

>
> It's unclear to me how AsyncTest provides something that isn't available today with Mochitest. What I do see though is that adopting another testing framework will create more work for the people who work on TBPL as well as developers who need to learn the special rules and syntax of another testing framework.

I hope you can see now that this thing is not a framework, but a simple library that would improve our code quality if we were to make more out of the benefits that TDD and other XP practices have to offer.

>
> Thanks,
> Jared


Mike.

[Jared Wein]

Hi Mike,

Thank you for correcting me regarding framework vs. library. Please see my responses inline.

What is your response to this?

> > I believe that some of the features of this unit testing framework
> > actually make it quite hard to read by someone else. For example,
> > below you mention that tests can be commented out by placing an
> > exclamation point at the beginning of the test description. This
> > seems like a very obscure and non-obvious trick that won't be
> > caught by many people without extensive documentation reading.
>
> I'm afraid you misunderstood me: this is not a unit testing
> framework, but a library that will enable us to write unit tests in
> a uniform format in *together* with the existing test frameworks:
> Mochitest and XPCShell. Please note that I'm using NodeJS style
> assertions here as an example. Perhaps I picked that one poorly, but
> I hope that you can fill in the gap.
>
> The most important thing about commenting out a unit test with a "!"
> is to allow more flexibility during a TDD workflow. This whole thing
> is meant as a tool for developers, not TBPL maintainers. Another
> example is the Growl notifications; they would not work and not make
> any sense on our build servers, it's just there to make your life a
> tad easier when doing TDD.
>
> The "<" in the test name is meant to signal out a test case and run
> only that specific one. This is something that is practically useful
> when doing TDD.

Many test frameworks will stop the test after failing the first assertion. Mochitest doesn't, so disabling a test while authoring isn't so much of a necessity. Regardless, we also have todo() which you could use while writing the test if you so please.

I think this conversation is too hard to discuss in the abstract. The sample code provided doesn't relate close enough to how we actually write tests.

This may be better discussed if we use a test from mozilla-central as an example. The following test could have been written before the code-under-test was written, aka TDD:

http://mxr.mozilla.org/mozilla-central/source/browser/components/sessionstore/test/browser_pageshow.js

How would this testing library make this test easier to read? What issues do you see with this test?

Thanks,
Jared

[David Rajchenbach-Teller]

On 4/22/13 1:23 PM, Mike de Boer wrote:
> The thing I always do when I encounter a large code base, is roam though
> it and make a mental map of the structure; the good parts and ones that
> would benefit from some TLC.

As you might be aware, we are currently reworking considerable chunks of
our codebase and unit tests to make them use Promise and Task.jsm.
Please don't break our ongoing effort :)

On the other hand, if you want to join Project Async, I'll be glad to
add you to the roster.

Cheers,
David

--
David Rajchenbach-Teller, PhD
Performance Team, Mozilla

[Gregory Szorc]

On 4/22/13 4:23 AM, Mike de Boer wrote:
> A part I noticed did not yet receive the amount of attention it deserves
> are unit tests and asynchronous code handling.
>
> I'll be blunt about it, here's what I did: I ported the async NodeJS
> library to a Mozilla-style module and on top of that I built a unit
> testing framework, inspired by Mocha (among others).

+1000 for realizing how lacking our testing frameworks are (especially
mochitest) and for doing something about it. I've been wanting to
introduce a unified testing framework to the tree for ages and this
could very well be the start of that. I even have a draft post to this
list outlining why I think we should do this. Perhaps I should post it
to this thread...

People will obviously cite the XKCD that says "now we have N + 1
standards." I don't buy that argument here because I don't believe we
currently have a JS testing framework in the tree that can easily be
converted to something sane like what you've coded. By "sane," I mean
something that facilitates decent test control flow (declaration,
setup/teardown functions, a decent collection of assertion functions,
etc) and can be leveraged across *all* (or at least most) of our testing
harnesses.

Python's unittest [1] module comes to mind as a testing framework that
gets it right. Just define a class with methods that are test functions
and it just works. I see this is roughly what you've done. Yay!

It is important to note that multiple testing frameworks can exist in
tree simultaneously, even within the same testing harness. A few years
ago, xpcshell tests were defined like mochitests are now. You had to
chain and advance test functions manually. It was tedious. It was ugly.
Then philikon came along in bug 648367 and added add_test() and
run_next_test(). Then I added add_task() in bug 819033 to support native
Task.jsm test functions. The result: most new xpcshell tests use these
functions instead of manual chaining because, well, they are obviously
superior.

I propose something similar with a new testing framework. Land the
testing framework in the tree. Hook it up to the harnesses such that it
"just works." Then, watch as new tests are written using the superior
tool. You get some cognitive dissonance by introducing a new framework,
sure. But, a superior and unified testing framework greatly outweighs
that temporary setback. Before long, people will be looking back at
mochitests written today and will say "writing these tests is so tedious
- why did we put up with this for so long?"

Because I dig too deep into your code, have you seen
/dom/imptests/testharness.js? At a cursory glance it seems very similar
and I /think/ may even be standardized.

[1] http://docs.python.org/2/library/unittest.html

[Paolo Amadini]

On 22/04/2013 20.19, Gregory Szorc wrote:
> I propose something similar with a new testing framework. Land the
> testing framework in the tree. Hook it up to the harnesses such that it
> "just works." Then, watch as new tests are written using the superior
> tool.

I'd suggest starting with unifying our Assert syntax, making a
consistent one available across all our frameworks. That's a bit
easier than unifying how the suite calls into tests (though that
can be part of the overall plan).

It might even be the NodeJS Assert syntax - you could start a proposal
on a wiki page mapping old asserts to new ones, and discuss from there.

Cheers,
Paolo

[This is a repost as I'm not sure my previous reply got to the list.]

[Gregory Szorc]

On 4/22/13 1:14 PM, Paolo Amadini wrote:
> On 22/04/2013 20.19, Gregory Szorc wrote:
>> I propose something similar with a new testing framework. Land the
>> testing framework in the tree. Hook it up to the harnesses such that it
>> "just works." Then, watch as new tests are written using the superior
>> tool.
>
> I'd suggest starting with unifying our Assert syntax, making a
> consistent one available across all our frameworks. That's a bit
> easier than unifying how the suite calls into tests (though that
> can be part of the overall plan).

I think it's all the same. For example, where do the assert/testing
functions actually live? If we go with the current mechanism for
declaring tests, they live as global functions. Boo symbol pollution in
the global namespace. If I had a nickle every time I accidentally
redefined a symbol exported by the test harness... Whereas if adopt a
define-the-tests-on-test-objects method, presumably those symbols could
be attached to the prototype of the test object.

Also, I highly recommend making the "assert" terminology similar across
test harnesses. An "assert" is a fatal test failure. An "expectation" is
a non-fatal test failure. e.g. this.assertEq(x, y) vs this.expectEq(x,
y). This whole
asserts-may-or-may-not-be-fatal-depending-on-the-build-configuration-or-test-harness-or-phase-of-the-moon
is quite silly and IMO needs to be stopped. If we write a new testing
library, we have the ability to get it right without and historical baggage.

We definitely should wiki a list of all the testing functions we wish to
implement. We should also define things like parameter order (is the
expected value first or second) so the UI around test output can be clearer.

[Paolo Amadini]

On 22/04/2013 22.29, Gregory Szorc wrote:
> I think it's all the same. For example, where do the assert/testing
> functions actually live? If we go with the current mechanism for
> declaring tests, they live as global functions.

That's a good point. We may need at least two entry points, one global
for tests declared "the current way", and a different one for the
define-the-tests-on-test-objects harnesses.

But the name of the function (.assertEq, etc) would be the same. We
would get consistency without limiting the use of the new assert
functions to tests using the define-the-tests-on-test-objects mode.

> Boo symbol pollution in
> the global namespace. If I had a nickle every time I accidentally
> redefined a symbol exported by the test harness... Whereas if adopt a
> define-the-tests-on-test-objects method, presumably those symbols could
> be attached to the prototype of the test object.

A non-redefinable getter named "Assert" might work for the global case.
(And the same protection should be implemented for the case the symbol
is attached to the test object.)

> An "assert" is a fatal test failure. An "expectation" is
> a non-fatal test failure. e.g. this.assertEq(x, y) vs this.expectEq(x,
> y).

How is a fatal assert different from a non-fatal expectation? Does the
benefit of having both types outweigh the cost of having to choose
which one to use?

> We should also define things like parameter order (is the
> expected value first or second) so the UI around test output can be
> clearer.

That's exactly what I mean by unification! :-)

Cheers,
Paolo

[Gregory Szorc]

On 4/22/13 1:48 PM, Paolo Amadini wrote:
>> An "assert" is a fatal test failure. An "expectation" is
>> a non-fatal test failure. e.g. this.assertEq(x, y) vs this.expectEq(x,
>> y).
>
> How is a fatal assert different from a non-fatal expectation? Does the
> benefit of having both types outweigh the cost of having to choose
> which one to use?

A fatal assert aborts the current test function and marks it as failure.
It may or may not abort all remaining test functions in that suite. A
non-fatal expectation by contrast will continue with execution as if
nothing happened. Non-fatal expectations should be reserved for things
you likely don't have control over, such as the wall clock execution
time, number of "assertions" in layout, etc. I concede it is sometimes
difficult to know which to use when.

There is an interesting open question: what should an assertion abort:
an individual test function or the entire suite? I don't think there is
a right or wrong answer here. A compromise without introducing another
failure mode is to abort the suite but to allow multiple suites to be
defined per file. We need to wiki all of this...

[Mike de Boer]

On Apr 22, 2013, at 7:50 PM, David Rajchenbach-Teller <dte...@mozilla.com> wrote:

> On 4/22/13 1:23 PM, Mike de Boer wrote:
>> The thing I always do when I encounter a large code base, is roam though
>> it and make a mental map of the structure; the good parts and ones that
>> would benefit from some TLC.
>
> As you might be aware, we are currently reworking considerable chunks of
> our codebase and unit tests to make them use Promise and Task.jsm.
> Please don't break our ongoing effort :)
>

I'm afraid that if we keep using Task.jsm the way we currently do, the amount of test failures due to timing issues will continue to grow. Before we collectively open a can of worms, let's say that I'd be more than happy to explain what I mean by that off-topic!

> On the other hand, if you want to join Project Async, I'll be glad to
> add you to the roster.

If I can be of service, please do!

[Mike de Boer]

I see I have done a poor job at showing a good & proper example, so please take a look at this:

• https://github.com/mikedeboer/mozAsync/blob/master/examples/browser_aboutHome.orig.js
• https://github.com/mikedeboer/mozAsync/blob/master/examples/browser_aboutHome.js

This is the browser_aboutHome.js unit test code vs. a rewritten version with AsyncTest.

The NLOC difference is minimal (~100 lines), but the more significant improvement here is the structure that is proposed as uniform.

Mike.

[Mike de Boer]

On Apr 22, 2013, at 8:19 PM, Gregory Szorc <g...@mozilla.com> wrote:

> On 4/22/13 4:23 AM, Mike de Boer wrote:
>> A part I noticed did not yet receive the amount of attention it deserves
>> are unit tests and asynchronous code handling.
>>
>> I'll be blunt about it, here's what I did: I ported the async NodeJS
>> library to a Mozilla-style module and on top of that I built a unit
>> testing framework, inspired by Mocha (among others).
>

> +1000 for realizing how lacking our testing frameworks are (especially mochitest) and for doing something about it. I've been wanting to introduce a unified testing framework to the tree for ages and this could very well be the start of that. I even have a draft post to this list outlining why I think we should do this. Perhaps I should post it to this thread…

Please, feel free to share it, you now know I'm interested! +1

>
> People will obviously cite the XKCD that says "now we have N + 1 standards." I don't buy that argument here because I don't believe we currently have a JS testing framework in the tree that can easily be converted to something sane like what you've coded. By "sane," I mean something that facilitates decent test control flow (declaration, setup/teardown functions, a decent collection of assertion functions, etc) and can be leveraged across *all* (or at least most) of our testing harnesses.
>

This is not (yet) an attempt to solve that issue, but boy, would that be a cool future! I think this is feasible, though, even when done incrementally.

> Python's unittest [1] module comes to mind as a testing framework that gets it right. Just define a class with methods that are test functions and it just works. I see this is roughly what you've done. Yay!
>
> It is important to note that multiple testing frameworks can exist in tree simultaneously, even within the same testing harness. A few years ago, xpcshell tests were defined like mochitests are now. You had to chain and advance test functions manually. It was tedious. It was ugly. Then philikon came along in bug 648367 and added add_test() and run_next_test(). Then I added add_task() in bug 819033 to support native Task.jsm test functions. The result: most new xpcshell tests use these functions instead of manual chaining because, well, they are obviously superior.
>
> I propose something similar with a new testing framework. Land the testing framework in the tree. Hook it up to the harnesses such that it "just works." Then, watch as new tests are written using the superior tool. You get some cognitive dissonance by introducing a new framework, sure. But, a superior and unified testing framework greatly outweighs that temporary setback. Before long, people will be looking back at mochitests written today and will say "writing these tests is so tedious - why did we put up with this for so long?"
>
> Because I dig too deep into your code, have you seen /dom/imptests/testharness.js? At a cursory glance it seems very similar and I /think/ may even be standardized.
>

I will certainly look at this, thanks!

> [1] http://docs.python.org/2/library/unittest.html

[Marco Bonardo]

On 22/04/13 23:23, Mike de Boer wrote:

I see I have done a poor job at showing a good & proper example, so please take a look at this:

• https://github.com/mikedeboer/mozAsync/blob/master/examples/browser_aboutHome.orig.js
• https://github.com/mikedeboer/mozAsync/blob/master/examples/browser_aboutHome.js

This is the browser_aboutHome.js unit test code vs. a rewritten version with AsyncTest.

The NLOC difference is minimal (~100 lines), but the more significant improvement here is the structure that is proposed as uniform.

I love the idea of a standardized organization of the tests, though I honestly find the original implementation with Task far easier to follow in this example. What I'd really love is something like add_task that made xpcshell tests much better to read and write.
We should probably try to get the best out of the 2 worlds, keep the simple sequential logic of Task with a better defined way to declare the tests and unified async utils.

-m