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

Using throw config of filesystem disks when faking #53779

Merged

Conversation

emulgeator
Copy link
Contributor

Problem

When production code initializes a storage instance, it relies on the filesystem configuration, which includes a crucial setting called throw. Enabling throw removes the need for additional checks in the code to verify whether file operations succeeded, as the exceptions themselves provide that information. Moreover, production code often depends heavily on the exceptions thrown.

However, the Storage::fake() method, by default, creates a storage instance where errors do not trigger exceptions. To align the test environment with production behavior, you must explicitly set the throw configuration to true each time. Forgetting to do so can lead to issues, including false positives in tests.

With this update, the Storage faker will now respect the configuration to set the throw value, though it can still be manually overridden.

Behavior Before the Change

  • throw always defaulted to false, regardless of the configuration.
  • throw could be set to true via the $config parameter.

Behavior After the Change

  • throw defaults to false if not specified in the configuration.
  • If present, the throw setting is retrieved from the configuration.
  • throw can still be explicitly set to true via the $config parameter.

@taylorotwell
Copy link
Member

Please update the PR to include the proper docblocks as used throughout the rest of the framework. Please mark as ready for review when the requested changes have been made. Thanks!

@taylorotwell taylorotwell marked this pull request as draft December 6, 2024 18:52
@emulgeator emulgeator marked this pull request as ready for review December 7, 2024 08:55
@taylorotwell taylorotwell merged commit a53d179 into laravel:11.x Dec 10, 2024
38 checks passed
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