Feature Request: Support asynchronous matchers

[jcarlson]

Do you want to request a feature or report a bug?

Feature

What is the current behavior?

Custom matchers must return an object of the form:
{ pass: [boolean], message: [string|function] }.

It would be helpful to support matchers that return a Promise, thereby allowing custom matchers that can utilize async/await to flush pending Promises.

This may be of use for folks waiting on #2157.

I'd like to be able to have a matcher something of the form (though this is a contrived example):

// matcher
async function toAwaitAsync (received, arg) {
  const pass = await received(arg)
  return { pass, message: 'expected one thing, got another' }
}

// thing to test
async function doStuff (arg) {
  await Promise.resolve()
  return !!(arg)
}

// test
test('something async', () => {
  expect(doStuff).toAwaitAsync('foo')
})

[thymikee]

Whoa, I like the idea.

[quantizor]

Shouldn't the new resolves matchers handle this since async functions just return promises?

[jcarlson]

@probablyup I've not used the resolves matchers, I'll take a look. But I think they would still put an impetus on the test to know something about the async nature of the thing it's testing, because the test would have to include .resolves when writing the matcher predicate.

My real motivation behind this is for testing action creators in Redux. When a component calls an action creator, it doesn't (and shouldn't) really know if the action creator returns an action object or a thunk. As such, I'd like to be able to write tests that look something like this:

// actions.js
export function someAction () {
  return async function (dispatch) {
    dispatch({ type: 'START_OF_ASYNC_THING' })
    await new Promise(res => setTimeout(res, 0))
    dispatch({ type: 'END_OF_ASYNC_THING' })
  }
}

// actions-test.js
import * as actions from './actions'

test('some action creator', () => {
  let action = actions.someAction()
  mockStore.dispatch(action)
  expect(mockStore).toHaveDispatched({ type: 'START_OF_ASYNC_THING' })
  expect(mockStore).toHaveDispatched({ type: 'END_OF_ASYNC_THING' })
})

With asynchronous support on the matcher side, the matcher itself could yield for a nextTick or a short timeout until either time expires, or the required action was dispatched through the mockStore.

[cpojer]

I think .resolves and .rejects fills this use-case reasonably nicely for now.

[nickserv]

I have a group of tests for an async function that replaces content in strings. In some test cases, I need to ensure that a given string changes or does not change after running the function. It would be great to be able to extract this testing behavior into a custom matcher, but I can't because the behavior has to be asynchronous through a Promise (it's making network requests through a third party library). If I used the rejects or resolves matchers, I would not be able to directly pass an input string to expect() and implicitly call the test function in a matcher, the expected value itself has to be a Promise and I can't wait on a Promise generated by the matcher. I would personally find async matcher support very helpful for this use case. For now, I have a function that takes a string and returns a test function that I can pass to test() or it().

Before

const expectToUpdate = url => () => expect(cdnm.update(url)).resolves.toBe(replaceVersion(url))
const expectNotToUpdate = url => () => expect(cdnm.update(url)).resolves.toBe(url)

test('empty string', expectNotToUpdate(''))
test('complete html document', expectToUpdate(html))

After

test('empty string', () => expect('').not.toUpdate())
test('complete html document', () => expect(html).toUpdate())

[SimenB]

Can't you use expect(await someAsyncThing).toBe(success)?

I haven't read through this issue, so might be missing something

[nickserv]

Unfortunately no, because I want the matcher's internal logic to be asynchronous while passing a synchronous value to the matcher. In this case, I would be passing a String (not wrapped in a Promise) to a toUpdate() matcher, and internally it would call and await on a function that returns a new String wrapped in a Promise. If I did expect(await cdnm.update(url)).toBe(url), I would have to repeat the url String twice in each test case. It would be nice to use a matcher to reduce code duplication in this case.

[therynamo]

@cpojer

I think .resolves and .rejects fills this use-case reasonably nicely for now.

Does the .resolves/.rejects handle custom matchers whose internals are async? For instance, if I have something like the following (using puppeteer):

// toMatchTextContent.js
const toMatchTextContent = async (received, expected) => {
  const isElementHandle = testUtils.isPuppeteerConstructor(received, 'ElementHandle');

  if (!isElementHandle) {
    return { actual: received, message: testUtils.constructorError('ElementHandle').message, pass: false };
  }

  const textContentProperty = await received.getProperty('textContent');
  const textJson = await textContentProperty.jsonValue();

  const pass = textJson.match(expected);

// rest of matcher and message logic
....
}

// toMatchTextContent.test.js

// The test now requires that you use `await` on the expect

it('should match text from an ElementHandle', async () => {
  await page.goto('http://www.example.com/');

  const header = await page.$('h1');

  await expect(header).toMatchTextContent('Example Domain');
});

Using await on expect seems to go against the grain of what users have come to expect. When writing custom matchers, you can point to some disclaimer of sorts to call that out, but there will probably still be pain having to prepend that expect.

I haven't personally had success with .resolves/.rejects heping in this situation (like @nickmccurdy mentioned), I may be using it incorrectly. If not, would you feel comfortable opening this issue back up for discussion?

[nickserv]

Does the .resolves/.rejects handle custom matchers whose internals are async?

I don't think so, that's the issue I'm having. In my opinion resolves and rejects do not fix this issue because the matcher API is still synchronous, and it should be reopened (though they are very useful in simpler cases where I don't need to implement a custom matcher).

[therynamo]

Agreed on resolve/reject usefulness in test cases.

I poking around in the CustomMatcher code, and it seems like this wouldn't be a bad place to implement something like this. I can throw something together.

Do you have any thoughts or ideas on implementation details?

Follow Up

I think here is where we'd want to "copy"/make similar. It is the promise resolution of the matcher function itself that is the tricky part.

[bilby91]

@therynamo I really want this feature to be able to write some custom matchers to DRY our test suite.

At the moment I have a working version that lets you create async matchers but only works in combination with resolves. I'll probably push a PR tomorrow.

Would love some feedback and any ideas on how to support this without having to use promises inside the expect call.

[bilby91]

@therynamo #5919

[therynamo]

Hey @bilby91, nice PR! Seems to make sense.

On important aspect to take into account is that asyncMatchers return a Promise, await will be needed to run the matcher.

This was the one thing I was trying to solve when I was working on this issue. I didn't like the idea of having to write tests like this #3874 (comment). However, incremental changes towards not having to do that are great. So it seems like what you have is a step in the right direction. 👍

[github-actions]

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.