[Bug] Always call parent teardown even if an exception is thrown #1129
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What:
Description:
This PR attempts to fix the issue outlined in #988.
Essentially, the core issue is that if an exception occurs when calling
->__callClosure
,parent::tearDown
is never called. This can cause issues in Test Suites that rely on performing certain tasks in tearDown. For example theRefreshDatabase
trait in Laravel relies on this in order to rollback the current transaction.The issue i was running into specifically was that my tests after the failing one were appearing to hang. But actually they were just waiting to get a lock on a table, this ended up never resolving and just exceeding my databases lock_wait_timeout. This (usually passing) test would then fail, before moving onto the next one, looping round again. The root cause appeared to be because Laravel couldn't cleanup the database transaction from the failing test as tearDown was never called due to a Mock exception.
I've added a try/finally block around the call to
->callClosure
to ensure teardown is always called. We should have a test case for this situation to ensure we don't have a regression in the future, however i was unsure on the approach that should be taken here as i'm not familiar with all the internals at play here.I've modified
Testable.php
locally on my project and my issue has gone away. The failing test correctly cleans up after itself (still reporting the fail, of course) before moving onto subsequent tests without having the lock issue.Related:
Fixes #988.
Fixes #533