-
-
Notifications
You must be signed in to change notification settings - Fork 109
feat(2.9): deprecate Factories trait and force PHPUnit extension usage
#968
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
feat(2.9): deprecate Factories trait and force PHPUnit extension usage
#968
Conversation
15eba47 to
a786b04
Compare
91d0ab5 to
876b505
Compare
b2d669d to
fec86ed
Compare
Factories trait and force PHPUnit extension usage
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR implements the deprecation of the Factories trait in favor of forcing PHPUnit extension usage. The purpose is to make Foundry's PHPUnit integration more robust by eliminating the possibility of forgetting to use the trait, which currently leads to errors.
- Deprecates the
Factoriestrait and shows deprecation warnings when used - Introduces new PHPUnit extension functionality to handle test lifecycle management
- Adds comprehensive test coverage for both trait and extension usage scenarios
Reviewed Changes
Copilot reviewed 33 out of 33 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Test/Factories.php | Adds deprecation warnings and early returns when PHPUnit extension is enabled |
| src/PHPUnit/FoundryExtension.php | Extends extension functionality with new subscribers and state tracking |
| src/PHPUnit/BootFoundryOnPreparationStarted.php | New subscriber to boot Foundry when test preparation starts |
| src/PHPUnit/ShutdownFoundryOnTestFinished.php | New subscriber to shutdown Foundry when test finishes |
| src/PHPUnit/DataProvider/*.php | New data provider handling subscribers for extension-based workflow |
| src/Persistence/PersistentObjectFromDataProviderRegistry.php | New registry for managing persistent objects from data providers |
| tests/Integration/ForceFactoriesTraitUsage/ | New test files to verify both trait and extension behaviors |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
e2bb951 to
bb8a6b1
Compare
Factories trait and force PHPUnit extension usageFactories trait and force PHPUnit extension usage
11e17e4 to
a9dd8cf
Compare
cccf9b7 to
f216d8b
Compare
12f9f16 to
8408b91
Compare
3302a9e to
5ec618e
Compare
08da3d3 to
5b85eac
Compare
8a39e3a to
684c814
Compare
f48643f to
1e7f5fd
Compare
| $reflector = new \ReflectionClass($factory::class()); | ||
|
|
||
| return $reflector->newLazyProxy(static function() use ($factory) { | ||
| return (new \ReflectionClass($factory::class()))->newLazyGhost(static function(object $ghost) use ($factory): void { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we must use "lazy ghost" objects, otherwise we'd have identity errors (see DataProviderWithPersistentFactoryTestCase::assert_it_provided_instance_is_the_same_than_returned_by_repository() which would not pass otherwise
| return $factory->create(); | ||
| $createdObject = $factory->withoutPersisting()->create(); | ||
|
|
||
| Hydrator::hydrateFromOtherObject($ghost, $createdObject); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this would break with readonly properties, this can be fixed using ReflectionProperty::setRawValueWithoutLazyInitialization(). Will be done in another PR
| if ($factory->isPersisting()) { | ||
| Configuration::instance()->persistence()->save($ghost); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this means after persist callbacks are not used, this will also be fixed a further PR
1e7f5fd to
5cabf6c
Compare
|
OK then, I think I have found the solution I was looking for! I've prepared a branch |
Removes the usage of
Factoriestrait in favor of the PHPUnit extension.This will be really less error prone, since it would be impossible to omit it, and it would be added once for all
fixes #965 (because we won't check anymore if the trait is used)
I had hard times (like REALLY hard times) to make the feature "use
PersistentObjectFactory::create()method in data providers" work without the trait. The code is kinda hacky, but since we cannot access the test instance from the PHPUnit extension (even with some hacky code, this is literally impossible), I had to be creative 😅