8

I'm trying to test the behavior of a particular route. It continues to run the middleware even when I create a stub. I want the event authentication to simply pass for now. I understand that it's not truly a "unit" test at this point. I'm getting there. I've also simplified the code a little. Here is the code to test:

const { rejectUnauthenticated } = require('../modules/event-authentication.middleware');

router.get('/event', rejectUnauthenticated, (req, res) => {
  res.sendStatus(200);
});

Here is the middleware I am trying to skip:

const rejectUnauthenticated = async (req, res, next) => {
  const { secretKey } = req.query;
  if (secretKey) {
    next();
  } else {
    res.status(403).send('Forbidden. Must include Secret Key for Event.');
  }
};

module.exports = {
  rejectUnauthenticated,
};

The test file:

const chai = require('chai');
const chaiHttp = require('chai-http');
const sinon = require('sinon');
let app;
const authenticationMiddleware = require('../server/modules/event-authentication.middleware');

const { expect } = chai;
chai.use(chaiHttp);

describe('with correct secret key', () => {
  it('should return bracket', (done) => {
    sinon.stub(authenticationMiddleware, 'rejectUnauthenticated')
      .callsFake(async (req, res, next) => next());

    app = require('../server/server.js');

    chai.request(app)
      .get('/code-championship/registrant/event')
      .end((err, response) => {
        expect(response).to.have.status(200);
        authenticationMiddleware.rejectUnauthenticated.restore();
        done();
      });
  });
});

I've tried following other similar questions like this: How to mock middleware in Express to skip authentication for unit test? and this: node express es6 sinon stubbing middleware not working but I'm still getting the 403 from the middleware that should be skipped. I also ran the tests in debug mode, so I know the middleware function that should be stubbed is still running.

Is this an issue with stubbing my code? Is this an ES6 issue?

Can I restructure my code or the test to make this work?

Luke Schlangen
  • 3,722
  • 4
  • 34
  • 69

3 Answers3

10

There is an issue indeed with stubbing your code.

When you require your server file

const app = require('../server/server.js');

your app is get created with the whole set of middlewares, including rejectUnauthenticated, and a reference to the latter is stored inside app.

When you do

sinon.stub(authenticationMiddleware, 'rejectUnauthenticated')
  .callsFake(async (req, res, next) => next());

you replace the rejectUnauthenticated exported method of authenticationMiddleware module, but not the reference to original rejectUnauthenticated that is already stored.

The solution is to create the app (i.e. require('../server/server.js');) after you mock the exoprted middleware method:

const chai = require('chai');
const chaiHttp = require('chai-http');
const sinon = require('sinon');

// don't create app right away
let app;
const authenticationMiddleware = require('../server/modules/event-authentication.middleware');

const { expect } = chai;
chai.use(chaiHttp);

describe('with correct secret key', () => {
  it('should return bracket', (done) => {
    sinon.stub(authenticationMiddleware, 'rejectUnauthenticated')
      .callsFake(async (req, res, next) => next());

    // method is stubbed, you can create app now
    app = require('../server/server.js');

    chai.request(app)
      .get('/code-championship/registrant/event')
      .end((err, response) => {
        expect(response).to.have.status(200);
        authenticationMiddleware.rejectUnauthenticated.restore();
        done();
      });
  });
});
Sergey Lapin
  • 2,633
  • 2
  • 18
  • 20
  • If the import path for `authenticationMiddleware` in the `app` matched the import path used in the unit test, would that work without requiring the `app` to be imported after the `authenticationMiddleware`? – Chris Shouts Dec 19 '18 at 19:32
  • @Sergey I appreciate the response, and I think this concept makes a lot of sense, but it still bypasses the stub and runs the initial code. – Luke Schlangen Dec 19 '18 at 19:34
  • @ChrisShouts there is no requirement for imports to match, because node resolves modules by their absolute paths. It would not work without requiring `app` imported after middleware, at least I don't see an option to. – Sergey Lapin Dec 19 '18 at 19:54
  • 3
    @LukeSchlangen that may be affected by other tests you run in the same process - if any other test requires `server.js`, then the reference to it (and middleware method too) got cached by node, and requiring again in test will just return cached instance. My suggestion would be to run the test in isolation, and console.log `Object.keys(require.cache)` before requiring `server.js` to ensure it is not cached already. – Sergey Lapin Dec 19 '18 at 19:58
  • 4
    Yes! That was it! None of the tests in the current file were the problem, but the tests from the other files that were requiring the `server.js` file were causing it to break! Thank you so much! – Luke Schlangen Dec 19 '18 at 20:18
  • 2
    @LukeSchlangen cool, glad to help. One more comment - with `.restore()` you don't actually get everything back to initial state, because module gets cached - the same issue you faced initially, but contrariwise. In this case you might want to either use a test runner that runs each file in separate a context (afaik `jest` does that) or make use of tool like `decache` to reset node module cache each time. – Sergey Lapin Dec 19 '18 at 20:41
  • 1
    I did switch to Jest this afternoon and the code was much simpler. – Luke Schlangen Dec 19 '18 at 22:23
3

Based on @Sergey's suggestion, I did switch to Jest. At least for this specific case, it greatly simplified the implementation. For those interested, here is the end result:

const express = require('express');
const request = require('supertest');
const registrantRouter = require('../server/routers/registrant.router');

jest.mock('../server/modules/event-authentication.middleware');
const { rejectUnauthenticated } = require('../server/modules/event-authentication.middleware');

const initRegistrantRouter = () => {
  const app = express();
  app.use(registrantRouter);
  return app;
};

describe('GET /registrant', () => {
  test('It should 200 if event authentication passes', async (done) => {
    const app = initRegistrantRouter();
    rejectUnauthenticated.mockImplementation((req, res, next) => next());
    const res = await request(app).get('/event');
    expect(res).toHaveProperty('status', 200);
    done();
  });
  test('It should 403 if event authentication fails', async (done) => {
    const app = initRegistrantRouter();
    rejectUnauthenticated.mockImplementation((req, res) => res.sendStatus(403));
    const res = await request(app).get('/event');
    expect(res).toHaveProperty('status', 403);
    done();
  });
});

Thanks also to this helpful blog post about testing express apps with Jest: https://codewithhugo.com/testing-an-express-app-with-supertest-moxios-and-jest/

Luke Schlangen
  • 3,722
  • 4
  • 34
  • 69
1

I have tried the way proposed by the accepted answer, but it didn't work for me. Something that worked, though, was splitting the middleware by using a hat/helper function in the following way:

/** Middleware used by routes */
export const rejectUnauthenticated = async (req, res, next) => 
  mockableRejectUnauthenticated(req, res, next)

/** Actual implementation. It can be mocked in tests to bypass authentication */
export const mockableRejectUnauthenticated = async (req, res, next) => {
  const { secretKey } = req.query;
  if (secretKey) {
    next();
  } else {
    res.status(403).send('Forbidden. Must include Secret Key for Event.');
  }
};

In this way, you can just mock mockableRejectUnauthenticated in your tests and don't have to worry about the instantiation order, caching or the references stored in express.

Until now, this has been my successful go-to strategy when I need to solve mocking issues.

Alvin Sartor
  • 2,249
  • 4
  • 20
  • 36