Supertest: .expect() with custom assertion method always succeeds

[freak4pc]

it('Should have response, time, code', function(done){
        apis.get('/')
            .expect('Content-Type', /json/)
            .expect(406)
            .expect(function(res){
                return true
                /*
                if(!('responses' in res.body)) return "Missing response key"
                if(!('timed' in res.body)) return "Missing time key"
                if(!('codes' in res.body)) return "Missing code key"*/
            })
            .end(done)
    })

My test is always successful even when returning true from the method - which should fail.
My other attempts at tackling these are commented in the code.

[freak4pc]

PS, throwing an exception does produce the right result.

e.g.

it('Should have response, time, code', function(done){
        apis.get('/')
            .expect('Content-Type', /json/)
            .expect(406)
            .expect(function(res){
                if(!('response' in res.body))   throw new Error("Missing response key")
                if(!('time' in res.body))       throw new Error("Missing time key")
                if(!('code' in res.body))       throw new Error("Missing code key")
            })
            .end(done)
    })

[bblack]

This is because the callback passed to .end gets called before any of the expect stuff does. This was equally frustrating for me to figure out.

[freak4pc]

This is still considered a bug...
The docs state that as long as the method returns Truthy the test should fail, but in reality only a thrown exception does that

[AlexZeitler]

@freak4pc Did you find a solution for this?

[freak4pc]

Nope. Only throwing an exception manually works for me.

[AlexZeitler]

Just tried this and it works:

var request = require('supertest');
var app = require('../app.js');
var assert = require("assert")

describe('GET /api/customers', function () {
    it('responds with customers', function (done) {
        request(app).get('/' +
            'api/customers')
            .expect('Content-Type', /json/)
            .expect(function (res) {
                assert.equal(res.body[0].name,'visionmedia');
            })
            .end(done)
    });
});

Does this make sense?

[freak4pc]

Clearly this works because assert/should/etc throw an exception on an error...

The docs say I could just return true and that should cause the test to fail... That situation doesn't happen

[AlexZeitler]

Right, they should be fixed or at least there should be an agreement that somebody can fix them by PR.

[ivanvpan]

So if you look at the code, the documentation is clearly wrong. Actually, according to "blame" this has been the case for more than a year.
https://github.com/visionmedia/supertest/blob/master/lib/test.js#L218

Errors are thrown _only_ if:

• you return a Error object
• you throw an Error

The commit that changed it is this one:
https://github.com/visionmedia/supertest/commit/e2d7fcc9a0e8a07fd51f9cef584ad00a317186dc

The reason it lists is "This presents problems in coffeescript". That does not sound like a good reason to modify the behavior of the function, and what's worse it does not include a Readme change. I feel the change should be reverted.

[gshively11]

:+1:, this just bit us. We were returning strings instead of Errors, per the documentation.

[FlorianLoch]

I fall about that too - why doesn't the README get fixed? Testing is crucial and testing libraries should therefore be well tested and documented.

[FlorianLoch]

Why does no maintainer care for this issue?

[freak4pc]

I'm not even sure a PR would help since the maintainer isn't answering this issue ... Throwing an exception would be your best bet. @FlorianLoch

[masterl]

The problem with throwing an error is that mocha doesn't run the afterEach function.

I'm using supertest to test my API, so after each test it should clear the database so each test has a clean environment, but when I throw on a function test, afterEach does not get called, so another test fails on the next run due to the database having data...

I guess I'll change to beforeEach then. =/

[hellatan]

i've been running into issues with the .expect method not actually making correct assertions as well. Say I was expecting a header named "Header Name" with the value "yes". Even when I would do this:

// expecting "Header Name" equals "yes"
agent.get('/')
    .expect('Header Name', 'no') // will pass
    .end(done);

It would always pass. I didn't dig into the source code, but I was able to make this always fail by rewriting the test like this:

// expecting "Header Name" equals "yes"
agent.get('/')
    .end((err, res) => {
        expect("Header Name").toBe("no"); // will fail
        done();
    });

It seems like the problem with this, however, is that if an assertion fails, it stops the rest of the tests from executing. Not sure how to get around that issue, but I'd much rather have proper failing tests than to have all tests run and "pass."

[finferflu]

This seems to be still an issue:

const request = require('supertest')
const app = require('../app')
const server = request.agent(app)

describe('GET /', function () {
  it('returns 200', function (done) {
    server
    .get('/')
    .expect(500, done)
  })
})

I wrote the above to intentionally fail the test, however it passes.

However, if I use a promise, it fails as expected:

describe('GET /', function () {
  it('returns 200', function (done) {
    server
    .get('/')
    .expect(500)
    .then(function () {
      done()
    })
    .catch(err => {
      done.fail(err)
    })
  })

Output:

Failures:
1) GET / returns 200
  Message:
    Failed: expected 500 "Internal Server Error", got 200 "OK"

[juliancoleman]

My tests will always pass, even if I expect them to fail, whether I use Promises or not

[mambax]

For anyone stumbling upon this 2020, I just found (even in the docs, they just don't annotate it clear):

    return request(app.getHttpServer())
      .get(url)
      .query({month: 'july'})
      .expect(200)
      .then((res) => {
        assert.strictEqual(res.body.price, 8, '9$')
      });

This assertion fails, 8 !== 9.

While this always passes:

    return request(app.getHttpServer())
      .get(url)
      .query({month: 'july'})
      .expect(200)
      .expect((res) => {
        assert.strictEqual(res.body.price, 8, '9$')
      });

The difference?
The assert is in a .then block and not an expect...

Section: You can also use promises

[Pointotech]

@mambax Do you understand why the two different code blocks that you posted give a different result? Because I certainly don't. Is this library maintained?

[Pointotech]

I feel like deterministic results are really important in a testing library, and this just seems random.

[Pointotech]

Anyone know why this is closed?

[freak4pc]

@Pointotech I imagine because they updated the docs to indicate you need to return Errors and not strings:

https://github.com/brigand/supertest/commit/058c4c008aae5342c8674b52eb39cb9a1548ab40#diff-1e290ac8433d555bce009b162cb869d0L174-R189

[mambax]

@Pointotech @freak4pc
While I from programmatic perspective totally understand why then works I don't fully understand why expect does not.
If I am not mistaken a failing assertion is an Error (AssertionError)...? So basically assert.fail() would mean there is an error thrown... imo.

That you can process it in then is logical as its a normal promise you validate and supertest/jest wait for it as its a non-resolved promise yet. As the assertion in then fails the promise fails and thus the test itself.

I wonder if that is on purpose by supertest. The fact that its even written like so in their docs makes me feel custom matchers need to be then-ed.