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

Make error messages more explicit for toBeCalledWith assertions #3913

Merged
merged 1 commit into from
Jul 7, 2017

Conversation

mkubilayk
Copy link
Contributor

Summary

This adds more information to toBeCalledWith assertion fail messages by specifically telling which argument(s) didn't match with the expected value(s).

Fix #3523.

I'm not exactly sure about messages. Please have a look and share your ideas. Currently it reports back all argument mismatches from the last 3 (CALL_PRINT_LIMIT) calls for toBeCalledWith fails. All argument mismatches from the last call for lastCalledWith fails.

const fn = jest.fn();

fn(undefined, { foo: 'bar', fizz: 'buzz' });
fn({}, { foo: 'bar', fizz: 'buzz1' }, 2);
fn({}, { fizz: 'buzz' }, 3, 'extra');

expect(fn).toHaveBeenCalledWith(
    expect.anything(),
    expect.objectContaining({ fizz: 'buzz' }),
    3
);

jest-tobecalledwith-fail-1

const fn = jest.fn();

fn(undefined, { foo: 'bar', fiz: 'buzz' });
fn({}, { foo: 'bar', fizz: 'buzz1' });

expect(fn).toHaveBeenLastCalledWith(
    expect.anything(), 
    expect.objectContaining({ fizz: 'buzz' })
);

jest-tobecalledwith-fail-2

@thymikee
Copy link
Collaborator

wow, looks so much better!

@thymikee
Copy link
Collaborator

Could we reverse the ordering so it reads chronologically?
Also cc: @aaronabramov @cpojer what do you think?

@mkubilayk
Copy link
Contributor Author

Sure. Do you still want to show the last N calls but in chronological order? Or have the reports from the first N calls? Either way it's going to be inconsistent with what we have in toBeCalled, though.

@thymikee
Copy link
Collaborator

Maybe let's wait with that until we get more feedback on this

@codecov-io
Copy link

codecov-io commented Jun 26, 2017

Codecov Report

Merging #3913 into master will increase coverage by 0.08%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3913      +/-   ##
==========================================
+ Coverage   59.85%   59.93%   +0.08%     
==========================================
  Files         196      196              
  Lines        6760     6777      +17     
  Branches        6        6              
==========================================
+ Hits         4046     4062      +16     
- Misses       2711     2712       +1     
  Partials        3        3
Impacted Files Coverage Δ
packages/jest-matchers/src/spy_matchers.js 94.59% <100%> (-0.49%) ⬇️
packages/jest-matchers/src/utils.js 96.29% <100%> (+0.29%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a7acc5a...5b78d9b. Read the comment docs.

@@ -13,6 +13,7 @@
"jest-get-type": "^20.0.3",
"jest-matcher-utils": "^20.0.3",
"jest-message-util": "^20.0.3",
"jest-regex-util": "^20.0.3"
"jest-regex-util": "^20.0.3",
"lodash.partition": "^4.6.0"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't want this dependency. Can you inline it or find something that isn't lodash? Thanks.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will update. 👍

@cpojer
Copy link
Member

cpojer commented Jun 27, 2017

Yeah I like it.

@mkubilayk
Copy link
Contributor Author

@cpojer removed the dependency.

@cpojer cpojer merged commit a49c09a into jestjs:master Jul 7, 2017
@cpojer
Copy link
Member

cpojer commented Jul 7, 2017

Awesome! Thank you for making this work.

@mkubilayk mkubilayk deleted the called-with-errmsg branch July 7, 2017 16:31
@SimenB SimenB mentioned this pull request Jul 8, 2017
@SimenB SimenB mentioned this pull request Apr 17, 2018
@github-actions
Copy link

This pull request 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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 13, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve messaging for failed toHaveBeenCalledWith assertions
5 participants