closure-library icon indicating copy to clipboard operation
closure-library copied to clipboard

Add $returnsAsync to mocks for working with async functions

Open cletusw opened this issue 6 years ago • 3 comments

When mocking async functions, it's not always obvious that $returns doesn't wrap the provided value in a Promise. Closure could provide a $returnsAsync method that wraps the provided value in a Promise.

Simplified example:

const utils = {
  async getData() {
    return await 42;
  },
};

async function myAsyncFunction() {
  console.log(await utils.getData());
}

function myNonAsyncFunction() {
  utils.getData().then((data) => console.log(data));
}

Tests:

const mock = goog.testing.createMethodMock(utils, 'getData');
mock().$returns(43).$times(2);
myAsyncFunction(); // Works, but only because `await utils.getData()` automatically wraps the 43 in a Promise
myNonAsyncFunction(); // Fails, because numbers aren't thenable

Current solutions:

mock().$returns(Promise.resolve(43));
mock().$does(async () => 43);

Proposal:

mock().$returnsAsync(43);

More than just reducing the character count, I think having this method available would remind developers of the need to do this. Additionally, if there's any way to know that the mocked method is an async function we could warn developers when passing in a non-Promise to $returns and suggest they use $returnsAsync instead.

cletusw avatar Feb 07 '19 22:02 cletusw

Personal thoughts: There's a more general problem that our mocking framework isn't type checkable. If it was, then having the compiler type check your tests would catch this.

If we did ever get to the point where mocks were type checkable I don't think we could have a $returnsAsync function that was checked correctly unless we had overloads. Generally the way to type check mocks is to have a when() method roughtly typed like

/**
 * @param {T} mock
 * @return {!Expectation<T>}
 * @template {T}
 */
function when(mock) {}

/** @template {T} */
class Expectation {
  /** @param {T} value */
  $returns(value) {}
  // etc
}

So an example usage would look like when(mock.methodCall(paramMatcher)).$return(0);. Using your above example, when(mock()).$returns(43) would fail to compile, as you passed a number where it expects a !Promise<number>, forcing you to change to when(mock()).$returns(Promise.resolve(43)) to compile.

Without overloads when() can only return a single value (at type checking), which would need both $returns and $returnsAsync on it, meaning nothing is preventing you from calling the wrong one. Though I guess you could have a whenAsync(mock()).$return(0) instead, where whenAsync has an AsyncExpectation or something?

jplaisted avatar Feb 07 '19 23:02 jplaisted

Yeah, the real problem is the lack of type-checking here, so if that can be solved, that would be better.

cletusw avatar Feb 07 '19 23:02 cletusw

As an aside, this is pretty reasonable to do in TypeScript: one can unwrap the promise easily enough.

shicks avatar Feb 26 '19 22:02 shicks