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

Fix inheritance when using SetUpTearDownTrait #33077

Closed
wants to merge 1 commit into from

Conversation

@alcaeus
Copy link
Contributor

commented Aug 9, 2019

Q A
Branch? 4.4
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets -
License MIT
Doc PR -

When using the SetUpTearDownTrait polyfill, the various setUp and tearDown methods don't call their parents, which makes it impossible to use the trait when having test cases inherit from each other and each adding their own functionality in setUp or tearDown.

I'm aware that this makes the trait unusable when used outside of a PHPUnit\Framework\TestCase class, but then I'm not aware where else this specific polyfill should be used. 🤷‍♂️

@nicolas-grekas nicolas-grekas added this to the next milestone Aug 9, 2019

@@ -30,13 +32,17 @@ public static function setUpBeforeClass()
public static function tearDownAfterClass()
{
self::doTearDownAfterClass();
parent::doTearDownAfterClass();

This comment has been minimized.

Copy link
@nicolas-grekas

nicolas-grekas Aug 9, 2019

Member

not sure: it's the responsibility of self::doTearDownAfterClass (reimplemened in the using class) to call parent::tearDownAfterClass instead

This comment has been minimized.

Copy link
@alcaeus

alcaeus Aug 9, 2019

Author Contributor

For one, I messed up the parent calls in a couple of places by calling the do... methods.

Also, after playing around with different setups, I'm not sure this is needed after all; I may just have done something wrong. Let me re-investigate and report back.

This comment has been minimized.

Copy link
@nicolas-grekas

nicolas-grekas Aug 9, 2019

Member

I think this PR is not needed yes, that's the point of my comment :)

@alcaeus

This comment has been minimized.

Copy link
Contributor Author

commented Aug 9, 2019

Yeah, classic case of "you're using it wrong", sorry for the noise. The next contribution will be more meaningful 😉

@alcaeus alcaeus closed this Aug 9, 2019

@alcaeus alcaeus deleted the alcaeus:fix-setup-teardown-inheritance branch Aug 9, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.