Added support for naming mocked functions #4586

gricard · 2017-10-02T21:52:05Z

Summary

This patch adds the ability to specify a name for a just.fn() mock, so if the test fails, you will see that name in the error output instead of "just.fn()", which should help make some test results clearer. It provides a few different ways to specify the mock name.

Motivation

I switched a frontend product over to Jest from Karma/Mocha/Chai a few months ago, and I love it. But there's a couple things that I wanted to see in the output, like specified mock names, and I did not see any existing means to add that to my tests.

Test plan

Unit tests and integration tests have been added.

Update from master

codecov-io · 2017-10-02T23:05:44Z

Codecov Report

Merging #4586 into master will decrease coverage by <.01%.
The diff coverage is 50%.

@@            Coverage Diff             @@
##           master    #4586      +/-   ##
==========================================
- Coverage   55.68%   55.67%   -0.01%     
==========================================
  Files         186      186              
  Lines        6348     6356       +8     
  Branches        3        3              
==========================================
+ Hits         3535     3539       +4     
- Misses       2812     2816       +4     
  Partials        1        1

Impacted Files	Coverage Δ
packages/jest-mock/src/index.js	`83.88% <50%> (-1.03%)`	⬇️

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 7aa3017...b303876. Read the comment docs.

SimenB · 2017-10-06T15:06:31Z

@gricard You can add an integration test, there you can get both stdout and stderr. Lots of examples in there

gricard · 2017-10-06T18:49:05Z

Thanks, @SimenB.

gricard · 2017-10-06T22:36:01Z

Interesting... AppVeyor build failed, and it looks like it's because the snapshot tests failed due to slight differences in the characters used. Should I not have committed the snapshots created on my Mac? That appears to be the problem. Output differences between macOS / Windows.

    Received value does not match stored snapshot 1.
    
    - Snapshot
    + Received
    
    - "PASS __tests__/index.js
    -   ✓ first test
    + "PASS __tests__\\index.js
    +   √ first test
      
      "

…ts vary by platform, apparently

gricard · 2017-10-07T01:26:46Z

Well, if the test output varies by platform, then I'll do regex matching instead

SimenB · 2017-10-07T06:37:01Z

You can do skipOnWindows, not sure if that's wanted here, though

SimenB · 2017-10-07T12:31:44Z

docs/en/MockFunctionAPI.md

@@ -138,13 +138,32 @@ console.log(myMockFn(), myMockFn(), myMockFn(), myMockFn());
 > 'first call', 'second call', 'default', 'default'
 ```

-### `mockFn.mockReturnThis()`


why is this removed?

Looks like an accident on my part. I'll fix it.

SimenB · 2017-10-07T12:38:29Z

integration_tests/mock-names/with-empty-mock-name/__tests__/index.js

+test('first test', () => {
+  mockFn();
+  expect(mockFn).toHaveBeenCalled();
+  expect(mockFn.mock.calls.length).toBe(1);


any particular reason why you do this instead of expect(mockFn).toHaveBeenCalledTimes(1)?

No reason. I just copied what was in another test suite in there that I based mine on.

SimenB · 2017-10-07T12:43:41Z

integration_tests/__tests__/mock_names.test.js

@@ -0,0 +1,70 @@
+/**


Can you add a test for failing expect(mockWithName).not.toHaveBeenCalled() and expect(mockWithName).toHaveBeenCalledTimes(5) to assert on the name for them as well? I see no reason for them not to work as you expect, but having the assertions would be nice

So, a test where the assertion expect(mockWithName).not.toHaveBeenCalled() will fail, and then check for that error?

And a test for expect(mockWithName).toHaveBeenCalledTimes(5) when it's not called 5 times?

Ok, I've added more tests to cover those.

Yup. Thanks!

cpojer · 2017-10-07T19:51:19Z

I quite like it. Thanks for contributing this change to Jets. However, I'd prefer to not add an argument to jest.fn at this point. Could we change this PR to only add .mockName? Thank you.

gricard · 2017-10-08T00:34:41Z

Whatever floats your boat.

…fn() and only use jest.fn().mockName() to set it

Remove obsolete unit tests

gricard · 2017-10-08T02:09:44Z

Hm. Well, I made the changes, but circleci test-node-8 is failing because of an unrelated test failure.

 FAIL  integration_tests/__tests__/transform.test.js (24.846s)
  ● babel-jest › runs transpiled code

    
          ORIGINAL CMD: yarn
          STDOUT: 
          STDERR: warning package.json: No license field
    warning ../../package.json: No license field
    warning No license field
    error An unexpected error occurred: "https://registry.yarnpkg.com/core-js/-/core-js-2.4.1.tgz: Request failed \"530 undefined\"".
    
          STATUS: 1
          ERROR: undefined
      
          
      at run (integration_tests/utils.js:33:11)
      at Object.beforeEach (integration_tests/__tests__/transform.test.js:28:7)
          at Promise (<anonymous>)
          at <anonymous>

SimenB · 2017-10-08T07:26:36Z

Restarted it, and CI is happy now

gricard · 2017-10-08T14:43:43Z

Sweet. Thanks, dude.

SimenB · 2017-10-08T15:05:27Z

Question: Is the name cleared by calling mock.mockReset()? Should it be?

gricard · 2017-10-08T16:44:56Z

Ooh. Yeah. That makes sense. I didn't think of that. I can change it so that clears out the name also.

SimenB · 2017-10-08T16:53:05Z

I think it should mockReset and mockRestore should clear it, but not mockClear.

gricard · 2017-10-08T17:42:10Z

I took a look at the docs for all of those. I think only mockReset() makes sense, in terms of clearing out the name. mockRestore() is specific to the mocked implementation.

cpojer · 2017-10-08T17:46:04Z

Yep, agreed. :)

…e() value also

gricard · 2017-10-08T18:03:47Z

It looks like mockReset() already does this, because it clears out the mock entirely with its config (where the mock name is stored). I've added a unit test to verify this, and updated the documentation for mockReset().

SimenB · 2017-10-08T18:06:32Z

For funsies, can you add a test that verifies that mock{Clear,Restore} does not clear the name?

(I always have to check the docs for the purpose of the 3 different resets, so having super explicit tests is a good thing I think).

It should be good to merge either way, this is awesome! Gonna start using it as soon as it's released 🙂

…e() value

gricard · 2017-10-08T18:21:10Z

All set, @SimenB

gricard · 2017-10-08T18:22:12Z

And thanks! I really like Jest and this will help me out a lot as well. Hopefully I can contribute some more to the project. :)

SimenB

LGTM (pending CI). Thanks for sticking around for our nit-picking

cpojer · 2017-10-08T19:54:11Z

Alright, I think this is almost good to go, I would just like to request one more change (sorry :( ). Could we split the function up into mockName as a setter and getMockName to get the value? I generally don't like functions that behave differently based on the amount of arguments. Thank you.

… getMockName() instead of overloading mockName() function to return it when no arguments are passed

gricard · 2017-10-08T20:34:44Z

All set

cpojer · 2017-10-08T20:38:01Z

Thanks for sticking with us, this is a great PR :)

gricard · 2017-10-08T21:08:58Z

Thanks, dude!

SimenB · 2017-10-08T21:23:32Z

docs/en/MockFunctions.md

+  .mockImplementation(scalar => 42 + scalar)
+  .mockName('add42');
+
+const myMockFn2 = jest.fn(scalar => 42 + scalar, 'add42');


this is wrong now

Damn. I thought I caught all the doc references. Can I still commit changes for this PR, or is that done since it's merged?

New PR is needed

SimenB · 2017-10-08T21:24:10Z

docs/en/MockFunctions.md

@@ -217,6 +217,19 @@ const otherObj = {
 };
 ```

+## Mock Names


should it include a "availble in jest 21.4" or something?

github-actions · 2021-05-13T05:11:21Z

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.

gricard added 2 commits October 2, 2017 16:38

Merge pull request #1 from facebook/master

5ed77b0

Update from master

Added ability to specify a name for mocked functions

348b77f

facebook-github-bot added the CLA Signed ✔️ label Oct 2, 2017

gricard added 3 commits October 6, 2017 16:04

Added integration tests for mock name functionality

c39eb37

Added unit tests for mock name functionality

76936ce

Linting fix

2f8066e

Switched from snapshot matching to regex matching because the snapsho…

29623ee

…ts vary by platform, apparently

SimenB reviewed Oct 7, 2017

View reviewed changes

Re-add accidentally removed doc section

9258feb

SimenB reviewed Oct 7, 2017

View reviewed changes

Fix the order of replaced doc section

2d284a4

SimenB reviewed Oct 7, 2017

View reviewed changes

gricard added 3 commits October 7, 2017 08:53

Fix test syntax

10f49e9

Additional unit tests

f8c7014

Fix linting problems

8202562

SimenB approved these changes Oct 7, 2017

View reviewed changes

gricard added 2 commits October 7, 2017 21:02

Removed functionality of having mock name passed as argument to jest.…

bb547bb

…fn() and only use jest.fn().mockName() to set it

Linting fix

8a05ec4

Remove obsolete unit tests

gricard added 2 commits October 8, 2017 14:03

Add a test to confirm that mockReset() clears out the mockName() value.

b53639b

Updated documentation to note that mockReset() clears out the mockNam…

8037727

…e() value also

Added tests to ensure mockClear() & mockReset() do not affect mockNam…

1aaa2e0

…e() value

SimenB approved these changes Oct 8, 2017

View reviewed changes

Changed mockName() API to return the current mock name via a separate…

a116716

… getMockName() instead of overloading mockName() function to return it when no arguments are passed

Update MockFunctionAPI.md

b303876

cpojer merged commit efe65d4 into jestjs:master Oct 8, 2017

SimenB reviewed Oct 8, 2017

View reviewed changes

chrismohr mentioned this pull request Jan 4, 2018

[jest] add mockName and getMockName DefinitelyTyped/DefinitelyTyped#22682

Merged

6 tasks

renovate-tests mentioned this pull request Feb 6, 2018

Update dependency jest to v22 - autoclosed renovate-tests/changelog-md#3

Closed

renovate bot mentioned this pull request Feb 14, 2018

fix(deps): update dependency jest to ^22.0.0 manekinekko/devoxx-assistant#3

Open

github-actions bot locked as resolved and limited conversation to collaborators May 13, 2021

Added support for naming mocked functions #4586

Added support for naming mocked functions #4586

Conversation

gricard commented Oct 2, 2017 • edited Loading

codecov-io commented Oct 2, 2017 • edited Loading

Codecov Report

SimenB commented Oct 6, 2017

gricard commented Oct 6, 2017 • edited Loading

gricard commented Oct 6, 2017

gricard commented Oct 7, 2017

SimenB commented Oct 7, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SimenB Oct 7, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SimenB Oct 7, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cpojer commented Oct 7, 2017

gricard commented Oct 8, 2017

gricard commented Oct 8, 2017

SimenB commented Oct 8, 2017

gricard commented Oct 8, 2017

SimenB commented Oct 8, 2017

gricard commented Oct 8, 2017

SimenB commented Oct 8, 2017

gricard commented Oct 8, 2017

cpojer commented Oct 8, 2017

gricard commented Oct 8, 2017

SimenB commented Oct 8, 2017 • edited Loading

gricard commented Oct 8, 2017

gricard commented Oct 8, 2017

SimenB left a comment • edited Loading

Choose a reason for hiding this comment

cpojer commented Oct 8, 2017

gricard commented Oct 8, 2017

cpojer commented Oct 8, 2017

gricard commented Oct 8, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented May 13, 2021

gricard commented Oct 2, 2017 •

edited

Loading

codecov-io commented Oct 2, 2017 •

edited

Loading

gricard commented Oct 6, 2017 •

edited

Loading

SimenB Oct 7, 2017 •

edited

Loading

SimenB Oct 7, 2017 •

edited

Loading

SimenB commented Oct 8, 2017 •

edited

Loading

SimenB left a comment •

edited

Loading