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] Add clear() method to facade fakes #46252

Closed
wants to merge 4 commits into from
Closed

[10.x] Add clear() method to facade fakes #46252

wants to merge 4 commits into from

Conversation

jerodev
Copy link
Contributor

@jerodev jerodev commented Feb 23, 2023

When testing using facade fakes, it used to be possible to override the facade by faking again to reset the state. Since #46188, this is no longer possible.

This PR includes a clear() function on these facades that allows clearing the internal queue/event/... history.


For example, I used to do the following to test notifications over a span of time:

$this->sendReminders();
Notification::assertSentTo($user, ReminderMail::class);

Notification::fake();
$this->travel(3)->days();
$this->sendReminders();
Notification::assertNothingSent();

Notification::fake();
$this->travel(6)->days();
$this->sendReminders();
Notification::assertSentTo($user, ReminderMail::class);

This new clear method can now be used instead to clear the internal $notifications array.

@driesvints
Copy link
Member

Thanks but we're going to be reverting all of these PRs up till the original one that started the chain of breaking changes.

@driesvints driesvints closed this Feb 24, 2023
@jerodev jerodev deleted the feature/clear-facade-fakes branch February 24, 2023 09:26
@driesvints
Copy link
Member

@jerodev we decided to fix it differently: #46257

Can you confirm if that works for you?

@jerodev
Copy link
Contributor Author

jerodev commented Feb 24, 2023

Can you confirm if that works for you?

@driesvints All tests successful here, thanks! 👍

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.

2 participants