Verifying stubs that return promises without warnings?

[christav]

If I'm stubbing out a function that returns a promise, I need to set it up with a td.when. Then I usually do a td.verify to check that it's getting passed the right stuff.

When I call td.verify I get the warning print about "both stubbed and verified".

In this case I don't actually care about the stub or the return value, I just need it to return a promise (of null) instead of just null.

Would it be possible to define a helper or a flag or something so that when you do a td.func you can tell it it's an async function and return a promise for null by default instead of just null? Then we can td.verify without warnings, just in an async context.

[sgtoj]

You can ignore warnings. The setting will persist throughout the whole test.

const td = require("testdouble");
td.config({ ignoreWarnings: true });

td.reset() // only resets state, not configurations
// `ignoreWarnings` is still set to true

[searls]

Would it be possible to define a helper or a flag or something so that when you do a td.func you can tell it it's an async function and return a promise for null by default instead of just null? Then we can td.verify without warnings, just in an async context.

This comes up often enough that we should probably figure something out. I continue to not relate well to this need because I don't have it myself, but I can definitely buy that there should be a way to alter the test double such that it returns a null-resolved Promise as opposed to a null return value.

[searls]

The tricky thing will be to figure out how to do this without muddying up the API. There currently isn't a way to configure a td function.

Perhaps the best thing to do would be—as opposed to changing the public API—to instead internally identify a td.when(foo(), {ignoreExtraArgs: true}).thenResolve(null) and have that particular case bypass this warning. That way you could create a little helper just for this case and not worry about the warning

[christav]

An idea I just had - could we have either a flag on the td.func() constructor (something like td.func({async: true}) or a separate constructor (td.asyncFunc() maybe) to create the stub to return a promise?

Or is that muddying up the api?

[searls]

I don't think the setting you're looking for is async: true (since lots of async funcs return nothing, and not necessarily promises). I think what we want is something like this:

td.func('some name', {defaultReturn: Promise.resolve('lol')})

[christav]

That reads fine to me, I'm not wedded to async by any means. It looks like having an option on the constructor could work well. Not sure how well that fits with td.object though.

[gurpreetatwal]

I think @christav's approach is better here. I feel like the defaultReturn option breaks with testdouble's philosophy (correct me if wrong though), as it allows you to specify the actual value to return without having to do a demonstration. It could be abused for non-promise cases by doing:

td.func('some name', {defaultReturn: 'lol')})

It'd better to just have the {async: true} option or have async equivalents like asyncFunc, asyncReplace etc.

Having the async equivalents might be easier to do as you don't have to modify the api

[searls]

Well, the reason tdjs currently doesn't offer this at all is because I agree with you, but enough people have stumbled over promise-happy APIs that I think addressing it makes sense.

As for:

the defaultReturn option breaks with testdouble's philosophy (correct me if wrong though), as it allows you to specify the actual value to return without having to do a demonstration

So would an async: true whose implementation was effectively to set a default return to an empty resolved promise, but in a way that would be more confusing since not all async functions return promises. I still feel like a scheme to set up default returns would be more honest.

[gurpreetatwal]

I'm a little confused by your statement, "not all async functions return promises". async/await are just syntactic sugar around promises, so by definition async functions return promises.

[gurpreetatwal]

The point I'm trying to argue is that if you want the promise to resolve to something other than undefined you must use the td.when api, as that is the case for non-promise returning functions

[searls]

Right, but colloquially, a function that takes a callback and returns nothing is also referred to as "async". I don't want to confuse anyone by adding a special keyword that means "async keyword function but not callback API function"

[gurpreetatwal]

Ah okay, I accidentally assumed you were referring to functions that were using the async keyword and not just asynchronous functions in general.

I personally think using the word async could be fine as bluebird uses "Async" as the suffix for promisified functions by default and also because of the async keyword which implies promise. Having said this I do agree that this might be confusing to people who are new to promises or have never used promises.

We could make the default return value parameter an enum of sorts, that defaults to a normal function.

So defaultReturn (or type) would take the values normal, promise, or callback and would throw an error if anything else was passed in.

Ex:

// a function that returns undefined
td.func('some name');

// a function that returns a resolved promise
td.func('some name', {type: td.func.PROMISE});  

// a function that calls the last argument passed 
td.func('some name', {type: td.func.CALLBACK});

This way the api is expandable but there are limited and well defined "entry points"

[gurpreetatwal]

@searls hate to be a bother, but any thoughts? I'd be happy to assist with implementation once the interface is set

[searls]

I would support a PR that created a defaultReturn option, but it catches us in the middle of a rewrite, where func() has been rewritten in function/index but not yet integrated in the main code path, which means that the live code path is still in function.js, so this option would need to be implemented for both at the moment, which isn't ideal.

More broadly, since we have so many ways to create test doubles (func, replace, object, constructor), adding an options object arg to one of them doesn't sit well with me. (And neither does adding one to all of them.) At the same time, setting a global default with td.config seems overkill. I'm unsure what to do.

[searls]

Closing in favor of #372