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

expect.assertions() is not the expected number when using toHaveReceivedCommandWith #122

Closed
1 task done
ColinMorris83 opened this issue Sep 30, 2022 · 7 comments
Closed
1 task done
Labels
bug Something isn't working

Comments

@ColinMorris83
Copy link

Checklist

  • I have read Caveats documentation and didn't find a solution for this problem there.

Bug description

After upgrading to version 2 and using the separate package for jest matchers, tests that are using an expect of toHaveReceivedCommandWith are now failing because the number of expects is now coming out as 1 higher than it was previously.

Reproduction

e.g.

expect.assertions(1);
const res = await handler(mockEvent, mockContext, callbackSpy);
expect(dynamoMock).toHaveReceivedCommandWith(GetCommand, {
      Key: {
           QueueName: 'myQueueName',
      },
      TableName: 'mytablename',
});

Will give this error:
Expected one assertion to be called but received two assertion calls

I am wondering if it is because toHaveReceivedCommandWith itself contains an expect call inside the function, and that is now being added to the number of expects called.

Environment

  • Node version: v16.14.0
  • Testing lib and version: Jest 29.1.2
  • Typescript version: 4.8.4
  • AWS SDK v3 Client mock version: 2.0.0
  • AWS JS SDK libs and versions:
    • "@aws-sdk/client-dynamodb": "~3.181.0",
    • "@aws-sdk/lib-dynamodb": "~3.181.0"
@ColinMorris83 ColinMorris83 added the bug Something isn't working label Sep 30, 2022
@m-radzikowski
Copy link
Owner

With the same Jest and TS version, I did run this test:

import {mockClient} from 'aws-sdk-client-mock';
import 'aws-sdk-client-mock-jest';
import {PublishCommand, SNSClient} from '@aws-sdk/client-sns';

const snsMock = mockClient(SNSClient);

it('asserts 1 time', async () => {
    expect.assertions(1);

    const sns = new SNSClient({});
    await sns.send(new PublishCommand({TopicArn: '', Message: ''}));

    expect(snsMock).toHaveReceivedCommand(PublishCommand);
});

and it works fine. Please double-check your code and, if the problem preserves, create a small repo with reproduction.

@ColinMorris83
Copy link
Author

ColinMorris83 commented Oct 9, 2022

Please try it with toHaveReceivedCommandWith, that's the one that within the function itself calls expect, which I suspect is the why there is an extra assertion.

@m-radzikowski
Copy link
Owner

Sorry, yes, you are right. The problem is with the toHaveReceivedCommandWith function that internally does the assertion(s) as well.

I will check if it's possible to easily get rid of it to get the assertions number check working correctly.

@jaska120
Copy link

What do you think if all calls of

expect(received).toEqual(
    expect.objectContaining(input),
);

Could be replaced with e.g. deep-equal library?

More specifically in

expect(received).toEqual(
expect.objectContaining(input),
);
&
expect(received.input).toEqual(
expect.objectContaining(input),
);
.

If the equality is not met, we could manually return jest like error messages stating where the issue is. Or is there a way to use the current expect implementation from Jest without increasing the count of assertions?

To consider: as the v2 has been out some time, this would be a breaking change and major version bump should probably be made, even when it is fixing regression in v2.

@m-radzikowski
Copy link
Owner

The currently used expect.objectContaining() is doing a partial match - only properties specified in the input are checked for equality, other are ignored. With deep-equal we probably need to select a subset of properties to compare first, but it's doable.

If you are willing to submit a PR, I would gladly review it.

Re version and breaking change... https://xkcd.com/1172/

But I guess there is no harm in bumping the major version, why not.

@lobbin
Copy link
Contributor

lobbin commented Feb 1, 2024

Added a potential PR to fix this behaviour. Didn't bump any versions :)
#209

@m-radzikowski
Copy link
Owner

Released in v4.0.0. Thank you @lobbin!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants