Support alternative testing frameworks such as Ava

[Widdershin]

From @kristianmandrup on March 17, 2017 20:39

Which test frameworks are currently supported? Please specify.
I'd like to use ava
Would just require support for async/await for Time.run() I think?
Thanks.

// Ava test
test('Counter', async t => {
  const Time = mockTimeSource();

  Time.assertEqual(count$, expectedCount$)

  await Time.run();
});

In the code for time-driver.js, I see the following

_runVirtually: function (done, timeToRunTo) so it looks like it currently requires a done callback argument for proper async

But then I also see time-source where the cb argument is optional

export interface MockTimeSource extends TimeSource {
  // ...
  run (cb?: (err?: Error) => void): void;
}

Copied from original issue: cyclejs/time#21

[Widdershin]

From @staltz on March 17, 2017 22:12

I think Time.run() is synchronous, so you could drop the async/await. Give it a try

[Widdershin]

@staltz Time.run() is asynchronous, just really fast!

My proposed solution to this is to return a promise from Time.run(), along with supporting callback style.

That would support Ava and a few other common frameworks.

We should also clearly document which testing frameworks are supported.

[Widdershin]

From @kristianmandrup on March 20, 2017 7:59

Thanks guys. It worked for me as shown above. I also created a small DSL for using RxJS marble testing with Ava.

[Widdershin]

After having a crack at implementing this, my only concern is that the usage would look like:

Time.run().then(success, fail);

Currently Time.run() will default to raising errors if no callback is passed. This is to prevent users forgetting to pass a callback and then having tests silently fail. In the above scenario, if the test fails, an error would be raised but fail would never be called.

So for this to work, we would need to make a breaking change such that Time.run() will no longer throw an error by default. This is probably fine for tests, as your test will timeout in this scenario anyway.

Another option is to make another run method that returns a promise.

My current preference is to bite the bullet and make this breaking change, but if anyone has feedback or ideas I'd like to hear it.

[Widdershin]

From @jvanbruegge on March 28, 2017 5:17

I will have to wrap Time.run() in a Promise anyway, so if for it

[Widdershin]

From @mightyiam on April 3, 2017 9:4

Regardless of what the API is or should be, why should this library have to support any testing framework specifically?

And AVA can work with callback-based APIs, as well.

[Widdershin]

From @jvanbruegge on April 3, 2017 9:5

No it should not, but having the choice about API type (callback or promise) is best, see #27

[Widdershin]

From @mightyiam on April 3, 2017 9:9

Regarding the API choice, what I see in the scene is promise-based APIs replacing callback-based APIs. With such small user base, why should we keep the callback API?

[Widdershin]

From @jvanbruegge on April 3, 2017 9:10

It does not hurt, because the Promise is just wrapping the callback anyway we don't have code duplication

[Widdershin]

I now realized Mocha supports promise style tests as well, which is nice.

I'm generally in favor of dropping callbacks and replacing them with promises.

[Widdershin]

From @staltz on April 3, 2017 11:56

:(
Callbacks are supported everywhere.
Have you considered Tape users?

[Widdershin]

From @mightyiam on April 3, 2017 12:5

Is Tape allergic to promises?

[Widdershin]

From @jvanbruegge on April 3, 2017 12:20

@mightyiam tape-testing/tape#262 (comment)

[Widdershin]

Okay, I've been thinking about this a fair bit. My biggest concern with some of the proposed solutions are around providing APIs that allow users to write tests that silently fail or don't run.

For example, if we change Time.run() to not blow up by default when a user does not pass a callback, there are two different cases where this could go wrong.

When using callback style, if the user forgets to take in the done callback and pass it to Time.run.

it('fails silently because the user forgot to take done and pass it in', () => {
  const Time = mockTimeSource();

  Time.assertEqual(
    Time.diagram('---a----b---c---'),
    Time.diagram('---x----y---z---')
  );

  Time.run();
});

Alternatively, if you are using the promise style, if you forget to return the promise the test will fail silently.

This also makes me think about another pattern I have been using. I dislike having to make a Time instance and call run in every test, so I have been writing tests like this:

it('is fun to write tests with less boilerplate', withTime(Time => {
  Time.assertEqual(
    Time.diagram('---a----b---c---'),
    Time.diagram('---x----y---z---')
  );
}));

Where the withTime helper is defined like this (for callback style tests):

function withTime (test) {
  return function (done) {
    const Time = mockTimeSource();

    test(Time);

    Time.run(done);
  }
}

We could also make a promise style withTime:

function withTime (test) {
  return function () {
    const Time = mockTimeSource();

    test(Time);

    return promisify(Time.run);
  }
}

My proposal is that instead of adding direct promise support, we instead add these helpers, probably two different ones (one for callback style, one for promise style).

This cuts down on potential user error, and removes boilerplate from the tests.

What do y'all think? Any objections to this approach? Can anyone think of a better name than withTime?

I would ensure that it supports:

• Mocha
• Jest
• Karma
• Tape
• Ava

[Widdershin]

From @jvanbruegge on April 17, 2017 14:55

I just tested that approach and it works quite good for me:

it('should interact correctly', () => {
    const property = forall(diagramArbitrary, diagramArbitrary, (addDiagram, subtractDiagram) => withTime(Time => {
        const add$ : Stream<any> = Time.diagram(addDiagram);
        const subtract$ : Stream<any> = Time.diagram(subtractDiagram);

        const DOM = mockDOMSource({
            '.add': { click: add$ },
            '.subtract': { click: subtract$ }
        });

        const app = onionify(App)({ DOM } as any);
        const html$ = app.DOM.map(toHtml);

        const expected$ = xs.merge(add$.mapTo(+1), subtract$.mapTo(-1))
            .fold((acc, curr) => acc + curr, 0)
            .map(expectedHTML);

        Time.assertEqual(html$, expected$, htmlLooksLike);
    }));

    return assert(property, testOptions);
});

(this is using the promise version)

[Widdershin]

Thinking about it more, we could perhaps only have one version that handles both styles automagically. I'll have a play and see if any of the above frameworks get upset if you both attempt to take a callback and return a promise.

[Widdershin]

From @mightyiam on April 17, 2017 19:54

@Widdershin thank you for investigating. Perhaps authors of said frameworks would be kind to advise if you mention them.

[IssueHuntBot]

@issuehuntfest has funded $20.00 to this issue. See it on IssueHunt