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

Issue #5197: Add descriptive error to Expect CalledWith methods when missing optional arguments #5547

Merged
merged 5 commits into from
Feb 15, 2018

Conversation

jessecarfb
Copy link
Contributor

@jessecarfb jessecarfb commented Feb 13, 2018

Summary

Issue: #5197
Specifically, formatMismatchedArgs will not add an error message when the received array gets extra argument(s) w/ undefined value since expected[i] and received[i] will both equal undefined (where received[i] is explicitly set to undefined whereas expected[i] is not in the array)

Test plan

Tested using @mrfunkycold repo: https://github.com/mrfunkycold/jest-demo. New error message:

● testFile › 1. when invoked › 1. fails to print the error line
expect(jest.fn()).toHaveBeenCalledWith(expected)

Expected mock function to have been called with:
  Did not expect argument 2 but it was called with undefined.
  11 | 
  12 |     it('1. fails to print the error line', () => {
> 13 |       expect(logger).toHaveBeenCalledWith('errorMsg');
  14 |     });
  15 |   });
  16 | 
  
  at Object.it (src/typescript_version/testFile.test.ts:13:22)

@codecov-io
Copy link

codecov-io commented Feb 13, 2018

Codecov Report

Merging #5547 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #5547   +/-   ##
=======================================
  Coverage   61.67%   61.67%           
=======================================
  Files         213      213           
  Lines        7160     7160           
  Branches        4        4           
=======================================
  Hits         4416     4416           
  Misses       2743     2743           
  Partials        1        1

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 4ca959b...b5118f0. Read the comment docs.

Copy link
Member

@SimenB SimenB left a comment

Choose a reason for hiding this comment

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

Could you add a test showing that the error message is correct?

@SimenB
Copy link
Member

SimenB commented Feb 14, 2018

Nice! Could you update the changelog as well? 🙂

@jessecarfb
Copy link
Contributor Author

Yep, thanks for reviewing @SimenB! Do we update the changelog for every change or is there a minimum bar?

@SimenB
Copy link
Member

SimenB commented Feb 14, 2018

My rule of thumb is every change that changes some behaviour should get an entry

@jessecarfb jessecarfb changed the title Issue #5197: Add descriptive error when undefined is passed as unexpe… Issue #5197: Add descriptive error to Expect CalledWith methods when missing optional arguments Feb 14, 2018
@SimenB
Copy link
Member

SimenB commented Feb 14, 2018

Can you rebase? The diff in this PR is hard to read with all the extra commits

@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 12, 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.

5 participants