Skip to content

Also destruct TestCase objects early that use a data provider#5875

Closed
talkinnl wants to merge 1 commit intosebastianbergmann:10.5from
talkinnl:10.5
Closed

Also destruct TestCase objects early that use a data provider#5875
talkinnl wants to merge 1 commit intosebastianbergmann:10.5from
talkinnl:10.5

Conversation

@talkinnl
Copy link
Copy Markdown

@talkinnl talkinnl commented Jun 20, 2024

Refactor unsetting tests during TestSuite::run, faster destruction.

  • Continuing on the destruction fix: The iterator also holds the array of tests. For tests with dataProviders, this results in not immediately destructing each Test, but only when the deeper TestSuite is done.
  • Refactored cleanup tricks: Just do a first loop so we no longer need properties nor the iterator, and then array_shift as we go.
  • This also reduces amount of code executed: One loop up front instead of the unset loop per test.
  • Added test coverage.

Refactor unsetting tests during TestSuite::run, for even faster destr…
054166c 
…uction of instances.

- Continuing on the destruction fix: The iterator also holds the array of tests. For tests with dataProviders, this results in not immediately destructing each Test, but only when the deeper TestSuite is done.
- Refactored cleanup tricks: Just do a first loop so we no longer need properties nor the iterator, and then array_shift as we go.
- Added test coverage.
@talkinnl talkinnl changed the base branch from main to 10.5 June 20, 2024 06:36
@sebastianbergmann sebastianbergmann added feature/test-runner CLI test runner type/performance Issues related to resource consumption (time and memory) labels Jun 20, 2024
@sebastianbergmann
Copy link
Copy Markdown
Owner

sebastianbergmann commented Jun 20, 2024

Thank you, Maarten.

I guess this also solves @mvorisek's issue with TestCase object destruction expressed in #5867 (comment).

@talkinnl
Copy link
Copy Markdown
Author

talkinnl commented Jun 20, 2024

In #4705 (comment) , @mvorisek noted that his issue wasn't fixed yet.
The situation was quite improved by #5861 , but I discovered the timing is still not optimal if dataProviders are used.

Seeing the order of events is easy, just add a __destruct() to TestCase which outputs a 'Z':
AFTER PR 5861:

./phpunit
PHPUnit 10.5.22 by Sebastian Bergmann and contributors.

Runtime:       PHP 8.2.18
Configuration: /Users/talkinnl/checkouts/phpunit/phpunit.xml

..ZZ....ZZZZ....ZZZZ......................................ZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZ...ZZZ..ZZ..ZZ.Z.Z..ZZ..   61 / 3493 (  1%)
ZZ..ZZ..ZZ..ZZ..ZZ..ZZ..ZZ..ZZ..ZZ..ZZ..ZZ..ZZ..ZZ..ZZ....ZZZZ..ZZ...ZZZ....ZZZZ...ZZZ...ZZZ.Z.Z.Z...ZZZ..ZZ..ZZ..ZZ..ZZ..  122 / 3493 (  3%)
ZZ..ZZ..ZZ...ZZZ..ZZ..ZZ..ZZ..ZZ..ZZ..ZZ..ZZ..ZZ..ZZ..ZZ..ZZ..ZZ..ZZ..ZZ..ZZ..ZZ..ZZ..ZZ..ZZ..ZZ..ZZ..ZZ..ZZ..ZZ..ZZ..ZZ..  183 / 3493 (  5%)

(Before that change, all Zs would be at the very end)

WITH THIS PR 5875:

./phpunit
PHPUnit 10.5.22 by Sebastian Bergmann and contributors.

Runtime:       PHP 8.2.18
Configuration: /Users/talkinnl/checkouts/phpunit/phpunit.xml

.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.   61 / 3493 (  1%)
Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.  122 / 3493 (  3%)
Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.Z.  183 / 3493 (  5%)

So a nice pattern between dots and Zs. :)

@talkinnl
Copy link
Copy Markdown
Author

talkinnl commented Jun 20, 2024

phpunit own test suite:

Time: 00:52.337, Memory: 42.00 MB
Tests: 3493, Assertions: 9540, Skipped: 10.

TO

Time: 00:51.999, Memory: 40.00 MB
Tests: 3493, Assertions: 9540, Skipped: 10.

And then I added the test, so tests and assertions did increase. So no regressions, and a change from 42 to 40MB.

@sebastianbergmann sebastianbergmann changed the title Improved Test destruction, fixed last delays and dataProvider behavior. Also destruct TestCase objects early that use a data provider Jun 20, 2024
@sebastianbergmann
Copy link
Copy Markdown
Owner

sebastianbergmann commented Jun 20, 2024

Merged manually.

@sebastianbergmann
Copy link
Copy Markdown
Owner

sebastianbergmann commented Jun 20, 2024

@talkinnl FYI: I changed your test case into an end-to-end test (67323d2) so that the execution order of its tests is not affected by randomness.

@mvorisek
Copy link
Copy Markdown
Contributor

mvorisek commented Jun 20, 2024

This is perfect and thank you @talkinnl!

This was referenced Jun 20, 2024
Closed
Merged
Closed
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Apr 17, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

feature/test-runner CLI test runner type/performance Issues related to resource consumption (time and memory)

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@talkinnl @sebastianbergmann @mvorisek