testdouble.js icon indicating copy to clipboard operation
testdouble.js copied to clipboard

Allow for a default return value when creating a test double

Open searls opened this issue 7 years ago • 2 comments

Problem

In promise-y code, a return value of undefined (testdouble's default return value) is nonsensical and will break things. As a result, people faking out promise APIs, will very often create a test double, set up a default stubbing (i.e. td.when(thing(), {ignoreExtraArgs: true}).thenResolve(null))) and then—if they use td.verify later—be hit with the "don't stub what you verify" warning.

This warning is valid in other cases, but here it's a symptom of a different root cause: that test double should allow for default return values for testdouble functions without considering those user-land stubbings.

Workaround

Until this is resolved, I recommend affected users set the td.config({ignoreWarnings: true}) option.

Proposal

I don't have a really clean API in mind for how to accomplish this (given that it shouldn't be a global config and given how many ways exist to create test doubles), but it's clear that there is a need for setting a default return value, particularly to functions that are expected to return promises.

One idea that might work is to overload td.config to optionally receive a test double function, then configurations scoped just to that function:

const func = td.func()
td.config(func, {defaultReturn: Promise.resolve(null)})

And then people could build their own helpers that would create the fakes they want in a test helper, in whatever means they typically use to make fakes:

// in a test-helper.js
global.promiseFake = (name) => {
  const func = td.func(name)
  td.config(func, {defaultReturn: Promise.resolve(null)})
}

// in a test
let thing
beforeEach () {
  thing = promiseFake('lol')
}

And so on.

Next steps

Thoughts on the above API as an initial attempt to resolve this? Alternate ideas?

For prior art in people being frustrated by this, see: #250, #245, #199, #148, #389

searls avatar Nov 05 '18 16:11 searls

It's not immediately obvious to me how this interface would play nicely with td.replace, which is how I've largely been using testdouble. Specifically, I've been using it a lot when I'm test-driving something that needs to speak to my database, and I've wrapped the DB access method in a DAO module. I get this problem a lot when I'm creating or updating a record; I add a method exports.create = function(params){/* The real thing will return a promise */}, in the module and in the test:

fooDao = td.replace('../src/foo_dao')
...
td.verify(fooDao.create(correctParams))

For this use case, I'd ideally like something like td.replace('../src/foo_dao', { async: all}), or td.replace('../src/foo_dao', { async: [methodOne, methodTwo ... ]}). However, I'm aware both that this doesn't play nicely with the existing API for a custom replacement, and that you have a preference for a default return over use of an 'async' keyword, so I guess my main point here is just to throw this additional use case into the mix.

robwold avatar Feb 03 '19 13:02 robwold

It's not immediately obvious to me how this interface would play nicely with td.replace

I believe the answer to this is probably that td.replace calls td.imitate, which could be adjusted to treat functions of type AsyncFunction differently such that they're configured to default to return a resolved null promise. This would, of course, only apply to modules for which the implementation is literally using the async keyword.

searls avatar Feb 03 '19 14:02 searls

Stale. Closing. Please reopen if still relevant and I will look into it.

giltayar avatar Sep 16 '23 11:09 giltayar