bustle / streaming-iterables

A Swiss army knife for async iterables. Designed to replace your streams.
https://www.npmjs.com/package/streaming-iterables
MIT License
43 stars 4 forks source link

Tests fail on Node 11 #31

Closed dotboris closed 5 years ago

dotboris commented 5 years ago

The tests fail on node 11. package.json states that this lib supports node >= 8 so in theory the tests should pass.

Here's what I suggest:

Relevant logs

$ node --version
v11.12.0
$ ./node_modules/.bin/mocha --require ts-node/register --require chai/register-assert lib/*-test.ts
# ... Skipping a the passing tests ...
  115 passing (116ms)
  2 failing

  1) flatTransform
       propagates transform errors after other generators finish:

      AssertionError: expected 1 to equal 2
      + expected - actual

      -1
      +2

      at Context.it (lib/flat-transform-test.ts:180:12)
      at runNextTicks (internal/process/next_tick.js:47:5)

  2) transform
       runs a concurrent number of functions at a time:

      AssertionError: expected 1 to equal 2
      + expected - actual

      -1
      +2

      at Context.it (lib/transform-test.ts:18:12)
      at runNextTicks (internal/process/next_tick.js:47:5)
Full logs ```sh $ node --version v11.12.0 $ ./node_modules/.bin/mocha --require ts-node/register --require chai/register-assert lib/*-test.ts batch ✓ batches async iterators ✓ batches sync iterators ✓ is curryable buffer ✓ buffers async data ✓ buffers sync data ✓ buffers sync iterables ✓ is curryable ✓ deals with an infinite size ✓ propagates errors after buffer drains sync ✓ propagates errors after buffer drains async collect ✓ collects async iterable data ✓ collects sync iterable data concat ✓ concatenates multiple async iterables ✓ concatenates multiple sync iterables ✓ concatenates mixed sync and async iterables consume ✓ consumes the entire async iterator ✓ consumes the entire sync iterator filter ✓ filters iterators ✓ filters async iterators ✓ async filters iterators ✓ is curryable flatmap ✓ flattens arrays ✓ flattens async and sync iterables ✓ ignores null and undefined values flatTransform ✓ runs a concurrent number of functions at a time ✓ allows the end to finish temporarily before the middle ✓ iterates a sync function over an async value ✓ iterates a sync function over a sync value ✓ iterates a sync function over a sync value and filters the output ✓ lets you curry the concurrency then the function ✓ lets you curry the concurrency and the function ✓ allows empty iterables ✓ allows infinite parallelism ✓ yields all values from a returned sync iterable right away and async iterable in parallel ✓ propagates source errors after the transforms have finished ✓ propagates transform errors after other transforms finish 1) propagates transform errors after other generators finish flatten ✓ flattens arrays ✓ flattens async and sync iterables ✓ flattens arrays of null and undefined values ✓ does not flatten strings fromStream (node:9970) ExperimentalWarning: Readable[Symbol.asyncIterator] is an experimental feature. This feature could change at any time ✓ takes a stream and returns an async iterable ✓ iterates empty streams getIterator ✓ gives a sync iterator for a sync iterable ✓ gives a sync iterator for a sync iterator ✓ gives an async iterator for an async iterable ✓ gives an async iterator for an async iterator map ✓ iterates a sync function over an async value ✓ iterates a sync function over a sync iterable ✓ iterates an async function over an async value ✓ iterates an async function over a sync value ✓ lets you curry a function ✓ propagates source errors after the transforms have finished merge ✓ iterates sync iterators ✓ iterates async iterators ✓ iterates iterables ✓ a mix of sync and async iterators and iterables parallelFlatMap ✓ iterates a sync function over an async value ✓ iterates a sync function over a sync iterable ✓ iterates an async function over an async value ✓ iterates an async function over a sync value ✓ lets you curry a function ✓ runs concurrent mapping operations ✓ can have a concurrency more than the items in a stream ✓ allows infinite parallelism parallelMap ✓ iterates a sync function over an async value ✓ iterates a sync function over a sync iterable ✓ iterates an async function over an async value ✓ iterates an async function over a sync value ✓ lets you curry a function ✓ runs concurrent mapping operations ✓ can have a concurrency more than the items in a stream ✓ allows infinite parallelism ✓ propagates source errors after the maps have finished ✓ propagates map errors after other maps finish parallelMerge ✓ iterates sync iterators ✓ iterates async iterators ✓ iterates iterables ✓ a mix of sync and async iterators and iterables ✓ works with node streams reduce ✓ calls an argument list of functions ✓ just invokes the 1st function reduce ✓ reduces sync functions with sync iterators ✓ reduces async functions with async iterators ✓ reduces async functions with iterables ✓ curryable take ✓ Returns the first n elements of the given async iterable ✓ Returns the first n elements of the given sync iterable ✓ lets you ask for more ✓ lets you curry the count tap ✓ iterates a sync function over an async value ✓ iterates a sync function over a sync iterable ✓ iterates an async function over an async value ✓ iterates an async function over a sync value ✓ lets you not curry the function time ✓ produces an iterator for an iterator ✓ produces an async iterator for an async iterator ✓ logs the total time to iterate ✓ logs the progress events ✓ logs the total time to iterate async ✓ logs the progress events async transform 2) runs a concurrent number of functions at a time ✓ allows the end to finish temporarily before the middle ✓ iterates a sync function over an async value ✓ iterates a sync function over a sync value ✓ lets you curry the concurrency and function ✓ lets you curry the function ✓ allows for resolving nothing ✓ works with node streams ✓ allows infinite parallelism ✓ tolerates resolving out of order ✓ propagates source errors after the transforms have finished ✓ propagates transform errors after other transforms finish writeToStream ✓ writes values to a stream ✓ respects backpressure ✓ doesn't close the stream ✓ deals with an empty write 115 passing (116ms) 2 failing 1) flatTransform propagates transform errors after other generators finish: AssertionError: expected 1 to equal 2 + expected - actual -1 +2 at Context.it (lib/flat-transform-test.ts:180:12) at runNextTicks (internal/process/next_tick.js:47:5) 2) transform runs a concurrent number of functions at a time: AssertionError: expected 1 to equal 2 + expected - actual -1 +2 at Context.it (lib/transform-test.ts:18:12) at runNextTicks (internal/process/next_tick.js:47:5) ```
dotboris commented 5 years ago

I believe that I know why this test fails on node 11 but not on node 10.

In node 11, or ordering of between setImmediate and microtasks (somePromise.then(...) & process.nextTick) has changed.

The difference is described here: https://jsblog.insiderattack.net/new-changes-to-timers-and-microtasks-from-node-v11-0-0-and-above-68d112743eb3

Since flatTransform does not guarantee the order of the resulting iterator and the failing test makes use of the setImmediate order semantics to ensure the order of execution this broke.

Good news is that this does not indicate that the lib is broken for node 11 (at least that's my impression).

What's interesting with this change ordering change is that it was done to match the behaviour of the browser. This means that this test would probably fail in the same way in the browser.

dotboris commented 5 years ago

Turns out that it's not only the flatTransform test that fails. There's also a transform test that fails as well.

I could not see the failing test because mocha was being run with --bail.

I'm updating the description with the full list of tests

reconbot commented 5 years ago

Not my impression either, thanks for the background on this. Your changes look good 👍