caolan / highland

High-level streams library for Node.js and the browser
https://caolan.github.io/highland
Apache License 2.0
3.42k stars 147 forks source link

Best Practice for Writing Unit Tests & Consuming Highland Streams? #567

Open jaidetree opened 8 years ago

jaidetree commented 8 years ago

I'm writing some mocha unit tests to test my reusable through pipeline files. However, I am not certain which is the best way to test them. The problem seems to stem from consuming the stream, ideally:

So far the best I have been able to come up with is:

it('Should work really really well', (done) => {
    _([ buildState ])
      .through(createBuildStream)
      .toCallback((err, state) => {
        expect(err).toBe(null);
        expect(state).toExist();
        expect(state.get('id')).toBe(1);

        done(err, state);
      });
});

It's important that assertions run always because when I was using the .each or .tap method I was getting false positives if nothing made it down stream and the assertions were not run at all. That is at least where .toCallback helps however some of my streams return multiple items over time in which .toCallback will not work.

I've been able to simplify a little bit by monkey-patching in some custom methods to ease testing:

it('Should work really really well', () => {
    return _([ buildState ])
      .through(createBuildStream)
      .inspect((err, state) => {
        expect(err).toBe(null);
        expect(state).toExist();
        expect(state.get('id')).toBe(1);
      })
     .toPromise();
});

Where inspect works a lot like toCallback except that it fires per each item coming down the stream and if no value was received it fires if it receives nil. Is there a better way to go about testing highland streams?

vqvu commented 8 years ago

Highland's own tests does assertion counting to verify that the correct number of assets are run, which eliminates the false positives that you mention. I'm not sure if mocha has something similar.

You could also use collect to handle multiple items.

it('Should work really really well', (done) => {
  _([ buildState ])
    .through(createBuildStream)
    .collect()
    .toCallback((err, stateArray) => {
      // Verify that the whole array is correct.
      ...
      done();
    });
});

Other than that, there's not a much better way to test highland streams than what you've identified. Feel free to use custom helpers if you feel like they simplify testing.

Just note that monkey-patching streams is not officially supported (it'll probably work but we don't promise not to break it in future updates). You can accomplish (practically) the same result by using through.

function inspect(cb) {
  return stream => stream
    .consume((err, x, push, next) => {
      cb(err, x);
      push(err, x);
      if (x !== _.nil) {
        next();
      }
    });
}

function toPromise(stream) {
  // Similar to what you have now. Just use `stream` instead of `this`.
}

it('Should work really really well', () => {
  return _([ buildState ])
      .through(createBuildStream)
      .through(inspect((err, state) => {
        expect(err).toBe(null);
        expect(state).toExist();
        expect(state.get('id')).toBe(1);
      }))
      .through(toPromise);
});
jaidetree commented 8 years ago

Ah yes, since this is for an internal app at work I'm not too worried about Highland's extensibility implementation as we can always lock the version down to current where it works or update the plugins since they exist in one place for now that are only used by the unit tests.

Since this is labeled cookbook I'll share my implementation for inspect & toPromise should anyone find them of value.

let _ = require('highland');

let proto = Object.getPrototypeOf(_());

function addMethod (methodName, func) {
  _.extend({ [methodName]: func }, proto);
}

addMethod('toPromise', function toPromise () {
  let self = this;

  return new Promise((resolve, reject) => {
    self.consume((err, result, push, next) => {
      if (err !== null) return reject(err);
      resolve(result);
      next();
    }).resume();
  });
});

addMethod('inspect', function expect (cb) {
  let hasValue = false;

  return this.consume((err, x, push, next) => {
    if (err) {
      throw err;
    }
    else if (x === _.nil) {
      // ensure our cb gets called at least once
      if (!hasValue) cb();
      push(null, x);
    }
    else {
      hasValue = true;
      cb(null, x);
      push(null, x);
      next();
    }
  });
});