-
Notifications
You must be signed in to change notification settings - Fork 11.4k
[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
base: 12.x
Are you sure you want to change the base?
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()
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.