What is the best approach to unit test node js controller which using a service method

Question

My Controller => below controller is using reqpoertService.getFiles method and that method itself using a external APIs to call the data.

function getFiles(req, res) {
    reportService.getFiles({
        fromDate: req.query.fromdate,
        endDate: req.query.enddate,
        fileTypes: req.query.filetypes || [],
        fileStatus: req.query.filestatus || []
    })
    .then(data => {
       logger.info('-> reportService.getFiles :: Successfully fetched data',
                   resolveLogger({ statusCode: res.statusCode })
       );
       res.send(data);
   })
   .catch(err => {
       logger.error('<- OOPS :: reportService.getFiles fail to fetch data');
       res.status(statusCodes.INTERNAL_SERVER_ERROR).send({});
       logger.error('<- ERROR', resolveLogger({
           statusCode: res.statusCode,
           errMessage: err.message,
           errorStack: err
       }));
   });
}

Reporter Service

function getFiles() {
    return new Promise((resolve, reject) => {
        requestPromise(options)
            .then(data => {
                var duration = new Date - start;
                logger.info(resolveLogger({
                    duration: duration + 'ms',
                    reqUrl: options.url,
                    bodyLengh: data && data.length
                }));
                logger.info('<= Request complete successfully.');
                var resData = JSON.parse(data);
                resolve(resData);
            })
            .catch(error => {
                logger.error('=> Request failed for URL:', options.url);
                reject(error);
            });
    });
}

My Unit Test Approach to test above controller

it('METHOD: getFiles -> should response 500 without data', done => {
    nock('http://localhost:1708/fakeapi')
        .get('/files')
        .reply(statusCodes.INTERNAL_SERVER_ERROR);

    const res = buildResponse();
    const req = httpMocks.createRequest({
        method: 'GET',
        url: '/api/submitted-data/1/files'
    });

    res.on('end', function () {
        var data = res._getData();
        expect(data).toEqual({});
        expect(statusCodes.INTERNAL_SERVER_ERROR).toBe(res.statusCode);
        done();
        nock.cleanAll();
    });

    reporterController.getFiles(req, res);
});

Can someone suggest me the approach I am following is acceptable or is there any better approach to do the unit test. As I am beginner to do the unit testing.

Answer 1

I think your approach is on the right path. Your tests should be decoupled as much from the implementation as much as possible. So your test code shouldn't really know how you've implemented your code. It just cares that when you hit your endpoints, the outcome is as expected. You want to mock the external parts of your code, i.e. the code that won't execute when you run your test such as external APIs. You can mock certain responses that the external APIs so you can write tests to cover those types of scenarios and then handle them as you want.

This article from ThoughtWorks is quite helpful in explaining this approach to testing: https://www.thoughtworks.com/insights/blog/mockists-are-dead-long-live-classicists

I'd also suggest watching this video title Ian Cooper: TDD where did it all go wrong: https://vimeo.com/68375232

I appreciate my suggestion is a little high level so in short I think your test should look like:

1. establish the context of your test, setting up data etc. So in your case ensure a file exists (this could be mocked response if coming from an external api)
2. set up mocking for your external apis (to cover if the external api times out, is down, 401, 500 etc).
3. call your api
4. assert the result that your api endpoint returns

You can then have different tests checking different responses that the external apis return.

Answer 2

I have one criticism of you unit test, and it is that you aren't using beforeAll/each to actually setup your test.

What I mean by this is:

• You can use the describe block in which the "it" is nested to declare variables that will be set during the test setup (beforeEach/All) and should be the ones that you actually "expect" during the "it".
• The "it"s themselves should be cleaner and smaller, almost only being composed by expects.

Appart from that it looks good.