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

Mock timestamp #4866

Merged
merged 4 commits into from Nov 9, 2017
Merged

Mock timestamp #4866

merged 4 commits into from Nov 9, 2017

Conversation

mattphillips
Copy link
Contributor

Summary

This PR adds the internal behaviour to the mock state to record a timestamp on every invocation to allow community matchers to solve #4402.

I'm happy to add the matcher wanted in #4402 to jest-extended

Test plan

  • Unit tests for new timestamp

@codecov-io
Copy link

codecov-io commented Nov 9, 2017

Codecov Report

Merging #4866 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4866      +/-   ##
==========================================
+ Coverage   59.24%   59.25%   +<.01%     
==========================================
  Files         200      200              
  Lines        6647     6648       +1     
  Branches        3        4       +1     
==========================================
+ Hits         3938     3939       +1     
  Misses       2709     2709
Impacted Files Coverage Δ
packages/jest-mock/src/index.js 84.47% <100%> (+0.05%) ⬆️

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 2a2429e...53b25f1. Read the comment docs.

@@ -313,6 +315,7 @@ class ModuleMockerClass {
const mockConfig = mocker._ensureMockConfig(f);
mockState.instances.push(this);
mockState.calls.push(Array.prototype.slice.call(arguments));
mockState.timestamps.push(new Date().getTime());
Copy link
Member

Choose a reason for hiding this comment

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

Date.now()

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.

Thanks for tackling this!

@mattphillips
Copy link
Contributor Author

@SimenB I've updated this to use Date.now() good suggestion 😄

@cpojer cpojer merged commit 67b4534 into jestjs:master Nov 9, 2017
@mattphillips mattphillips deleted the mock-timestamp branch November 9, 2017 11:08
@ChrisCinelli
Copy link

ChrisCinelli commented Jan 5, 2018

Unfortunately this will not solve #4402 if the calls happen at the same time (see jest-community/jest-extended#98 (comment)).
For this use case, in order to implement .toHaveBeenCalledBefore, we need a global counter incremented at every call. The current value can be saved in .mock.invocationOrder (this is what jasmine uses).
Pretty simple implementation but it will require a new PR.

@AlanFoster
Copy link

AlanFoster commented Feb 2, 2018

@ChrisCinelli Confirming that I've run into the same problem too -

  Expected first mock to have been called before, timestamps:
    [1517598132978]
  Received second mock with timestamps:
    [1517598132978]

Is this a PR you're currently working on? :]

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

None yet

7 participants