testdouble / testdouble.js

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

Feature request: reset specific module(s) #520

Open JakobJingleheimer opened 1 year ago

JakobJingleheimer commented 1 year ago

It would be great if td.reset() optionally accepted a list of modules to reset, like:

td.reset(); // nuclear option → reset everything

td.reset('module-a.js');
td.reset(
  'module-a.js',
  'module-b.js',
);

The problem this solves is avoiding erroneously blowing away global mocks: a particular test may need to mock a specific module; after that test runs, only the module(s) it itself mocked should be reset.

JakobJingleheimer commented 1 year ago

Gentle nudge

If you're low on capacity but the feature is acceptable, I can try to implement it.

JakobJingleheimer commented 8 months ago

@searls would you have an opinion on this?

searls commented 8 months ago

I don't really understand a "global mock" in this context, as I've never made a mock that survived a single test.

If for some reason you want that, I think I'd no longer call that a "test double" and instead a fake / alternate implementation for the sake of building a test harness. And if that's the case, I'd suggest dropping down to quibble to accomplish this.

of course, as soon as I think of this, quibble doesn't support this either because it just has a single process-wide reset() function. But if one could set a context for quibbles of other modules, and took that context as an optional arguemnt in reset(), then maybe something like this could be accomplished.

JakobJingleheimer commented 8 months ago

A global mock would be something that is very common in an app but that has an adverse impact on testing. For example, I have an app that interacts with IndexedDB (a browser API); aside from the spec of that util, I never want that util to be real in the rest of my test suite, so I do something like:

$ node --import ./test/setup.mjs --test ./app/**/*.test.*
// test/setup

await td.replaceEsm('…/app/storage.mjs', {
  createTransaction: mock_createTransaction,
  storage: mock_storage,
});

Then in foo.test.mjs, let's say it replaces bar.mjs. At the end of foo.test.mjs, bar.mjs should be restored, but storage.mjs should not.

I think the most flexible solution would be for td.reset() to optionally accept arguments for which modules to reset to the originals.

searls commented 8 months ago

Yeah, I hear you. I personally would rather folks used quibble directly for that, because the semantics of td.js are so organized around per-test setup and teardown (including its reset() implementation) that I think it'd be better if this lived in quibble.

That said, whether it's via named contexts like I mentioned or a list of modules to not reset (because accidentally letting a test-scoped mock survive across tests because it was missed would be bad), i'd be :+1: for a well-tested quibble PR that did this

JakobJingleheimer commented 8 months ago

Hmmm, I think the ergonomics of the inverse would be rather tedious (both unexpected and not particularly what users want, as well as a bit more difficult to implement).

because accidentally letting a test-scoped mock survive across tests because it was missed would be bad

Yes 🙂 That's why I think having the default (no arguments provided) is right: unless the user chooses to do something quite specific, your concern is avoided.

And this way it's not a breaking change (td.reset() current blows everything away).

I don't know this lib well enough to know the difference between putting this in quibble vs testdouble. Quibble exists as a dependency by proxy of testdouble, so it's not extra burden on users aside from being an extra thing to know (and I think most will not expect and thus won't know where to look).