-
Notifications
You must be signed in to change notification settings - Fork 11.4k
[9.x] Use @before
and @after
annotations in TestCase
#39913
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
this is the sexiest thing that the laravel test suite needed. 👍 |
cc @crynobone |
In my test class I call parent after a specific setup in my own code, which is paramount to get my complex / private / company code base set up working. Pseudo-code: abstract class TestCase extends BaseTestCase
protected function setUp(): void
{
// Import stuff which **must** be done before setUp
// ...
parent::setUp();
// Now the rest
// ... How can I translate this with this PR?
TL;DR I need to exactly time when the test suite is set up and I'm unsure how to do this with this PR Thank you 🙏🏼 |
Yep, in complex cases where you need fine-grained control, you can always just implement |
@mfn Another option would be to do that work in |
I tested this on L8 and can confirm it works 👍🏼, thanks! |
What is actually the benefit of this change except for using a different/newer option in PHPUnit? the only benefit i can think of is that you could easily add multiple @before and @afters via traits. But since this is a base class and not a trait it wouldn't actually benefit from this. any other benefits? @inxilpro I do think more people override the default setup function than you'd think. |
@joelharkes The main benefit is that you don't have to remember to call The downside seems minimal. Most people extending |
Is the order in which PHPUnit executes @before, setUp, teardown, etc. explicitly documented on the PHPUnit website? If so, can you link to it? |
@taylorotwell it's not documented, but the order was discussed in a PHPUnit issue (where PHPUnit broke the existing order of things). The default order runs:
The The The other thing to consider is how This order is:
(This order is mentioned in a php.net comment, and I've independently confirmed it in PHP 7.3–8.1). This means that because It still doesn't 100% solve the order problem, but I think this, combined with #39947 would get us to a more predictable sequence of events. |
If it's not explicitly documented as a thing that will not change in the future I'm not sure we want to depend on it since we have been bitten before. 😞 |
Right now, the Laravel docs need to display this warning because the framework's base
TestCase
relies on logic in thesetUp
andtearDown
methods:This PR moves the
setUp
andtearDown
functionality into newsetUpLaravelApplication
andtearDownLaravelApplication
methods with the@before
and@after
annotations applied to them respectively.PHPUnit sets up and tears down tests in the following order (ignoring the undocumented preCondition/postCondition phases):
@before
annotation (in order: traits, base class, extending class(es))setUp()
methodtearDown()
method@after
annotation (in order: extending class(es), base class, traits)This means that the new
setUpLaravelApplication()
method is called before the test'ssetUp
method, and thetearDownLaravelApplication
is called after the test'stearDown
method.More details about inheritance order
Basically, PHPUnit determines what methods to call by using the
ReflectionClass::getMethods()
function. This function returns methods in the following order:use
statements)PHPUnit then adds these hooks to an array using
array_unshift
for@before
andarray_push
for@after
. These arrays are called in order during the test run. That means@before
hooks in the parent class are prepended last and run first, and@after
hooks in the parent class are appended last and run last.The end result is that framework users can go on using
setUp
andtearDown
exactly as they do right now, but don't have to worry about remembering to callparent::setUp()
orparent::tearDown()
(just like normal PHPUnit tests).I'm submitting this against
9.x
because it could theoretically affect someone using a method calledsetUpLaravelApplication
or intentionally trying to override the default LaravelsetUp
method (in which case the solution is just to rename their custom methodsetUpLaravelApplication
).