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

Do not run CLEAN section of PHPT test in separate process when it is free of side effects that modify the parent process #5999

Merged
merged 1 commit into from
Oct 19, 2024

Conversation

staabm
Copy link
Contributor

@staabm staabm commented Oct 18, 2024

Don't use subprocess for —CLEAN— as long as the code cannot modify the parent process (e.g. file IO is fine within the parent process).

utilizes https://github.com/staabm/side-effects-detector

followup to #5998

php phpunit tests/end-to-end/event/phpt-clean.phpt
before this PR 109-110ms
after this PR: 85-87ms

tested on PHP 8.3.12 macos m1 pro

@staabm staabm marked this pull request as ready for review October 18, 2024 15:51
Copy link

codecov bot commented Oct 18, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.78%. Comparing base (bc42422) to head (2dbb7d3).
Report is 1 commits behind head on 11.5.

Additional details and impacted files
@@            Coverage Diff            @@
##               11.5    #5999   +/-   ##
=========================================
  Coverage     94.78%   94.78%           
- Complexity     6814     6821    +7     
=========================================
  Files           720      720           
  Lines         21626    21642   +16     
=========================================
+ Hits          20498    20514   +16     
  Misses         1128     1128           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@staabm staabm force-pushed the clean-skipif branch 2 times, most recently from b6dcfa9 to 82bfe56 Compare October 18, 2024 16:10
@staabm staabm marked this pull request as draft October 18, 2024 16:17
@staabm staabm force-pushed the clean-skipif branch 6 times, most recently from 7bda895 to 787f13c Compare October 18, 2024 17:08
@staabm staabm marked this pull request as ready for review October 19, 2024 05:26
Comment on lines -27 to -28
Child Process Started
Child Process Finished
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here we can see the new feature in action. a pre-existing test no longer needs a subprocess to cleanup

@sebastianbergmann sebastianbergmann added type/enhancement A new idea that should be implemented feature/phpt End-to-end tests in PHPT format feature/test-runner CLI test runner type/performance Issues related to resource consumption (time and memory) labels Oct 19, 2024
@sebastianbergmann sebastianbergmann added this to the PHPUnit 11.5 milestone Oct 19, 2024
@sebastianbergmann sebastianbergmann merged commit 34b647e into sebastianbergmann:11.5 Oct 19, 2024
31 checks passed
@staabm staabm deleted the clean-skipif branch October 19, 2024 05:36
@sebastianbergmann sebastianbergmann changed the title Inline CLEAN evaluation without main-process pollution Do not run CLEAN section of PHPT test in separate process when it is free of side effects that modify the parent process Oct 19, 2024
@staabm
Copy link
Contributor Author

staabm commented Oct 20, 2024

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature/phpt End-to-end tests in PHPT format feature/test-runner CLI test runner type/enhancement A new idea that should be implemented type/performance Issues related to resource consumption (time and memory)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants