Skip to content

Changed end() to return a Promise when called without arguments.#163

Closed
jussi-kalliokoski wants to merge 1 commit into
forwardemail:masterfrom
jussi-kalliokoski:end-promise
Closed

Changed end() to return a Promise when called without arguments.#163
jussi-kalliokoski wants to merge 1 commit into
forwardemail:masterfrom
jussi-kalliokoski:end-promise

Conversation

@jussi-kalliokoski

Copy link
Copy Markdown

This is a pretty handy thing to have, especially with Mocha, allowing you to go from this (example from supertest's own tests):

it('should support nested requests', function(done){
  var app = express();
  var test = request(app);

  app.get('/', function(req, res){
    res.send('supertest FTW!');
  });

  test
  .get('/')
  .end(function(){
    test
    .get('/')
    .end(function(err, res){
      (err === null).should.be.true;
      res.should.have.status(200);
      res.text.should.equal('supertest FTW!');
      done();
    });
  });
});

to this:

it('should support nested requests', function(){
  var app = express();
  var test = request(app);

  app.get('/', function(req, res){
    res.send('supertest FTW!');
  });

  return test
  .get('/')
  .end()
  .then(function (res) {
    return test
    .get('/')
    .end();
  }).then(function (res) {
    res.should.have.status(200);
    res.text.should.equal('supertest FTW!');
  });
});

Or, in a simpler case, the boilerplate reduction is even more obvious:

it('should do something', function (done) {
  request(app)
  .get('/')
  .expect(200)
  .end(function (err, res) {
    if (err) { return done(err); }
    done();
  });
});

vs

it('should do something', function(){
  return request(app)
  .get('/')
  .expect(200)
  .end();
});

Do note that this depends on ES6 Promises so it works only on node 0.11.x and later, but the es6-promise polyfill can be used for older versions.

@gjohnson

Copy link
Copy Markdown
Contributor

Let's keep this around until we can get superagent updated to keep them somewhat similar. It needs lots of work though, so might be a while :-/

@jakecraige

Copy link
Copy Markdown

@gjohnson What's the status of this? This would be a great addition, specifically for use with mocha

@gaastonsr

Copy link
Copy Markdown

👍

@jclem

jclem commented Dec 11, 2014

Copy link
Copy Markdown

I desperately needed this and hacked around it with this:

var Bluebird  = require('bluebird'); // Promise lib
var Test      = require('supertest/lib/test');
Test.prototype.end = Bluebird.promisify(Test.prototype.end);

@gaastonsr

Copy link
Copy Markdown

Exists a library for this for anyone interested.

@avimar

avimar commented Mar 19, 2015

Copy link
Copy Markdown

Hmm. I've also been using supertest-as-promised, which doesn't even require end() to return a promise, but I don't really know how hacky it is.
My main async calls - stripe and ORM (waterline) provide promises built-in, it would be great if my main API tester supertest also supported promises natively, since my test runner mocha is already able to handle them.

That said, supertest-as-promised seems to function great!

@arikon

arikon commented Mar 20, 2015

Copy link
Copy Markdown

It is not very hacky — you just need to add then() method to supertest's Test, that will accept callback and errback, that will call end(). And that's it.

@arikon

arikon commented Mar 20, 2015

Copy link
Copy Markdown

This is an example for Q library:

var q = require('q');

Test.prototype.then = function(onFulfilled, onRejected) {
    var self = this;
    return q.Promise(function(resolve, reject) {
        self.end(function(err, res) {
            if (err) {
                return reject(err);
            }
            resolve(res);
        });
    }).then(onFulfilled, onRejected);
};

With that fix you could use supertest like this (in mocha tests, for example)

var supertest = require('supertest');
it('should be ok', function() {
    return supertest(app).get('/').expect(200);
});

@gaastonsr

Copy link
Copy Markdown

I can confirm supertest-as-promised is a tiny wrap around supertest.

@JSteunou

JSteunou commented Jan 7, 2016

Copy link
Copy Markdown

👍 for this, would be great to see it in mainline project even if "promisified" solutions exist.

@r1b

r1b commented Jan 20, 2016

Copy link
Copy Markdown

+1

@grindfuk

Copy link
Copy Markdown

bump

@tilfin

tilfin commented Mar 9, 2016

Copy link
Copy Markdown

+1

Comment thread lib/test.js
var end = Request.prototype.end;

if (!fn) {
return new Promise(function (resolve, reject) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should probably be using any-promise.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Be aware that the .pipe() method of Superagent expects .end() to return the instance itself and not a promise. IE. this breaks .pipe(). I managed to "promisify" .end() like shown below.

var originalEnd = Test.prototype.end;

Test.prototype.end = function (callback) {
  var deferred = Promise.defer();

  originalEnd.call(this, function (err, res) {
    if (callback) {
      callback(err, res);
    }

    if (err) {
      deferred.reject(err);
    } else {
      deferred.resolve(res);
    }
  });

  this.then = deferred.promise.then.bind(deferred.promise);
  this.catch = deferred.promise.catch.bind(deferred.promise);

  return this;
};

@gabzim

gabzim commented Jun 6, 2016

Copy link
Copy Markdown

+1

2 similar comments
@yf-hk

yf-hk commented Jun 27, 2016

Copy link
Copy Markdown

+1

@jeonghwan-kim

Copy link
Copy Markdown

+1

@robinvdvleuten

Copy link
Copy Markdown

I've opened a new PR for this functionality which uses native promises by default and the .then() method as suggested by @arikon

@rimiti

rimiti commented Apr 23, 2018

Copy link
Copy Markdown
Contributor

@jussi-kalliokoski can you resolve these conflicts ? Could you please take in consideration the @PlasmaPower and @robinvdvleuten comments.

@mikelax

mikelax commented May 17, 2018

Copy link
Copy Markdown
Contributor

@rimiti I think this PR can be closed at least in favor of #380
We should separately decide what we need beyond the Promise support already in superagent.

@rimiti rimiti closed this May 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.