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

Add shift to TimeMachineFixture #247

Closed
henribru opened this issue May 4, 2022 · 7 comments
Closed

Add shift to TimeMachineFixture #247

henribru opened this issue May 4, 2022 · 7 comments

Comments

@henribru
Copy link

henribru commented May 4, 2022

Description

It would be convenient if TimeMachineFixture supported shift in addition to move_to. You can work around the lack of this by doing time_machine.coordinates.shift(...), but it's slightly more verbose and requires you to have already called move_to. It also doesn't interact great with a typechecker since it will think time_machine.coordinates could still be None even if you've called move_to.

Would a contribution for this feature be accepted?

Edit: It would also simplify migration from pytest-freezegun since there you have both move_to and tick

@adamchainz
Copy link
Owner

Yes sure!

@AgDude
Copy link
Contributor

AgDude commented Nov 2, 2022

I was thinking through what this api would be. In the case one has already called move_to, it seems clear that time_machine.shift is simply an alias to time_machine.coordinates.shift.

There are other cases where a test doesn't care about the actual time, simply that some time has elapsed. In this case a user might call time_machine.shift without first calling move_to, my expectation is that this would create a new traveler starting at the current (real) time, then apply the shift. Creating a traveller without an explicit start point appears to be new functionality to time-machine. One very real case where this would be helpful is when Django data migrations create db records with a created timestamp, then a test needs to perform some action after those are created.

@adamchainz what are your thoughts on this logic? And if you think it makes sense at all, is the fixture the right place for it? From a quick look at the code, I am not sure where else to put it, unless the travel class made the destination optional.

@AgDude
Copy link
Contributor

AgDude commented Nov 2, 2022

I see similar functionality has been proposed in #38. That seems like a good way to address my concern about putting entirely new functionality into the fixture, since the fixture could simply use a zero delta to initialize the traveller.

@adamchainz
Copy link
Owner

@AgDude adding timedelta() support would be neat. Would you like to make a PR?

@soxofaan
Copy link
Contributor

I created PR #312 to add shift() method to the pytest fixture.

It just addresses the easy part of this request: when move_to() has been used before.
The use case discussed by @AgDude , where move_to() is not called first, the implementation just raises a RunTimeError at the moment.

soxofaan added a commit to soxofaan/time-machine that referenced this issue Dec 16, 2022
soxofaan added a commit to soxofaan/time-machine that referenced this issue Jan 9, 2023
adamchainz pushed a commit to soxofaan/time-machine that referenced this issue Sep 19, 2023
@llucax
Copy link

llucax commented Dec 29, 2023

Isn't this done?

@adamchainz
Copy link
Owner

Yup, thanks for noting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants