Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mocking non-direct dependency #379

Closed
jsardev opened this issue Aug 22, 2018 · 9 comments
Closed

mocking non-direct dependency #379

jsardev opened this issue Aug 22, 2018 · 9 comments

Comments

@jsardev
Copy link

jsardev commented Aug 22, 2018

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

  • node: 9.11.2
  • npm: 5.6.0
  • testdouble: 3.8.1
@searls
Copy link
Member

searls commented Aug 24, 2018

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
Copy link
Member

searls commented Aug 24, 2018

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:

  • requiring modules multiple times is slow, so continuing to use the cache when a test loads things that are not quibbled, using the cache is much faster
  • requiring node.js modules should but certainly are not always idempotent, side-effect free operations. As a result, bypassing the cache and allowing multiple require invocations to load the same module multiple times will very often have nasty, unrealistic and unforeseen consequences that are nearly impossible to predict or debug unless yo know to be looking for them.

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.

@searls searls closed this as completed Aug 24, 2018
@jsardev
Copy link
Author

jsardev commented Aug 25, 2018

@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
Copy link
Member

searls commented Sep 21, 2018

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
Copy link
Author

jsardev commented Sep 21, 2018

@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
Copy link
Member

searls commented Sep 21, 2018

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
Copy link
Author

jsardev commented Sep 21, 2018

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
Copy link
Member

searls commented Sep 21, 2018

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

@searls
Copy link
Member

searls commented Sep 21, 2018

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants