test.promise fails where standard bluebird does not

[neverfox]

This works:

var request = noder.$di.get('authRequest');
request.getAsync({})
    .spread(function (res, body) {
        res.statusCode.must.be(200);
        body.must.be('ERROR:999:invalid command');
    })
    .catch(function (err) {
        test.fail(err.message);
    })
    .finally(done);

This does not:

test.promise
    .given(request.getAsync({}))
    .spread(function (res, body) {
        res.statusCode.must.be(200);  // Cannot read property 'statusCode' of undefined
        body.must.be('ERROR:999:invalid command');
    })
    .catch(function (err) {
        test.fail(err.message);
    })
    .finally(done)
    .done();

I promisifyAll'd request with bluebird (and set a default URL, which is why I'm only passing an empty object parameter). If test.promise is truly just bluebird, then I don't see why the second case is failing.

So I stepped through and the problem is that inside of given, it thinks the result is an object but not a Promise:

  // object (not a promise)
  if(typeof value === 'object' && !promise.is(value)) {
    return promise.props(value);
  }

But it's most definitely a Promise, straight out of bluebird (same version). In fact, this passes:

Promise.is(request.getAsync({})).must.be.true(); // Promise is a bluebird I brought in

However, this does not:

test.promise.is(request.getAsync({})).must.be.true();

[Nicolab]

Hello,

it's strange, I will test today.
Thanks for your detailed report ;)

[Nicolab]

var request = noder.$di.get('authRequest');
request.getAsync({})
    .spread(function (res, body) {
        res.statusCode.must.be(200);
        body.must.be('ERROR:999:invalid command');
    })
    .catch(function (err) {
        test.fail(err.message);
    })
    .finally(done);

Here the errors are unhandled, Bluebird catches the error and ensures that there are no error which breaks the script execution, this behavior allow to handle the errors (even in the catch method).

With Bluebird, to be able to rethrow an error, you should add .done() at the end:

var request = noder.$di.get('authRequest');
request.getAsync({})
    .spread(function (res, body) {
        res.statusCode.must.be(200);
        body.must.be('ERROR:999:invalid command');
    })
    .catch(function (err) {
        test.fail(err.message);
    })
    .finally(done)
    .done();

This works:

var test = require('unit.js');

test.$di.factory('httpRequest', function() {
  return test.promisifyAll(require('request'));
});

describe('HTTP promise', function() {

  it('should passes', function(done) {
    var request = test.$di.get('httpRequest');

    request.getAsync('http://google.com')
      .spread(function (res, body) {
        test
          .number(res.statusCode)
            .is(200)

          .string(body)
            .contains('google')
        ;
      })
      .catch(function (err) {
        test.fail(err.message);
      })
      .finally(done)
      .done();
  });

  it('should not pass', function(done) {
    var request = test.$di.get('httpRequest');

    request.getAsync()
      .spread(function (res, body) {
        test
          .number(res.statusCode)
            .is(200)

          .string(body)
            .contains('google')
        ;
      })
      .catch(function (err) {
        test.fail(err.message);
      })
      .finally(done)
      .done()
  });
});

But if I remove the .done()

it('should not pass', function(done) {
    var request = test.$di.get('httpRequest');

    request.getAsync()
      .spread(function (res, body) {
        test
          .number(res.statusCode)
            .is(200)

          .string(body)
            .contains('google')
        ;
      })
      .catch(function (err) {
        test.fail(err.message);
      })
      .finally(done);
  });

This test case passes with a warning in the console:
Possibly unhandled AssertionError: options.uri is a required argument ...

You can test.dump() the error for better visibility

.catch(function (err) {
  test.dump(err).fail(err.message);
})

---

In passing, noder is integrated in Unit.js, you can

var request = test.$di.get('authRequest');

All noder API is usable with the test variable.

[neverfox]

Yes, that was my mistake on the missing done, thanks. But that's unrelated to the matter of the promise.is check, right?

[Nicolab]

Indeed it's a subtlety of Bluebird, but it's handy especially in a context of unit tests where we need sometime rethrow and sometime just test the error.

For nonce, I just added this clarification in the doc.

... that's unrelated to the matter of the promise.is check, right?

test.promise.is() and test.promise.given() are ok.

This passes

'use strict';

var test = require('unit.js');

test.$factory('httpRequest', function() {
  return test.promisifyAll(require('request'));
});

describe('HTTP promise', function() {

  it('must be a promise', function() {
    var request = test.$di.get('httpRequest');

    test.promise.is(request.getAsync('http://google.com')).must.be.true();
  });

  it('should work with `given()`', function(done) {
    var request = test.$di.get('httpRequest');

    test.promise
      .given(request.getAsync('http://google.com'))
      .spread(function (res, body) {
        test
          .number(res.statusCode)
            .is(200)

          .string(body)
            .contains('google')
        ;
      })
      .catch(function (err) {
        test.fail(err.message);
      })
      .finally(done)
      .done();
  });
});

Maybe your problem is your service test.$di.get('authRequest') ?
How is it defined, it's a singleton ?

[neverfox]

It's a factory:

'use strict';

module.exports.__noder = function requestPlugin(noder) {
    noder.$factory('authRequest', ['config'], function (config) {
        var request = noder.request.defaults({
            url: 'http://us-proxies.com/api.php?api&uid=' +
                 config.get('US_PROXIES_UID') +
                 '&pwd=' +
                 config.get('US_PROXIES_PWD')
        });

        return noder.Promise.promisifyAll(request);
    });

    return noder;
};

noder.Promise comes from a plugin:

'use strict';

module.exports.__noder = function promisePlugin(noder) {
    noder.$require('Promise', 'bluebird');

    return noder;
};

My test spec:

'use strict';

var noder;

describe('Request', function () {

    before(function () {
        process.env.US_PROXIES_UID = '1234';
        process.env.US_PROXIES_PWD = 'abc123';
        require('../mock-api');
        noder = require('../../');
    });

    describe('authRequest', function () {

        it('should be a request that authenticates to US Proxies API', function (done) {
            var request = noder.$di.get('authRequest');

            test.promise
                .given(request.getAsync({}))
                .spread(function (res, body) {
                    res.statusCode.must.be(200);
                    body.must.be('ERROR:999:invalid command');
                })
                .catch(function (err) {
                    test.fail(err.message);
                })
                .finally(done)
                .done();

            // This works:
            // request.getAsync({})
            //  .spread(function (res, body) {
            //      res.statusCode.must.be(200);
            //      body.must.be('ERROR:999:invalid command');
            //  })
            //  .catch(function (err) {
            //      test.fail(err.message);
            //  })
            //  .finally(done)
            //  .done();
        });

    after(function () {
        delete process.env.US_PROXIES_UID;
        delete process.env.US_PROXIES_PWD;
    });

});

Perhaps yours works because the promisification is coming from the same bluebird that it being used in the test, whereas, in my case, the test.promise bluebird and the bluebird from my plugin aren't the same? That doesn't make any sense (the bluebirds in question are the same version even), but it is the only difference I can see.

[Nicolab]

Good catch, I struggled all morning. Bluebird seems impervious to a single instance. Maybe there is a solution provided for that, I have not found.

So you must use either directly request.getAsync({})

Or the same instance of Bluebird. For that, you need to install Bluebird before Unit.js. NPM will not install Bluebird in dependency of Unit.js given that it will be already in node_modules.

Personally, in a case like this, I use directly to request.getAsync ({}).
The given() method, I use it only for create a promise from

• an object
• or a function
• or empty for a serie.

Or for the context (BDD style) related to the current test.

it('Update user', function(done) {
  test.promise
   .given(request.getAsync('http://localhost:3000/api/user/42/update'))
    // ...test case
});

it('Delete user', function(done) {
  test.promise
   .given(request.getAsync('http://localhost:3000/api/user/42/delete'))
    // ...test case
});

[Nicolab]

I opened an issue Bluebird #398.

[Nicolab]

Works with test.promise.resolve() https://github.com/Nicolab/bluebird-test-case/blob/master/test/index.js#L38

So I should be able to make a patch :)

[Nicolab]

Fixed and released in the v1.1.1

[neverfox]

👍

[Nicolab]

In complement, it's also possible to write done(err) like this:

it('should pass', function(done) {
    var request = test.$di.get('httpRequest');

    request.getAsync()
      .spread(function (res, body) {
        test
          .number(res.statusCode)
            .is(200)

          .string(body)
            .contains('google')
        ;
      })
      .catch(function (err) {
        test.fail(err.message);
        done(err);
      })
      .done();
  });

With the .done() method at the end, otherwise is an unhandled error.