jest-extended icon indicating copy to clipboard operation
jest-extended copied to clipboard

New matcher: .toHaveBeenCalledTimely(array)

Open acostalima opened this issue 7 years ago β€’ 11 comments

Feature Request

Follow-up of: https://github.com/facebook/jest/issues/5363

.toHaveBeenCalledTimely accepts an array of temporal differences in milliseconds which specify the maximum time in between invocations to a mock. The matcher fails when at least one of the specified timing constraints is not fulfilled.

Example:

it('should retry 2 times with exponential back-off with a factor of 2', async () => {
  jest.doMock('foo-api', jest.fn(() => fooApiMocks.helloSuccessAsync()));
  const fooApi = require('foo-api');  

  myModuleWhichDependsOnFooApi.execute(); 

  expect(fooApi.hello).toHaveBeenCalledTimely([  
      2000, // 1st retry - time between 1st and 2nd call to fooApi.hello()
      4000 // 2nd retry - time between 2nd and 3rd call to fooApi.hello()
    ]);
});

Following the reasoning outlined here and issue reported in #98, would the above be possible before #98 is addressed and resolved?

If possible, I'd very much like to work on a PR for this. πŸ˜„ Thanks!

acostalima avatar Jan 23 '18 18:01 acostalima

Hey @acostalima #98 has now been published in v0.6.0.

This matcher is possible to make now, like #98 to work the mocks will need to be asynchronous to be ran in different contexts to generate different timestamps. That is something that will happen in user land. So we can just assume the test author is doing this and perform the comparisons on the timestamps :smile:

If possible, I'd very much like to work on a PR for this. πŸ˜„

I'd be more than happy for you to send a PR for this, if you need any help just let me know πŸ‘ as a place to start take a look at 966db3c220c48c0892679a524848761d20dbe9f2

mattphillips avatar Feb 04 '18 00:02 mattphillips

@mattphillips Great! I'll work on this ASAP.

Concerning the implementation, the behavior I'm actually looking for, at least in my specific use case, it to ensure the mock is called only after the specified milliseconds have elapsed. So, following the example I presented above:

  • The 1st retry should only be called after 2000 milliseconds have elapsed since the first call
  • The 2nd retry should only be called after 4000 milliseconds have elapsed since the 1st retry

Sounds good?

acostalima avatar Feb 09 '18 14:02 acostalima

hey @acostalima, did you manage to do this? if not I am happy enough to raise a PR for this as it would be helpful for myself anyways.

benjaminkay93 avatar Jun 30 '18 18:06 benjaminkay93

@benjaminkay93 not yet, but I will have some time tomorrow. Thanks for the reminder too! πŸ™

acostalima avatar Jun 30 '18 20:06 acostalima

I just learned Jest does not store timestamps of mock's invocations anymore 😞(https://github.com/facebook/jest/pull/5867). Without such information, I don't think we can implement this matcher anymore... do we?

acostalima avatar Jun 30 '18 22:06 acostalima

Ahh I hadn't seen this 😒 it explains the changes to toHaveBeenCalledBefore that have been merged in but not published here.

It might be worth asking for this to be revisited to reinstate the timestamps to enable this, I can see a legitimate use case for a matcher such as this. Without that, this wouldn't be possible from what I know of the .mock information. If we had both we could maintain support for the toHaveBeenCalledBefore matcher in jest 22 as well as jest 23 in the next release of jest-extended.

benjaminkay93 avatar Jun 30 '18 23:06 benjaminkay93

I guess we can ask. However, .timestamps was explicitly removed in favor of .invocationCallOrder (https://github.com/facebook/jest/issues/4402, https://github.com/facebook/jest/pull/5867) due to precision issues, but @brigonzalez was headed towards a possible solution (https://github.com/facebook/jest/issues/4402#issuecomment-375939427).

acostalima avatar Jul 01 '18 14:07 acostalima

You can reinstate timestamps and use node’s process.hrtime() to calculate milliseconds between another process.hrtime() call. Although, not sure how process.hrtime() really looks as a β€œtimestamp” since it’s using an arbitrary process to calculate a time difference. Looks like convert-hrtime handles that conversion to milliseconds nicely.

brigonzalez avatar Jul 01 '18 18:07 brigonzalez

Ok. I'll start a discussion in Jest's repo to get feedback. If we are good to go, let me know if you'd like to do the honors, @brigonzalez, and open the PR to reinstate the record of timestamps. Otherwise, I can try to do it myself. πŸ˜„

acostalima avatar Jul 01 '18 23:07 acostalima

Sure thing! Let me know if we’ve got the green light, or shoot me a link to the discussion!

brigonzalez avatar Jul 02 '18 14:07 brigonzalez

@brigonzalez https://github.com/facebook/jest/issues/6672

acostalima avatar Jul 10 '18 21:07 acostalima