3

I have an Express app that uses node-slack-sdk to make posts to Slack when certain endpoints are hit. I am trying to write integration tests for a route that, among many other things, calls a method from that library.

I would like to prevent all default behavior of certain methods from the Slack library, and simply assert that the methods were called with certain arguments.

I have attempted to simplify the problem. How can I stub a method (which is actually nested within chat) of an instance of an WebClient, prevent the original functionality, and make assertions about what arguments it was called with?

I've tried a lot of things that haven't worked, so I'm editing this and providing a vastly simplified set-up here:

index.html:

const express = require('express');
const {WebClient} = require('@slack/client');
const app = express();
const web = new WebClient('token');

app.post('/', (req, res) => {

    web.chat.postMessage({
        text: 'Hello world!',
        token: '123'
    })
        .then(() => {
            res.json({});
        })
        .catch(err => {
            res.sendStatus(500);
        });
});

module.exports = app;

index.test.html

'use strict';
const app = require('../index');
const chai = require('chai');
const chaiHttp = require('chai-http');
const sinon = require('sinon');

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

const {WebClient} = require('@slack/client');


describe('POST /', function() {
    before(function() {
        // replace WebClient with a simplified implementation, or replace the whole module.
    });

    it('should call chat.update with specific arguments', function() {
        return chai.request(app).post('/').send({})
            .then(function(res) {
                expect(res).to.have.status(200);
                // assert that web.chat.postMessage was called with {message: 'Hello world!'}, etc
        });
    });
});

There are a few things that make this difficult and unlike other examples. One, we don't have access to the web instance in the tests, so we can't stub the methods directly. Two, the method is buried within the chat property, web.chat.postMessage, which is also unlike other examples I've seen in sinon, proxyquire, etc documentation.

bookcasey
  • 39,223
  • 13
  • 77
  • 94
  • If you want a mock object that has none of the actual functionality, then just create your own object and define the same methods and then you can assert on the arguments passed to it. Don't even use the actual object. – jfriend00 Apr 20 '18 at 23:38
  • That's not possible because the dependency is used deeply within an Express route. How can I replace the dependency with my object without modifying the code I am trying to test? – bookcasey Apr 21 '18 at 02:38
  • Replace the module with a mock implementation. – jfriend00 Apr 21 '18 at 02:39
  • You can use nock. You can save a real response. Then you can configure nock to response when you run the tests. https://github.com/node-nock/nock – Carlos Apr 30 '18 at 12:23
  • @bookcasey, any feedback on this? the bounty is about to expire and working answer has already been posted – Tarun Lalwani May 01 '18 at 15:09

3 Answers3

3

The design of your example is not very testable which is why you're having these issues. In order to make it more testable and cohesive, it's better to pass in your WebClient object and other dependencies, rather than create them in your route.

const express = require('express');
const {WebClient} = require('@slack/client');
const app = express();//you should be passing this in as well. But for the sake of this example i'll leave it


module.exports = function(webClient) {
   app.post('/', (req, res) => {

       web.chat.postMessage({
          text: 'Hello world!',
          token: '123'
       })
           .then(() => {
              res.json({});
           })
           .catch(err => {
              res.sendStatus(500);
           });
   })
   return app;
};

In order to implement this, build your objects/routes at a higher module. (You might have to edit what express generated for you. I'm not sure, personally I work with a heavily refactored version of express to fit my needs.) By passing in your WebClient you can now create a stub for your test.

'use strict';

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

const expect = chai.expect;
chai.use(chaiHttp);
const {WebClient} = require('@slack/client');
const web = new WebClient('token');
let app = require('../index')(web);

describe('POST /', function() {

    it('should call chat.update with specific arguments', function() {
        const spy = sinon.spy();
        sinon.stub(web.chat, 'postMessage').callsFake(spy);

        return chai.request(app).post('/').send({})
            .then(function(res) {
                expect(res).to.have.status(200);
                assert(spy.calledWith({message: 'Hello world!'}));
        });
    });
});

This is known as Dependency Injection. Instead of having your index module build it's dependency, WebClient, your higher modules will pass in the dependency in order for the them to control the state of it's lower modules. Your higher module, your test, now has the control it needs to create a stub for the lower module, index.

The code above was just quick work. I haven't tested to see if it works, but it should answer your question.

Plee
  • 581
  • 7
  • 15
  • Thank you for your response. It really feels like this should be possible, and a lot cleaner with a tool like `proxyquire` or `rewiremock`. That's what those libraries are for, no? – bookcasey Apr 24 '18 at 17:55
  • Yes, you can certainly use those tools, but I wouldn't say it's "cleaner". By passing in your dependency, you not only make your code testable, but it helps achieve higher cohesion. You can find more on the topic here: https://stackoverflow.com/questions/14301389/why-does-one-use-dependency-injection?utm_medium=organic&utm_source=google_rich_qa&utm_campaign=google_rich_qa – Plee Apr 24 '18 at 18:39
  • Those tools were primarily made as just another type of mocking library such as sinon or mockery. They just include the functionality to stub private functions and dependencies. But just because they include tools to clean up a mess, wouldn't it be better to just keep your code clean in the first place? – Plee Apr 24 '18 at 18:49
  • 1
    Thank you, I'm looking to use those sorts of tools (proxyquire) for the moment, so I don't have to drastically modify a pretty large project to write integration tests for something that works. I'm looking for an answer that allows me to keep the code the same, it might not the ideal solution, but that is what I'm looking for with this question. Thank you! +1 – bookcasey Apr 24 '18 at 21:50
  • If that's the case then using those tools will be fine. Just be sure it's what u want. Being afraid of changing the source code is a red flag that the code is designed poorly. If that's the case sometimes it's best to step back and rewrite your system for future updates and longevity of your application. But that's up to you. Anyways, glad i can help! – Plee Apr 25 '18 at 07:21
1

So @Plee, has some good points in term of structuring. But my answer is more about the issue at hand, how to make the test work and things you need to understand. For getting better at writing unit tests you should use other good resources like books and articles, I assume there would be plenty of great resources online for the same

The first thing you do wrong in your tests is the first line itself

const app = require('../index');

Doing this, you load the index file which then executes the below code

const {WebClient} = require('@slack/client');
const app = express();
const web = new WebClient('token');

So now the module has loaded the original @slack/client and created an object which is not accessible outside the module. So we have lost our chance of customizing/spying/stubbing the module.

So the first thumb rule

Never load such modules globally in the test. Or otherwise never load them before stubbing

So next we want is that in our test, we should load the origin client library which we want to stub

'use strict';
const {WebClient} = require('@slack/client');
const sinon = require('sinon');

Now since we have no way of getting the created object in index.js, we need to capture the object when it gets created. This can be done like below

var current_client = null;

class MyWebClient extends WebClient {
    constructor(token, options) {
        super(token, options);
        current_client = this;
    }
}

require('@slack/client').WebClient = MyWebClient;

So now what we do is that original WebClient is replaced by our MyWebClient and when anyone creates an object of the same, we just capture that in current_client. This assumes that only one object will be created from the modules we load.

Next is to update our before method to stub the web.chat.postMessage method. So we update our before method like below

before(function() {
    current_client = null;
    app = require('../index');
    var stub = sinon.stub();
    stub.resolves({});
    current_client.chat.postMessage = stub;
});

And now comes the testing function, which we update like below

it('should call chat.update with specific arguments', function() {
    return chai.request(app).post('/').send({})
        .then(function(res) {
            expect(res).to.have.status(200);
            expect(current_client.chat.postMessage
                .getCall(0).args[0]).to.deep.equal({
                text: 'Hello world!',
                token: '123'
            });
        });
});

and the results are positive

Results

Below is the complete index.test.js I used, your index.js was unchanged

'use strict';
const {WebClient} = require('@slack/client');
const sinon = require('sinon');

var current_client = null;

class MyWebClient extends WebClient {
    constructor(token, options) {
        super(token, options);
        current_client = this;
    }
}

require('@slack/client').WebClient = MyWebClient;

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

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


let app = null;
describe('POST /', function() {
    before(function() {
        current_client = null;
        app = require('../index');
        var stub = sinon.stub();
        stub.resolves({});
        current_client.chat.postMessage = stub;
    });

    it('should call chat.update with specific arguments', function() {
        return chai.request(app).post('/').send({})
            .then(function(res) {
                expect(res).to.have.status(200);
                expect(current_client.chat.postMessage
                    .getCall(0).args[0]).to.deep.equal({
                    text: 'Hello world!',
                    token: '123'
                });
            });
    });
});
Tarun Lalwani
  • 142,312
  • 9
  • 204
  • 265
1

Based on the other comments, it seems like you are in a codebase where making a drastic refactor would be difficult. So here is how I would test without making any changes to your index.js.

I'm using the rewire library here to get and stub out the web variable from the index file.

'use strict';

const rewire = require('rewire');
const app = rewire('../index');

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

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

const web = app.__get__('web');

describe('POST /', function() {
    beforeEach(function() {
        this.sandbox = sinon.sandbox.create();
        this.sandbox.stub(web.chat);
    });

    afterEach(function() {
        this.sandbox.restore();
    });

    it('should call chat.update with specific arguments', function() {
        return chai.request(app).post('/').send({})
            .then(function(res) {
                expect(res).to.have.status(200);
                const called = web.chat.postMessage.calledWith({message: 'Hello world!'});
                expect(called).to.be.true;
        });
    });
});
Sanketh Katta
  • 5,961
  • 2
  • 29
  • 30