thlorenz / proxyquire

🔮 Proxies nodejs require in order to allow overriding dependencies during testing.
MIT License
2.75k stars 100 forks source link

Mocking Dependency Doesn't Work With Classes #247

Closed JimLynchCodes closed 5 years ago

JimLynchCodes commented 5 years ago

I would expect the code below to call the mock "doThings" function and print out "goodbye", but it doesn't. It doesn't mock anything and still prints "hello"...

// ClassDepService.js

class ClassDepService {

  doThings() {

    console.log('hello')

  }

}
module.exports = ClassDepService;

// ClassService.js

const ClassDepService = require('./ClassDepService')

module.exports = class AppService {

  callDep () {

    console.log('in class app service')

    const classDepService = new ClassDepService()
    classDepService.doThings()

  }

}

// users.js

var ClassService = require('./ClassService');

var stubs = {
  './ClassDepService': {
    doThings: function(val) {
      console.info('goodbye');
    },
    '@global': true,
    '@noCallThru': true
  }
};

var proxyquire = require('proxyquire').noCallThru();

var x = proxyquire('./ClassDepService', stubs)

classService = new ClassService();
classService.callDep()
JimLynchCodes commented 5 years ago

update: still doesn't work even with "@noCallThru"

bendrucker commented 5 years ago

Hi, please don't leave comments on old issues like you did. It notifies lots of people. Opening a new issue about your problem is fine if you feel it's unsolved.

The first thing I notice here is that you're exporting a constructor from module ./ClassDepService and then trying to override it with an object. If proxyquire had actually replaced your dependency, your code would start throwing:

Ctor = { doThings: () => {} }
new Ctor()
// => TypeError: Ctor is not a constructor

Second, you're requiring ./ClassService first, which loads ./ClassDepService, and then trying to globally override it later. That's not how proxyquire is meant to work. More like this:

class MockClassDepService {}
MockClassDepService['@noCallThru']

const ClassService = proxyquire('./ClassService', {
  './ClassDepService': MockClassDepService
})

// now this copy of ClassService will use MockClassDepService

Closing because AFAICT there is no proxyquire issue here, but happy to try to help further or take PRs improving the documentation.

JimLynchCodes commented 5 years ago

Hey,

Thanks a lot for getting back to me quickly and sorry about the multiple posts!

When I used your code exactly it failed silently and just used the original ClassDepService and not the mock. However, when I changed the string to "./ClassDepService" then it worked! Is it supposed to work with Class names as well or only file paths?

Also, super nitpick thing but if it does have to be filepaths then it would cool if it gave you autocompletion in code editors after you hit ./ inside of the string. Not sure if that's possible in any random npm package, but it would be cool.

bendrucker commented 5 years ago

Is it supposed to work with Class names as well or only file paths?

Yes, that's my mistake. Should be a file path relative to the subject-under-test (./ClassService), edited.

Also, super nitpick thing but if it does have to be filepaths then it would cool if it gave you autocompletion in code editors after you hit ./ inside of the string. Not sure if that's possible in any random npm package, but it would be cool.

An editor could totally handle this, just a matter of telling it about proxyquire. If your editor does it for require, it could be configured to do it for proxyquire. We can't make that happen by default but it should be possible to adapt an editor plugin to do it.

JimLynchCodes commented 5 years ago

Hi @bendrucker, thanks for helping out, but sadly your code does not work...

Following your advice, my test file now looks like this:

const chai = require("chai")
const expect = require('chai').expect
const sinonChai = require('sinon-chai')
const sinon = require("sinon")
chai.use(sinonChai);

var proxyquire = require('proxyquire')

let consoleLogSpy
const overriddenOutput = 'fake things, yay!'

describe("users route", () => {

    before(() => {
        consoleLogSpy = sinon.spy(console, "log");
    });

    after(() => {
        console.log.restore()
    });

    it("should call a function and use a mock of a dependency's dependecy", () => {

        class MockClassDepService {

            doThings() {
                console.log(overriddenOutput)
            }

        }
        MockClassDepService['@noCallThru']

        const Main = require('./main')

        const ClassService = proxyquire('./../services/ClassService', {
            './ClassDepService': MockClassDepService
        })

        const main = new Main()

        expect(consoleLogSpy).to.have.been.calledWith(overriddenOutput);

    });
});

However, this test always fails because it uses the real ClassDepService and not the mock...

You can run it yourself here (proxyquire-example branch): https://github.com/JimTheMan/NodeJS-Mocking-Deps-of-Deps-Example/blob/proxyquire-example/route-controllers/main.test.js

using node v10.16.0

rwalle61 commented 4 years ago

@JimLynchCodes thanks for raising this issue, I also didn't know how to use proxyquire this way for classes. @bendrucker thanks for helping out

Changing MockClassDepService['@noCallThru'] to MockClassDepService['@noCallThru'] = true; worked for me