flyd
flyd copied to clipboard
paldepind
Fix test issue as reported in #195
The issue came about because the test is asynchronous but wasn't supplied a done callback.
This lead to a swallowed error.
This commit fixes the test such that it gives the expected result and calls the done callback
Coverage remained the same at 100.0% when pulling 35cb544f60ae123f9ab06999893b1ee07dc80e4f on fix/chain-test into 180e8f5b859ac9ae388a193d334e94f03e02feef on master.
Coverage remained the same at 100.0% when pulling 3e3dfca13acf68848270393c1ea77bc301636e16 on fix/chain-test into 180e8f5b859ac9ae388a193d334e94f03e02feef on master.
Coverage remained the same at 100.0% when pulling 35cb544f60ae123f9ab06999893b1ee07dc80e4f on fix/chain-test into 180e8f5b859ac9ae388a193d334e94f03e02feef on master.
From my perspective, done callback is a mess and it's not unusual to forget it.
I always settle my system, defining something like drain(Stream<T>) → Promise<T>, then making all tests async and return drain(s) from them, where s is (in some sence) considered main instance of that test.
When I need series of values, I use concat(Stream<T>) → Promise<T[]> with similar approach: expect(await concat(s)).eq(sequence) (in async test).
@StreetStrider I do believe the test framework and eslint settings are due for an overhaul.
For instance the tests fail if I use arrow functions as that's not permissible by the es version.
With that in mind, how would you change the test?
@nordfjord with that in mind, my strategy still can be applied. Before async/await and arrows I'd just used function () { … } and return promise. mocha supports this, while async/await and arrows can be used as sugar.
@StreetStrider I've updated the PR returning a Promise instead of using done