testdouble / testdouble.js

A minimal test double library for TDD with JavaScript
MIT License
1.42k stars 142 forks source link

mocking non-direct dependency #379

Closed jsardev closed 6 years ago

jsardev commented 6 years ago

Description

I would like to use testdouble for integration tests to mock some dependencies and make tests more predictable and easier to write/read. In this particular example, I'd like to mock a deep, non-direct dependency of a module. Let me explain, you have something like this:

subject:

const dep1 = require('./dep1');
module.exports = () => dep1();

dep1:

const dep2 = require('./dep2');
module.exports = () => dep2();

dep2:

module.exports = () => {};

Let's say that the subject is a express route handler (subject) which saves data to the database (dep1), which generates an unique UUID for the entry before saving (dep2). I'd like to mock the returning UUID's to easily check the new entries in the database.

For this I'd create an integration test, but to introduce this example as simply as possible let's stay with the easy things above. So let's have this test:

const t = require('tap');
const td = require('testdouble');

let dep2, subject;
t.beforeEach(done => {
  dep2 = td.replace('./dep2');
  subject = require('./subject');
  done();
});

t.afterEach(done => {
  td.reset();
  done();
})

t.test('1', async t => {
  td.when(dep2()).thenReturn(1);

  const result = subject();

  t.equal(result, 1);
});

t.test('2', async t => {
  td.when(dep2()).thenReturn(2);

  const result = subject();

  t.equal(result, 2);
});

Issue

So the first test passes, but the second one does not. dep2 returns nothing, and td.explain() shows that there are 0 stubbings.

What am I doing wrong? 😄

Environment

searls commented 6 years ago

Warning: this comment is a giant wall of text of thoughts and opinions raised by this issue. I will add a second comment about the specific technical reason this is happening

Replacing indirect/transitive dependencies is not supported by testdouble.js. After reading the example above, I'd recommend either calling through to an actual instance UUID service and loosening your assertions to accommodate that or using a different tool to fake UUIDs in a more extrinsic way than mocking out the module with testdouble.

As a result, if testdouble.js just so happened to work for this, I wouldn't do anything to stop it from working (similar to the fact that td's ability to mock 3rd party libs ended up being almost incidental). But I don't think the library should go out of its way to add this as a feature and expend any energy to continue to support it, either, because the potential for harm outweighs the potential utility, IMO.


More broadly—not considering your case specifically—I generally recommend against faking out indirect/second-order/transitive dependencies.

Many, many people use mocking libraries to fake out distant dependencies, and the two most common cases in which they do this is:

  1. To fake a function that returns some kind of data-providing module that fetches remote information in a convenient way with a data fixture.
  2. To fix a test failure or hard-to-setup test by punching a hole in reality

Case #1 is almost always a long-term poor choice, IME, because the data representation being faked is not extrinsically meaningful, can change for any reason (meaning the test can break and all it means is you now have to update the test to match), and tools like nock and supertest do a better job of specifying the boundaries between your code and remote systems (see testdouble-nock). For anyone reading this, you can check 24m03s of this video for more: https://vimeo.com/257056050

Case #2 is also usually a bad idea, because the veracity of the thing being tested is usually being compromised and so confidence that the test is behaving in a realistic way is very low (in part, because the fact the hole is necessary means that the code won't work without it, so there exists a poorly-specified gap between what's running in the test and what needs to happen in production).

searls commented 6 years ago

The reason this is happening is because td.js's dependency quibble will only bypass the module cache if require is being called from a file that has been "quibbled", per this branch here

The reason quibble does this is two-fold:

Because changing quibble's behavior to always bypass the cache in order to support this use case would incidentally result in many existing codebases having tests fail when a (more likely than not incidentally important) module is loaded multiple times.

As a result, I'm going to close this issue as "won't fix", even acknowledging that the current state (of it just happening to seem to work in the first case but then failing quietly for subsequent uses ) is very much not good. One PR to quibble I would 100% support, however, is a warning or error to preemptively tell people what's happening and to not attempt this.

jsardev commented 6 years ago

@searls First of all, thank you very much for this expanded answer! Second, I'm sorry if my questions are a little bit chaotic, I've watched your whole talk and were inspired to change the ways I'm doing things, but I still struggle to put all of it into practice. If you could give me a minute more to explain me one thing I'd very appreciate!

So I think I get all the reasons you've written about, but in practice, I still don't know what to do and feel kinda stuck.

Consider that my external system (the very end of the dependency call stack of my test subject - not anywhere in the middle - the correct way, if I understand it properly, according to your talk) is just an external library which talks to the filesystem and has this kind of simple interface:

doSomething(file, function(error) {})

What I wanted to do in my integration test is to check if my system returns a 500 and proper message when the library is failing (by mocking the callback function). How do I achieve that not having to mock the entire external module? What would you do in this situation?

searls commented 6 years ago

Hey @sarneeh I am sorry for not responding to this sooner.

The case you're describing can be described as "test when the 3rd party library fails", but what you're actually looking to verify (it sounds like) is "test that my application does the right thing if the 3rd party library fails"

And in that case, what I would do is write an integration test that interacts with your app over (apparently) HTTP, triggers the failure state in the third party library by using a network faker like nock, and then validating your own 500 message.

jsardev commented 6 years ago

@searls No problem 😄 You're right. It's more like "test that my app does the right thing on 3rd party library fail". But what you're suggesting on the integration test is mocking an external http call - this is easy with nock. What if the thing that has to fail is fs.writeFile? I'd have to mock fs somewhere deep in the app.

searls commented 6 years ago

In that case, I won't send the Mocking Police after you if you were to use a mocking library to fake out the third party thing, but I'd still be reticent because it'd leave you exposed to the possibility of that fake inaccurately mimicking a failure state of the lib such that the test is creating fake work for you. Or (more likely), as the lib changes the fake you write today does mimic the real failure state successfully but future versions of the lib will fail differently.

In either case I'd write a thin wrapper module around it, call through that, and fake that, but to each their own.

jsardev commented 6 years ago

Oh no, the Mocking Police! 😆 Thanks for your insight. One more question: as testdouble is rather suited for unit testing and direct dependency mocking - do you have any favorite, battle-tested libraries for use in integration testing?

searls commented 6 years ago

No. I'd probably use proxyquire or sandboxed-module and just roll my own fake

searls commented 6 years ago

scratch that, I'd probably use td.replace('pathtothing', {my: {own: {fake}}})