Unit Testing Expressjs Controller (Part 2)

Detailed Example Of Express Mongoose Update API

This is part 2 on the topic of Expressjs controller unit testing. Previously I added unit tests for 5 basic CRUD APIs for vehicle. In this post I’ll show one more example, where controller logic gets a little longer.

If you haven’t already, please check part 1 before continuing as I’ll build upon the same controller and spec files. You can also get the full code from github repository .


New API Requirement

Let’s assume another entity Driver. New requirement states that any vehicle can have multiple drivers assigned to it but not more than three at a time. Also, a driver can only be assigned if available. It’s a one to many relationship where Vehicle has many Drivers; minimum of 0 and maximum of 3.

Mongoose Schemas

Since we have an upper limit, we can comfortably model this relationship by keeping an array of driver references on vehicle side. It’s a good idea to add validation on drivers length in schema also, but we need to manage this on API side anyway so let’s skip mongoose validation.

Our slightly modified Mongoose schema then will be:

'use strict';

const mongoose = require('mongoose');
const Schema = mongoose.Schema

var VehicleSchema = new Schema({
    name: { type: String, required: true },
    model: { type: String, required: true },
    manufacturer: { type: String, required: true },
    drivers: [{ type: Schema.Types.ObjectId, ref: 'Driver' }]
});

module.exports = mongoose.model('Vehicle', VehicleSchema);

And driver schema.

'use strict';

const mongoose = require('mongoose');
const Schema = mongoose.Schema;

var DriverSchema = new Schema({
    name: { type: String, required: true },
    licenseNum: { type: String, required: true },
    ssn: { type: String, required: true },
    available: { type: Boolean, default: true }
});

module.exports = mongoose.model('Driver', DriverSchema);

Controller

Add controller method by the name of assignDriver.

Controller.assignDriver = function (req, res) {

};

Router Definition

Here router definition can’t exactly be RESTful because we need to use additional verb. It’ll be a PUT call and take both vehicle id and driver id as params.

router.put('/:id/assign-driver/:driverId', Controller.assignDriver);

Test File Setup

Using the same test file from previous tutorial, let’s describe our new test “assignDriver” and add beforeEach:

const sinon = require('sinon');
const Controller = require('./vehicle.controller')
const Driver = require('../driver/driver.model')
const Vehicle = require('./vehicle.model')

describe('Vehicle Controller', function () {
    let req = {
        body: {
            manufacturer: "Toyota",
            name: "Camry",
            model: "2018",
        },
        params: {
            id: "5aa06bb80738152cfd536fdc",
            driverId: "5aa13452e1e2c3277688e734"
        }
    },
        error = new Error({ error: "blah blah" }),
        res = {}, expectedResult;

    // Other CRUD tests from last post

       describe('assignDriver', function () {
        const expectedDriver = { _id: req.params.driverId, available: true };
        beforeEach(function () {
            res = {
                json: sinon.spy(),
                status: sinon.stub().returns({ end: sinon.spy(), json: sinon.spy() }) //json required here to respond with message in addition to 404
            };
            expectedResult = req.body;
            expectedResult.drivers = [req.params.driverId]
        });
    });
});

API Development

To keep it brief, I’ll try to simulate test driven development (TDD) approach with at most one non-passing test and its fix. When developing from scratch using TDD, the code obviously needs multiple tweaks before passing a test.

So, let’s add one by one.

1. Should return status 500 on server error (finding driver)

it('should return status 500 on server error (finding driver)', sinon.test(function () {
    this.stub(Driver, 'findOne').yields(error);
    Controller.assignDriver(req, res);
    sinon.assert.calledOnce(Driver.findOne);
    sinon.assert.calledWith(res.status, 500);
    sinon.assert.calledOnce(res.status(500).end);
}));
Controller.assignDriver = function (req, res) {
    return Driver.findOne({}, function (err, driver) {
    {
        if (err) {
            return res.status(500).end();
        }        
    })
};
 
  Vehicle Controller
    assignDriver
      ✓ should return status 500 on server error (finding driver)


  1 passing (15ms)
 

2. Should return 404 with a message for non-existing driver

it('should return 404 with a message for non-existing driver', sinon.test(function () {
    this.stub(Driver, 'findOne').yields(null, null);
    Controller.assignDriver(req, res);
    sinon.assert.calledWith(Driver.findOne, { _id: req.params.driverId });
    sinon.assert.calledWith(res.status, 404);
    sinon.assert.calledWith(res.status(404).json, { message: "driver not found" });
}));
Controller.assignDriver = function (req, res) {
    return Driver.findOne({}, function (err, driver) {
        if (err) {
            return res.status(500).end();
        }  
        else if (!driver) {
            return res.status(404).json({message: "driver not found"});
        }      
    })
};
 
  Vehicle Controller
    assignDriver
      ✓ should return status 500 on server error (finding driver)
      1) should return 404 with a message for non-existing driver


  1 passing (29ms)
  1 failing

  1) Vehicle Controller assignDriver should return 404 for non-existing driver:
     expected findOne to be called with arguments { _id: "5aa13452e1e2c3277688e734" }, { new: true }
 

The test failed because of our handy assertion that Driver.findOne should be called with two arguments { _id: req.params.driverId }, { new: true }. Adding that passes our test.

Controller.assignDriver = function (req, res) {
    return Driver.findOne({ _id: req.params.driverId }, { new: true }, function (err, driver) {
        if (err) {
            return res.status(500).end();
        }  
        else if (!driver) {
            return res.status(404).json({message: "driver not found"});
        }      
    })
};
 
  Vehicle Controller
    assignDriver
      ✓ should return status 500 on server error (finding driver)
      ✓ should return 404 with a message for non-existing driver


  2 passing (20ms)
 

3. Should return 403 with a message for unavailable driver

    it('should return 403 with a message for unavailable driver', sinon.test(function () {
        expectedResult.available = false;
        this.stub(Driver, 'findOne').yields(null, expectedResult);
        Controller.assignDriver(req, res);
        sinon.assert.calledWith(Driver.findOne, { _id: req.params.driverId });
        sinon.assert.calledWith(res.status, 403);
        sinon.assert.calledWith(res.status(403).json, { message: "driver unavailable" });
    }));
Controller.assignDriver = function (req, res) {
    return Driver.findOne({ _id: req.params.driverId }, { new: true }, function (err, driver) {
        if (err) {
            return res.status(500).end();
        }  
        else if (!driver) {
            return res.status(404).json({message: "driver not found"});
        }   
        else if (!driver.available){
            return res.status(403).end();
        }   
    })
};
 
  Vehicle Controller
    assignDriver
      ✓ should return status 500 on server error (finding driver)
      ✓ should return 404 with a message for non-existing driver
      1) should return 403 with a message for unavailable driver


  2 passing (30ms)
  1 failing

  1) Vehicle Controller assignDriver should return 403 with a message for unavailable driver:
     AssertError: expected spy to be called with arguments { message: "driver unavailable" }

 

Oops, we forgot to return a message with 403. Add return res.status(403).json({message: "driver unavailable"}); to pass.

 
  Vehicle Controller
    assignDriver
      ✓ should return status 500 on server error (finding driver)
      ✓ should return 404 with a message for non-existing driver
      ✓ should return 403 with a message for unavailable driver


  3 passing (27ms)
  

4. Should return status 500 on server error (finding vehicle)

    it('should return status 500 on server error (finding vehicle)', sinon.test(function () {
        this.stub(Driver, 'findOne').yields(null, expectedDriver);
        this.stub(Vehicle, 'findById').yields(error);
        Controller.assignDriver(req, res);
        sinon.assert.calledOnce(Vehicle.findById);
        sinon.assert.calledWith(res.status, 500);
        sinon.assert.calledOnce(res.status(500).end);
    }));
Controller.assignDriver = function (req, res) {
    return Driver.findOne({ _id: req.params.driverId }, { new: true }, function (err, driver) {
        if (err) {
            return res.status(500).end();
        }  
        else if (!driver) {
            return res.status(404).json({message: "driver not found"});
        }   
        else if (!driver.available){
            return res.status(403).json({message: "driver unavailable"});
        } 
        else {
            return Vehicle.findById(req.params.id, function (err, vehicle) {
                if (err) {
                    return res.status(500).end();
                }
            })
        }  
    })
};
 
  Vehicle Controller
    assignDriver
      ✓ should return status 500 on server error (finding driver)
      ✓ should return 404 with a message for non-existing driver
      ✓ should return 403 with a message for unavailable driver
      ✓ should return status 500 on server error (finding vehicle)


  4 passing (64ms)

5. Should return 404 with a message for non-existing vehicle

    it('should return 404 with a message for non-existing vehicle', sinon.test(function () {
        this.stub(Driver, 'findOne').yields(null, expectedDriver);
        this.stub(Vehicle, 'findById').yields(null, null);
        Controller.assignDriver(req, res);
        sinon.assert.calledWith(Driver.findOne, { _id: req.params.driverId });
        sinon.assert.calledWith(Vehicle.findById, req.params.id);
        sinon.assert.calledWith(res.status, 404);
        sinon.assert.calledWith(res.status(404).json, { message: "vehicle not found" });
    })); 
Controller.assignDriver = function (req, res) {
    return Driver.findOne({ _id: req.params.driverId }, function (err, driver) {
        if (err) {
            return res.status(500).end();
        }
        else if (!driver) {
            return res.status(404).json({ message: "driver not found" });
        }
        else if (!driver.available) {
            return res.status(403).json({ message: "driver unavailable" });
        }
        else {
            return Vehicle.findById(req.params.id, function (err, vehicle) {
                if (err) {
                    return res.status(500).end();
                }
                else if (!vehicle) {
                    return res.status(404).json({ message: "vehicle not found" });
                }
            })
        }  
    })
};
 
  Vehicle Controller
    assignDriver
      ✓ should return status 500 on server error (finding driver)
      ✓ should return 404 with a message for non-existing driver
      ✓ should return 403 with a message for unavailable driver
      ✓ should return status 500 on server error (finding vehicle)
      ✓ should return 404 with a message for non-existing vehicle


  5 passing (32ms)
 

6. Should return 403 with a message if 3 drivers already assigned

  it('should return 403 with a message if 3 drivers already assigned', sinon.test(function () {
        expectedResult.drivers = [1, 2, 3]
        this.stub(Driver, 'findOne').yields(null, expectedDriver);
        this.stub(Vehicle, 'findById').yields(null, expectedResult);
        Controller.assignDriver(req, res);
        sinon.assert.calledWith(Driver.findOne, { _id: req.params.driverId });
        sinon.assert.calledWith(Vehicle.findById, req.params.id);
        sinon.assert.calledWith(res.status, 403);
        sinon.assert.calledWith(res.status(403).json, { message: "maximum drivers assigned, can't assign new" });
    }));
Controller.assignDriver = function (req, res) {
    return Driver.findOne({ _id: req.params.driverId }, function (err, driver) {
        if (err) {
            return res.status(500).end();
        }
        else if (!driver) {
            return res.status(404).json({ message: "driver not found" });
        }
        else if (!driver.available) {
            return res.status(403).json({ message: "driver unavailable" });
        }
        else {
            return Vehicle.findById(req.params.id, function (err, vehicle) {
                if (err) {
                    return res.status(500).end();
                }
                else if (!vehicle) {
                    return res.status(404).json({ message: "vehicle not found" });
                }
                else if (vehicle.drivers.length === 3) {
                    return res.status(403).json({ message: "maximum drivers assigned, can't assign new" });
                }
            })
        }  
    })
};
 
  Vehicle Controller
    assignDriver
      ✓ should return status 500 on server error (finding driver)
      ✓ should return 404 with a message for non-existing driver
      ✓ should return 403 with a message for unavailable driver
      ✓ should return status 500 on server error (finding vehicle)
      ✓ should return 404 with a message for non-existing vehicle
      ✓ should return 403 with a message if 3 drivers already assigned


  6 passing (36ms)
 

7. Should return status 500 on server error (assigning driver)

    it('should return status 500 on server error (assigning driver)', sinon.test(function () {
        this.stub(Driver, 'findOne').yields(null, expectedDriver);
        this.stub(Vehicle, 'findById').yields(null, expectedResult);
        this.stub(Vehicle, 'findByIdAndUpdate').yields(error);
        Controller.assignDriver(req, res);
        sinon.assert.calledWith(Vehicle.findById, req.params.id);
        sinon.assert.calledWith(res.status, 500);
        sinon.assert.calledOnce(res.status(500).end);
    }));
Controller.assignDriver = function (req, res) {
    return Driver.findOne({ _id: req.params.driverId }, function (err, driver) {
        if (err) {
            return res.status(500).end();
        }
        else if (!driver) {
            return res.status(404).json({ message: "driver not found" });
        }
        else if (!driver.available) {
            return res.status(403).json({ message: "driver unavailable" });
        }
        else {
            return Vehicle.findById(req.params.id, function (err, vehicle) {
                if (err) {
                    return res.status(500).end();
                }
                else if (!vehicle) {
                    return res.status(404).json({ message: "vehicle not found" });
                }
                else if (vehicle.drivers.length === 3) {
                    return res.status(403).json({ message: "maximum drivers assigned, can't assign new" });
                }
                else {
                    return Vehicle.findByIdAndUpdate(req.params.id, { $addToSet: { drivers: req.params.driverId } }, { new: true }, function (err, vehicle) {
                        if (err) {
                            return res.status(500).end();
                        }
                    })
                }
            })
        }
    })
};
 
  Vehicle Controller
    assignDriver
      ✓ should return status 500 on server error (finding driver)
      ✓ should return 404 with a message for non-existing driver
      ✓ should return 403 with a message for unavailable driver
      ✓ should return status 500 on server error (finding vehicle)
      ✓ should return 404 with a message for non-existing vehicle
      ✓ should return 403 with a message if 3 drivers already assigned
      ✓ should return status 500 on server error (assigning driver)



  7 passing (48ms)
 

8. Should return updated vehicle obj with drivers array

And finally the test for success scenario.

    it('should return updated vehicle obj with drivers array', sinon.test(function () {
        this.stub(Driver, 'findOne').yields(null, expectedDriver);
        this.stub(Vehicle, 'findByIdAndUpdate').yields(null, expectedResult);
        Controller.assignDriver(req, res);
        sinon.assert.calledWith(Driver.findOne, { _id: req.params.driverId });
        sinon.assert.calledWith(Vehicle.findByIdAndUpdate, req.params.id, { $addToSet: { drivers: req.params.driverId } }, { new: true });
        sinon.assert.calledWith(res.json, sinon.match({ drivers: expectedResult.drivers }));
    }));
Controller.assignDriver = function (req, res) {
    return Driver.findOne({ _id: req.params.driverId }, function (err, driver) {
        if (err) {
            return res.status(500).end();
        }
        else if (!driver) {
            return res.status(404).json({ message: "driver not found" });
        }
        else if (!driver.available) {
            return res.status(403).json({ message: "driver unavailable" });
        }
        else {
            return Vehicle.findById(req.params.id, function (err, vehicle) {
                if (err) {
                    return res.status(500).end();
                }
                else if (!vehicle) {
                    return res.status(404).json({ message: "vehicle not found" });
                }
                else if (vehicle.drivers.length === 3) {
                    return res.status(403).json({ message: "maximum drivers assigned, can't assign new" });
                }
                else {
                    return Vehicle.findByIdAndUpdate(req.params.id, req.body, { new: true }, function (err, vehicle) {
                        if (err) {
                            return res.status(500).end();
                        }
                        else {
                            return res.json(vehicle);
                        }
                    })
                }
            })
        }
    })
};

The test will fail because the second argument of findByIdAndUpdate doesn’t have $addToSet. Adding the correct object will pass the test: return Vehicle.findByIdAndUpdate(req.params.id, { $addToSet: { drivers: req.params.driverId } }, { new: true }, function (err, vehicle)

 
  Vehicle Controller
    assignDriver
      ✓ should return status 500 on server error (finding driver)
      ✓ should return 404 with a message for non-existing driver
      ✓ should return 403 with a message for unavailable driver
      ✓ should return status 500 on server error (finding vehicle)
      ✓ should return 404 with a message for non-existing vehicle
      ✓ should return 403 with a message if 3 drivers already assigned
      ✓ should return status 500 on server error (assigning driver)
      ✓ should return updated vehicle obj with drivers array



  8 passing (41ms)
 

Code Refactoring

We can see the above code ended up in callback hell. As we have tests in place we can confidently try to refactor it. Both async and promise can be used to streamline the code. Promise will even help us merging three tests for 500 server error by using catch block. But here I’m using async waterfall because using promise will require tweaks in tests (as yields is used in callbacks which wont’ work with promise).

Install and save npm install async --save

Require async at the top const async = require('async');

And replace the controller method:

const async = require('async');

.
.
.

Controller.assignDriver = function (req, res) {
    async.waterfall([
        function (callback) {
            Driver.findOne({ _id: req.params.driverId }, function (err, driver) {
                if (err) {
                    return callback(500);
                }
                else if (!driver) {
                    return callback({ status: 404, message: "driver not found" });
                }
                else if (!driver.available) {
                    return callback({ status: 403, message: "driver unavailable" });
                }
                else {
                    return callback(null);
                }
            })
        },
        function (callback) {
            Vehicle.findById(req.params.id, function (err, vehicle) {
                if (err) {
                    return callback(500);
                }
                else if (!vehicle) {
                    return callback({ status: 404, message: "vehicle not found" });
                }
                else if (vehicle.drivers.length === 3) {
                    return callback({ status: 403, message: "maximum drivers assigned, can't assign new" });
                }
                else {
                    return callback(null);
                }
            })
        },
        function (callback) {
            Vehicle.findByIdAndUpdate(req.params.id, { $addToSet: { drivers: req.params.driverId } }, { new: true }, function (err, vehicle) {
                if (err) {
                    return callback(500);
                }
                else {
                    return callback(null,vehicle)
                }
            })
        }

    ], function (err, updatedVehicle) {
        if (err) {
            if (err.message) {
                return res.status(err.status).json({ message: err.message });
            }
            else{
                return res.status(500).end();
            }
        }
        else {
            return res.json(updatedVehicle);
        }
    });
};

The tests still pass!

 
    assignDriver
      ✓ should return status 500 on server error (finding driver)
      ✓ should return 404 with a message for non-existing driver
      ✓ should return 403 with a message for unavailable driver
      ✓ should return status 500 on server error (finding vehicle)
      ✓ should return 404 with a message for non-existing vehicle
      ✓ should return 403 with a message if 3 drivers already assigned
      ✓ should return status 500 on server error (assigning driver)
      ✓ should return updated vehicle obj with drivers array


  8 passing (30ms)