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 the RefreshDatabase 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.
What:
Description:
This PR attempts to fix the issue outlined in https://github.com/pestphp/pest/issues/988.
Essentially, the core issue is that if an exception occurs when calling
->__callClosure,parent::tearDownis never called. This can cause issues in Test Suites that rely on performing certain tasks in tearDown. For example theRefreshDatabasetrait 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
->callClosureto 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.phplocally 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 https://github.com/pestphp/pest/issues/988. Fixes https://github.com/pestphp/pest/issues/533