RFC: syntax sugar for mocha tests calling multiple async functions

rfc.js

// Original code
it('does something complex', function(done) {
  async.waterfall(
    [
      function setupSchema(cb) {
        db.setupSchema(/*...*/, cb);
      },
      function createTestUser(cb) {
        db.createUser({ name: 'a user name' }, cb);
      }
      function act(data, cb) {
        this.data = data;
        db.users.edit(this.data.UserId, { name: 'new name' }, cb);
      },
      function assert(cb) {
        db.users.findById(this.data.userId, function(user) {
          expect(user.name).to.equal('new name');
          cb();
        }
      }
    ],
    done);
});

// Proposed API:
it('does something complex', [
  function setupSchema(cb) {
    db.setupSchema(/*...*/, cb);
  },
  function createTestUser(cb) {
    db.createUser({ name: 'a user name' }, cb);
  }
  function act(data, cb) {
    this.data = data;
    db.users.edit(this.data.UserId, { name: 'new name' }, cb);
  },
  function fetchNewData(cb) {
    db.users.findById(this.data.userId, cb);
  },
  function assert(user, cb) {
    expect(user.name).to.equal('new name');
    cb();
  }
]);

Why isn't the timeout shared by the whole test good enough?

If each step can timeout, you can more easily see what is causing the complex test to be slow. This would only really be true if you used named functions. Also it gives you a more realistic threshold for timing out. Given n steps I would assert that n * 2 seconds is much more reasonable for timing out than always 2 seconds no matter the complexity of the test.

Hmm, that looks useful. However, how would you distinguish between these two functions?

Good point. You can't without making this a bit too magical.

Given the savings is just hiding a call to async.waterFall(), I'm not sure I see the value.

That aside, I think I would prefer all of the setup be in a before() or accessed by mocks/helpers from within the test. Starting with a style like this:

describe('some feature set', function() {
  var db = ...;

  before(function(doneSetup) {
    async.waterfall([
      function setupSchema(cb) {
        db.setupSchema(/*...*/, cb);
      },
      function createTestUser(cb) {
        db.createUser({ name: 'a user name' }, cb);
      }],
      doneSetup);
  }

  it('updated the user', function(done) {
    db.users.edit(db.data.UserId, { name: 'new name' }, function() {
      db.users.findById(db.data.userId, function() {
        assert.equal(db.user, 'new name');
        done();
      });
    });
  });
});

Given that as a starting point, I'd rather see a simpler way to mock the DB or set the data in a mocked DB that some helper gives us out of the box in this style:

describe('some feature set', function() {

  usingDB(setupDB, function(db) {

    it('updated the user', function(done) {
      db.users.edit(db.mockedUserId, { name: 'new name' }, function() {
        db.users.findById(db.mockedUserId, function(err, user) {
        assert.equal(user, 'new name');
        done();
      });
    });

  });

  function setupDB(db, doneSetup) {
    async.waterfall([
      function setupSchema(cb) {
        db.setupSchema(/*...*/, cb);
      },
      function createTestUser(cb) {
        db.createUser({ name: 'a user name' }, function(err, user) {
          db.mockedUserId = user.id;
          cb()
        });
      }],
      doneSetup);
  }
});

Thank you all for valuable comments. Because the main reason behind the high number of waterfall steps is the code for building models (as Ryan pointed out), I decided to simplify that piece.

This is my current code:

it('sends notification to the correct device', function(done) {
  var context = {};
  async.waterfall([
    function arrange(cb) {
      context.pushManager = new PushManager();
      var builder = new TestDataBuilder();
      builder
        .def('application', Application, {
          pushSettings: { stub: { } }
        })
        .def('anotherDevice', Device, {
          appId: builder.ref('application.id'),
          deviceType: 'another-device-type'
        })
        .def('device', Device, {
          appId: builder.ref('application.id'),
          deviceType: mockery.deviceType
        })
        .def('notification', Notification)
        .buildToContext(context, cb);
    },

    function act(cb) {
      context.pushManager.pushNotificationByRegistrationId(
        context.device.id,
        context.notification,
        cb
      );
    },

    function assert(cb) {
      expect(mockery.firstPushNotificationArgs())
        .to.deep.equal([context.notification, context.device.deviceToken]);
      cb();
    }
  ], done);
});

How do you like that?

I'll leave the implementation in loopback-push-notification for now, later we can move it to a standalone module and write a blogpost on usage.

I have to agree that I'm on the "less magic" side, as @bajtos should know from my response to his email thread. That said, if you want to go down the "waterfall, building data as you go" model, let me shamelessly plug Stepdown: https://github.com/schoonology/stepdown . That's exactly what it does.