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

Stubbing callbacks #66

Closed
searls opened this issue Feb 12, 2016 · 20 comments
Closed

Stubbing callbacks #66

searls opened this issue Feb 12, 2016 · 20 comments

Comments

@searls
Copy link
Member

searls commented Feb 12, 2016

What if you could do this:

var fetch = td.function()

td.when(fetch('/some/path', td.callback(null, 'some data'))).thenReturn(undefined)

myFunction(fetch)

The td.callback API would just immediately invoke whatever function it captures at call-time with the provided arguments.

All so that you could synchronously specify this behavior in your source:

myFunction(fetch) {
  fetch('/some/path', function(er, data){
    console.log('my data is', data)
  }
}
@searls
Copy link
Member Author

searls commented Feb 12, 2016

Related: this introduces a situation where thenReturn() isn't really providing any value at all. Perhaps we should figure out how to let folks call when without chaining it.

@BaseCase
Copy link

In this example, is there some assumed wiring-up happening so that myFunction uses the correct fetch? (edit to clarify: since I am assuming that myFunction exists in a different file and is using a different version of fetch in production...that might be a bad assumption though)

@searls
Copy link
Member Author

searls commented Feb 12, 2016

@BaseCase yes i was just hand-waving - fixed

@davemo
Copy link
Contributor

davemo commented Feb 12, 2016

td.when(fetch('/some/path', td.callback(null, 'some data'))).done()

or

td.when(fetch('/some/path', td.callback(null, 'some data'))).andNoAndThen() :trollface:

@davemo
Copy link
Contributor

davemo commented Feb 12, 2016

I suppose alternatively you could just support the omission of a .thenX as indication there shouldn't be any followup expected.

@airhadoken
Copy link

Jasmine spies default to doing nothing and returning nothing (i.e. undefined) when first instantiated. That seems like a reasonable thing to do. I’m looking over the implementation of td.when() and I think it could be easily rengineered to start with a default stubbing of (undefined, config, ‘thenReturn’) and semantically set rather than add the stubbing if the user explicitly calls one of the then methods.

You will lose this previously valid pattern:

var fn = td.function();
var stub = fn.when();
fn({ “foo” : “bar” });
stub.thenReturn(“baz”);

…but is anyone using it like that?

@BaseCase
Copy link

Given that the API tries to be pretty English sentencey, maybe something like when(fetch('/some/path', td.callback(null, some data'))).thenCallbackImmediately()? I dunno, that's pretty cumbersome, but both when with nothing after it and thenReturn(undefined) feel kind of awkward to me. Just gut feels though! Don't have any stronger argument than that...

@airhadoken
Copy link

Sometimes (in rare cases) code relies on not having callbacks fire in-thread to work correctly. You might also want .thenCallbackLater() or .thenCallbackNextTick()

@airhadoken
Copy link

Also 👍 to @davemo's .done() for simplicity.

@jasonkarns
Copy link
Member

when is supposed to specify how the stubbing will be invoked, and we later specify (with .thenReturn) how it behaves.

I think the when(fetch(foo, td.callback(null, data))) seems weird because it's specifying both how fetch will be invoked and how it behaves.

So perhaps:

td.when(fetch('/some/path', td.callback)).thenCallback(null, 'with data')

This way the "how is the stubbing invoked" is still separate from "how does the stubbing behave".

(I'm not sure what purpose or value there is in having td.callback here, vs a generic function matcher.)

Open to options with streamlined errorBacks. (keeping the node interface would be consistent, but ugly for the 99% case that the callback is a success callback)

td.when(fetch('/some/path', td.callback)).thenCallback('with data') // same as cb(null, 'with data')
td.when(fetch('/some/path', td.callback)).thenErrorback('with err') // same as cb('with err')

@deanrad
Copy link

deanrad commented Feb 12, 2016

FWIW I find @jasonkarns syntax clearest, because of the separation he mentioned.

@searls
Copy link
Member Author

searls commented Feb 13, 2016

^ @davemo, that is my inclination. From a user's perspective that would be least surprising i think

@searls
Copy link
Member Author

searls commented Feb 13, 2016

@jasonkarns @deanius -- I started with what Jason suggested, but I dislike it for two reasons:

  • Having td.callback serve as a semantic marker for which argument the callback goes in is weird/awkward and people will get confused (they'd also get confused/upset if we just assumed the last argument was a callback and left it implicit).
  • Chaining thenCallback forfeits the opportunity for the function to also have a meaningful return value, and it's certainly possible someone would have a method that both took a callback param and had a useful return value (esp when the callback is a logical yield and not there for async reasons). I dislike that impedance mismatch after giving it some thought

@jasonkarns
Copy link
Member

Why would thenCallback prevent returning also? If thenCallback() returns the stubbing, you would just chain thenReturn on it as well.

td.when(fetch('/some/path', td.callback)).thenCallback(foo).thenReturn(bar). This also makes the synchronicity more explicit, anyway. (and i don't think it's a pattern that we would necessarily recommend, so the ugliness of the setup has valuable design pressure)

I'm conflicted on td.callback as a marker. But since we're supposed to be telling the stubbing how it must be invoked before behaving as prescribed, I would expect the when invocation to receive standard argument matchers/captors. Whether we have a magic one for callbacks shouldn't matter. It could be as simple as a standard 'any' matcher, or any other function matcher.

@searls
Copy link
Member Author

searls commented Feb 13, 2016

@jasonkarns after seeing the complete mess Sinon's chainable API became,
I'm not at all eager to adopt a multi-chaining API in this case.

In the example you gave I'd much rather just treat td.callback like an
argument matcher or captor and pass it (null, 'foo') and have that be
that.

On Sat, Feb 13, 2016 at 2:51 PM Jason Karns notifications@github.com
wrote:

Why would thenCallback prevent returning also? If thenCallback() returns
the stubbing, you would just chain thenReturn on it as well.

td.when(fetch('/some/path', td.callback)).thenCallback(foo).thenReturn(bar).
This also makes the synchronicity more explicit, anyway.

I'm conflicted on td.callback as a marker. But since we're supposed to be
telling the stubbing how it must be invoked before behaving as prescribed,
I would expect the when invocation to receive standard argument
matchers/captors. Whether we have a magic one for callbacks shouldn't
matter. It could be as simple as a standard 'any' matcher, or any other
function matcher.


Reply to this email directly or view it on GitHub
#66 (comment)
.

@searls
Copy link
Member Author

searls commented Feb 13, 2016

Aside from just wanting things to be terser (and avoiding a special marker
matcher singleton), after thinking a little more about it, another reason
why the impedance mismatch of mapping an argument (of which there are many)
to a test double function (of which there is one in a stubbing), is that a
thenCallback() API like is being discussed would preclude the
specification of any function that has two callbacks.

If someone has a series(callback1, callback2), then
td.when(series(td.callback(null, 'item1'), td.callback(null, 'item2'))
could specify the interaction, but a td.callback marker + thenCallback
would be unwieldy or unable to specify the interaction.

On Sat, Feb 13, 2016 at 5:47 PM Justin Searls searls@gmail.com wrote:

@jasonkarns after seeing the complete mess Sinon's chainable API became,
I'm not at all eager to adopt a multi-chaining API in this case.

In the example you gave I'd much rather just treat td.callback like an
argument matcher or captor and pass it (null, 'foo') and have that be
that.

On Sat, Feb 13, 2016 at 2:51 PM Jason Karns notifications@github.com
wrote:

Why would thenCallback prevent returning also? If thenCallback() returns
the stubbing, you would just chain thenReturn on it as well.

td.when(fetch('/some/path',
td.callback)).thenCallback(foo).thenReturn(bar). This also makes the
synchronicity more explicit, anyway.

I'm conflicted on td.callback as a marker. But since we're supposed to
be telling the stubbing how it must be invoked before behaving as
prescribed, I would expect the when invocation to receive standard
argument matchers/captors. Whether we have a magic one for callbacks
shouldn't matter. It could be as simple as a standard 'any' matcher, or any
other function matcher.


Reply to this email directly or view it on GitHub
#66 (comment)
.

@searls
Copy link
Member Author

searls commented Mar 9, 2016

Hey, a quick update: I implemented this on the ✈️ today. I ended up still requiring a chained thenReturn or a thenCallback, but otherwise I implemented basically all the options discussed in this thread.

These should all work (td.callback as a matcher w/ thenReturn):

td.when(fetch('/some/path', td.callback(null, 'some data'))).thenReturn(undefined)

And this (td.callback as position marker):

td.when(fetch('/some/path', td.callback)).thenCallback(null, 'with data')

And also this (implicit td.callback tacked on by the library as last argument when one isn't specified):

td.when(fetch('/some/path')).thenCallback(null, 'with data')

@searls searls closed this as completed in #70 Mar 9, 2016
@mbaranovski
Copy link

mbaranovski commented May 14, 2019

Hi @searls, is there any way force delay on td.callback only? I have an example like:

td.when(someAsyncFunction('arg1'), td.callback(null, 'some data'))).thenResolve('whatever');

I'd like someAsyncFunction to resolve first and then the data specified in the td.callback should come with a delay.
I've read the delay/defer sections but it seems delay/defer is only applicable to the chained method after td.when

Thanks

[EDIT]
So essentially something like:

td.when(someAsyncFunction('arg1'), td.callback(null, 'some data', {delay: 5}))).thenResolve('whatever');

@searls
Copy link
Member Author

searls commented May 14, 2019

This is already already supported, but if you want to control the callback function then you should configure it separately. Trying to do both in one shot would be too confusing, and any additional parameters to the td.callback matcher would be passed to the callback function, since all its args are forwarded.

Would this work?

const cb = td.when(td.func(), {delay: 500}).thenCallback(null, 'some data')
td.when(someFunc('arg1', cb)).thenResolve('whatever')

@mbaranovski
Copy link

That's what I was looking for, thanks!

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

No branches or pull requests

7 participants