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

[10.x] Sleep syncing #50392

Merged
merged 3 commits into from
Mar 7, 2024
Merged

[10.x] Sleep syncing #50392

merged 3 commits into from
Mar 7, 2024

Conversation

timacdonald
Copy link
Member

@timacdonald timacdonald commented Mar 7, 2024

Adds a testing helper to keep carbon in sync when sleeping.

// Test...
Sleep::fake();
Sleep::syncWithCarbon();

// Code...
$start = now();

Sleep::for(1)->second();

$start->diffForHumans(); // 1 second ago

Before you would commonly do the following...

Carbon::fake();

Sleep::whenFakingSleep(function ($duration) {
    Carbon::setTestNow(now()->add($duration);
});

Docs: laravel/docs#9462

@abenerd
Copy link
Contributor

abenerd commented Mar 7, 2024

I like this change, makes sense. I feel like this should be the default behavior for L11, would this be update for there too?

@taylorotwell taylorotwell merged commit f43ad93 into laravel:10.x Mar 7, 2024
23 of 24 checks passed
@timacdonald timacdonald deleted the sleep-syncing branch March 8, 2024 03:37
@timacdonald
Copy link
Member Author

@abenerd, I thought about that as well, but wondered if it might be too magical. I usually want my tests to be really explicit and if there are side-effects happening I haven't asked for it might cause headaches.

An application just put Sleep::syncWithCarbon() in their global set up so it applies to every test.

@abenerd
Copy link
Contributor

abenerd commented Mar 8, 2024

@timacdonald I feel like this is less magic and more like expected behavior, when I wake up after sleeping I want to get the correct time and not when I fell asleep. Alternatively one can also add the option to sync with carbon to the fake method, I think what "bugs" me the most is the extra call to syncWithCarbon you have to make. I could PR this.

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

Successfully merging this pull request may close these issues.

3 participants