-
Notifications
You must be signed in to change notification settings - Fork 11.5k
[12.x] Allow globally disabling Factory parent relationships via Factory::dontExpandRelationshipsByDefault()
#56154
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
Conversation
Would a descriptive method be more Laravel-y? UserPermissionFactory::disableExpandedRelationships() |
This gave me an idea. What if we could declare the methods to call when invoking a given Factory inside the test? Something like this: #[UserFactory('withoutParents', 'someOtherThing')]
class TestSomething extends TestCase
{
// ...
} Then, instead of replicating all calls in each test, only call the method on top of the class and call it a day. My guess is that the Test Case would put these methods on a static array that would be called each time the Factory is instantiated, and flushed once the test case ends. |
That's an interesting idea, though to me it feels like a pretty limited use-case 🤷 What other factory methods do you think a user might set via this attribute? And would it require userland to create an attribute for each factory? Maybe I would prefer to see a way to setup the database once for all tests in a test case. That to me seems like a much more common problem, which would improve test run time. Take for instance all of my test cases inside of a single class require a user, an organization, a user permission for that organization, and a team. These have to be built for each test method, and then are torn down at the end. Would love if there was a way to build that data once for all tests, run each test so they can leverage those common values, and then remove it once we're done with all tests in the class. This is a totally separate issue to the one I'm proposing here, but just thinking that this would get some serious mileage in the codebase I am working on. I don't fully understand how parallel testing runs via paratest, so maybe this is a non-starter based on that. 🤔 |
You're right. I had that thought, but came across these issues:
I'll wait to see what Taylor's thoughts are on this. Happy to make whatever changes are necessary. |
I would do something like |
@taylorotwell do you mean a static function with that name? |
bf839be
to
4291fc6
Compare
Factory:: dontExpandRelationshipsByDefault()
Factory:: dontExpandRelationshipsByDefault()
Factory::dontExpandRelationshipsByDefault()
…tory::dontExpandRelationshipsByDefault()` (laravel#56154) * default expanding relationships static property * flush * flush in test * static method * formatting --------- Co-authored-by: Taylor Otwell <taylor@laravel.com>
This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [laravel/framework](https://laravel.com) ([source](https://github.com/laravel/framework)) | `12.20.0` -> `12.21.0` | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | --- ### Release Notes <details> <summary>laravel/framework (laravel/framework)</summary> ### [`v12.21.0`](https://github.com/laravel/framework/blob/HEAD/CHANGELOG.md#v12210---2025-07-22) [Compare Source](laravel/framework@v12.20.0...v12.21.0) - fix(vite): [#​55793](laravel/framework#55793) add explicit as-script to link tag for script modul… by [@​midsonlajeanty](https://github.com/midsonlajeanty) in laravel/framework#55794 - \[12.x] Allow globally disabling Factory parent relationships via `Factory::dontExpandRelationshipsByDefault()` by [@​cosmastech](https://github.com/cosmastech) in laravel/framework#56154 - \[12.x] Adds checking if a value is between two columns by [@​DarkGhostHunter](https://github.com/DarkGhostHunter) in laravel/framework#56119 - \[12.x] Ensure database connection is always restored by [@​xurshudyan](https://github.com/xurshudyan) in laravel/framework#56258 - \[12.x] Fix handling of `Htmlable` objects in `Js::convertDataToJavaScriptExpression()` by [@​jj15asmr](https://github.com/jj15asmr) in laravel/framework#56253 - Reduce meaningless intermediate variables. by [@​LjjGit](https://github.com/LjjGit) in laravel/framework#56265 - \[12.x] Improve typehints for `AbstractCursorPaginator@through()` by [@​cosmastech](https://github.com/cosmastech) in laravel/framework#56267 - Use `Date` facade instead of `time()` for `password_confirmed_at` check by [@​dylanbr](https://github.com/dylanbr) in laravel/framework#56270 - \[12.x] fix: Collection::transform() and Paginator::through() return types by [@​calebdw](https://github.com/calebdw) in laravel/framework#56273 - \[12.x] Merge 11.x into 12.x by [@​u01jmg3](https://github.com/u01jmg3) in laravel/framework#56289 - \[12.x] Reduce meaningless intermediate variables by [@​AhmedAlaa4611](https://github.com/AhmedAlaa4611) in laravel/framework#56288 - \[12.x] Refactor build Method to Use Null Coalescing Assignment for Default C… by [@​Ashot1995](https://github.com/Ashot1995) in laravel/framework#56283 - \[12.x] minor code formatting improvements by [@​browner12](https://github.com/browner12) in laravel/framework#56296 - \[12.x] Use more specific route binding exception message for child routes by [@​jessekoerhuis](https://github.com/jessekoerhuis) in laravel/framework#56298 - \[12.x] Fix Possible Undefined Variables by [@​calfc](https://github.com/calfc) in laravel/framework#56292 - \[12.x] Fix: Ensure scheduler `dailyAt()` method parses minutes and ignores seconds when seconds are provided by [@​amirhshokri](https://github.com/amirhshokri) in laravel/framework#56308 - \[12.x] Allows for strict boolean validation by [@​peterfox](https://github.com/peterfox) in laravel/framework#56313 - Improve `SeedCommand` console output by [@​Jehong-Ahn](https://github.com/Jehong-Ahn) in laravel/framework#56310 - \[12.x] Add unified enum support across framework docs by [@​amirhshokri](https://github.com/amirhshokri) in laravel/framework#56271 - \[12.x] Allows for strict numeric validation by [@​peterfox](https://github.com/peterfox) in laravel/framework#56328 - \[12.x] Update PHPDoc annotations in `Validation` by [@​mrvipchien](https://github.com/mrvipchien) in laravel/framework#56321 - \[12.x] Add operator class support for PostgreSQL GiST spatial indexes by [@​joteejotee](https://github.com/joteejotee) in laravel/framework#56324 - Fix multipart array value parsing in HTTP client ([#​55732](laravel/framework#55732)) by [@​joteejotee](https://github.com/joteejotee) in laravel/framework#56302 - Fixes bug with ShouldBeUniqueUntilProcessing locks getting stuck due to Middleware by [@​TWithers](https://github.com/TWithers) in laravel/framework#56318 - \[12.x] add prompts based expectations to PendingCommand by [@​BinaryKitten](https://github.com/BinaryKitten) in laravel/framework#56260 - \[12.x] Add Singleton and Scoped attributes to Container by [@​riasvdv](https://github.com/riasvdv) in laravel/framework#56334 - Fix unsetting model castable attribute when cast to object ([#​56335](laravel/framework#56335)) by [@​guram-vashakidze](https://github.com/guram-vashakidze) in laravel/framework#56343 - \[12.x] Fix/memory improvement by [@​CharrafiMed](https://github.com/CharrafiMed) in laravel/framework#56345 - \[12.x] Add hasMailer method to the mailable class by [@​kevinb1989](https://github.com/kevinb1989) in laravel/framework#56340 - \[12.x] Consistent use of `mb_split()` to split strings into words by [@​shaedrich](https://github.com/shaedrich) in laravel/framework#56338 - \[12.x] Add toStringable to Uri by [@​Kyrch](https://github.com/Kyrch) in laravel/framework#56359 - \[12.x] Fix PHPStan Integrations by [@​crynobone](https://github.com/crynobone) in laravel/framework#56369 - Add 'isEmpty' and 'isNotEmpty' to Fluent by [@​cworreschk](https://github.com/cworreschk) in laravel/framework#56370 - \[12.x] Add mergeMetadata method to the Mailable class by [@​kevinb1989](https://github.com/kevinb1989) in laravel/framework#56376 - Add 'dontReportUsing' to filter exceptions to be reported by [@​pelmered](https://github.com/pelmered) in laravel/framework#56361 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0MS40Mi4zIiwidXBkYXRlZEluVmVyIjoiNDEuNDMuMCIsInRhcmdldEJyYW5jaCI6ImRldmVsb3AiLCJsYWJlbHMiOlsiZGVwZW5kZW5jaWVzIl19--> Reviewed-on: https://code.itinerare.net/itinerare/Mundialis/pulls/282 Co-authored-by: Amadeus[bot] <renovate@itinerare.net> Co-committed-by: Amadeus[bot] <renovate@itinerare.net>
A continuation of #53450. This feature was added by @browner12 and it's been incredibly helpful for our team in speeding up tests.
That said, it requires us to write
->withoutParents()
on each factory make, which has led to bugs. Namely, we don't add the trait to refresh the database and then data gets left over between tests, which breaks expectations in other tests... but we don't discover it for a month 😆 😭 and then it's debug city.It would be great if in the test's setup, we can just indicate no factory should create a parent relationship.
Even better, I would just add this to the class's
setUp()
method.