Destroy TestCase object after its test was run
#5861
Conversation
sebastianbergmann
added
the
type/performance
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5861 +/- ##
============================================
- Coverage 92.34% 92.33% -0.02%
- Complexity 6554 6559 +5
============================================
Files 699 699
Lines 19772 19781 +9
============================================
+ Hits 18259 18265 +6
- Misses 1513 1516 +3 ☔ View full report in Codecov by Sentry. |
|
The groups property still also holds the test instances, so tests which are part of group(s) will still not be destructed in this first commit. |
I am aware of that, but thank you for pointing it out. |
sebastianbergmann
force-pushed
the
destroy-testcase-objects-after-execution
branch
from
f11b51a
to
4df94e0
Compare
|
With TYPO3's (unit) test suite ( |
|
This change can be applied to PHPUnit 10.5 and PHPUnit 11.2: |
sebastianbergmann
added
version/10
|
@sebastianbergmann I've tested it on a (small) Laravel project containing 102 tests (mainly feature, few unit). Here are the results: Before ( After ( I'll try it later today on some other projects which largers test suites. |
|
I did a full successful run on our project. I started before your second commit, and I actually just did a $this->groups=[]; once before the loop to clear the groups. If we’re done making the tests selection, might just as well clear that property at once instead of slowly. :) Patched v10. Note: Mosts tests already had a tearDown() for unsetting properties, and we unset all our own properties using reflection because we kept forgetting tear downs. exec summary: I’m quite hyped by this proposal, and only positive changes with a huge, old project! |
|
Just tested this in a project with three test suites: Unit tests, DB integration tests and REST API E2E tests. Unit testsBefore: After: Integration testsBefore: After: API testsBefore: After: Everything worked transparently. Memory saving is more obvious the more tests the test suite has (which I guess is expected). |
The "base" needs to be the same, the only difference must be the changes from this PR. In other words: Testing PHPUnit 11.1 without these changes versus PHPUnit 11.3-dev with these changes does not help. Sorry! |
Yes, that is expected. Thanks! |
Good idea, thanks! |
Does not work, though. With these changes ... diff --git a/src/Framework/TestSuite.php b/src/Framework/TestSuite.php
index c663aec88..6492c1207 100644
--- a/src/Framework/TestSuite.php
+++ b/src/Framework/TestSuite.php
@@ -359,6 +359,8 @@ public function run(): void
return;
}
+ $this->groups = [];
+
foreach ($this as $test) {
if (TestResultFacade::shouldStop()) {
$emitter->testRunnerExecutionAborted();
@@ -375,22 +377,6 @@ public function run(): void
break;
}
}
-
- if ($test instanceof TestCase || $test instanceof self) {
- foreach ($test->groups() as $group) {
- if (!isset($this->groups[$group])) {
- continue;
- }
-
- foreach (array_keys($this->groups[$group]) as $key) {
- if ($test === $this->groups[$group][$key]) {
- unset($this->groups[$group][$key]);
-
- break;
- }
- }
- }
- }
}
$this->invokeMethodsAfterLastTest($emitter);... the tests for group-based filtering of PHPUnit's own test suite fail because the filters are applied while iterating for execution. I am afraid that this cannot be addressed without making more significant changes which are outside the scope of this short-term solution. |
|
Ah, pity. Didn’t knew the filtering was during the loop. |
As of 52d2488, the |
Don't feel sorry 😊 I've checked it again, but I was using the correct PHPUnit changes. FYI: I've updated Output for memory usage was the same after a second run. |
|
For one of my personal projects: After: |
|
Works fine, no issues. |
⬆️ The above was with xdebug & code coverage. Without it: |
|
Merged into |
|
@sebastianbergmann This PR means #4705 is fixed :) |
|
Late feedback, but feedback: This works very well: I confirm the numbers with TYPO3 core v13 (phpunit v11) unit test suite, "functional" test suite is even better, and core v12 (phpunit v10) is confirmed to work and shows similar numbers. |
|
TYPO3 core v13 functional test are down to 1.37 GB from > 2 GB before. |
|
In our project ecamp/ecamp3 this resulted in our tests taking ~3min instead of ~13min. 🤯 |
I may have found a short-term solution (before more architectural changes have been implemented for the test runner) to reduce PHPUnit's memory usage.
With PHPUnit's own test suite, I see a memory usage reduction from 48.00 MB (778070d) to 44.00 MB (4df94e0).
I have tested this solution with a couple of test suites so far and 1) it does not break anything (good!) and 2) it reduces memory usage (also good!).
However, I am not comfortable shipping this without (a lot) more testing. So, if you want to help me out, then please test this change with your own test suite:
This change can be applied to PHPUnit 10.5 and PHPUnit 11.2.