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

runTimersToTime is badly named/not clear from docs #4723

Closed
WickyNilliams opened this issue Oct 18, 2017 · 10 comments
Closed

runTimersToTime is badly named/not clear from docs #4723

WickyNilliams opened this issue Oct 18, 2017 · 10 comments

Comments

@WickyNilliams
Copy link

What is the current behavior?
runTimersToTime implies that it is idempotent, since you are running to a time. But it actually advances by a time.

What is the expected behavior?
Either renamed to something more descriptive (advanceTimersByTime), or behaviour changed so that the following test fails.

it("should run to a time, not advance by time", () => {
    jest.useFakeTimers();
    const mock = jest.fn();
    setTimeout(mock, 1000);

    jest.runTimersToTime(500);
    jest.runTimersToTime(500);

    expect(mock).toHaveBeenCalled();
  });

Please provide your exact Jest configuration and mention your Jest, node, yarn/npm version and operating system.

N/A

@SimenB
Copy link
Member

SimenB commented Oct 20, 2017

We can start by clarifying the docs. Could you send a PR for that?

@jack-lewin
Copy link
Contributor

I'd be happy to put in a PR for the docs.

If the behaviour is going to stay the same, would it be useful to add an alias for this function, with a view to renaming it in future?

@SimenB
Copy link
Member

SimenB commented Oct 23, 2017

I agree the name is not really indicative of what it does.

@cpojer up for an alias? Or just a rename, as we're getting close to a major release anyways

@cpojer
Copy link
Member

cpojer commented Oct 23, 2017

I'd prefer an alias for now.

@WickyNilliams
Copy link
Author

Excuse the delay in reply. Thanks for taking my suggestion on board!

@WickyNilliams
Copy link
Author

Closing, since @jack-lewin's PR has been merged. Thanks again!

@makeshift3ds
Copy link
Contributor

just a note: the documentation is ahead of the latest release. advanceTimersByTime is available in master but not 21.2.1. Not sure if that is intentional.

Docs
http://facebook.github.io/jest/docs/en/timer-mocks.html#advance-timers-by-time

@thymikee
Copy link
Collaborator

thymikee commented Nov 7, 2017

@makeshift3ds there should be a note: ##### available in Jest **21.3.0+**. Would you like to add this? 🙂

@makeshift3ds
Copy link
Contributor

@thymikee i adjusted the wording to make it clear the functionality is currently available under a different name.

#4857

@github-actions
Copy link

This issue 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

No branches or pull requests

6 participants