Skip to content

[PhpUnitBridge] Don't use die() in PHPT --SKIPIF--#58680

Merged
xabbuh merged 1 commit intosymfony:7.2from
staabm:skip
Oct 29, 2024
Merged

[PhpUnitBridge] Don't use die() in PHPT --SKIPIF--#58680
xabbuh merged 1 commit intosymfony:7.2from
staabm:skip

Conversation

@staabm
Copy link
Copy Markdown
Contributor

@staabm staabm commented Oct 27, 2024

Q A
Branch? 7.2
Bug fix? no
New feature? no
Deprecations? no
Issues --
License MIT

Unlocks a performance optimization in PHPUnit 11.5.x

PHPUnit 11.5.x will be able to avoid subprocess creation, when --SKIPIF-- code in PHPT tests does not exit() or die() and the logic is side-effect free (output is allowed).

more details in https://staabm.github.io/2024/10/19/phpunit-codesprint-munich.html

@carsonbot
Copy link
Copy Markdown

Hey!

To help keep things organized, we don't allow "Draft" pull requests. Could you please click the "ready for review" button or close this PR and open a new one when you are done?

Note that a pull request does not have to be "perfect" or "ready for merge" when you first open it. We just want it to be ready for a first review.

Cheers!

Carsonbot

@staabm
Copy link
Copy Markdown
Contributor Author

staabm commented Oct 27, 2024

//cc @xabbuh

@staabm staabm marked this pull request as ready for review October 27, 2024 06:57
@staabm staabm requested a review from chalasr as a code owner October 27, 2024 06:57
@carsonbot
Copy link
Copy Markdown

Hey!

Thanks for your PR. You are targeting branch "7.2" but it seems your PR description refers to branch "7.2 for features / 5.4, 6.4, and 7.1 for bug fixes".
Could you update the PR description or change target branch? This helps core maintainers a lot.

Cheers!

Carsonbot

@carsonbot carsonbot added this to the 7.2 milestone Oct 27, 2024
@carsonbot carsonbot changed the title Don't use die() in PHPT --SKIPIF-- [PhpUnitBridge] Don't use die() in PHPT --SKIPIF-- Oct 27, 2024
@xabbuh
Copy link
Copy Markdown
Member

xabbuh commented Oct 28, 2024

@staabm FYI, with these changes our phpt tests are about 9% faster than without them

@staabm
Copy link
Copy Markdown
Contributor Author

staabm commented Oct 28, 2024

Awesome, thanks for reporting back.

@xabbuh
Copy link
Copy Markdown
Member

xabbuh commented Oct 29, 2024

Thank you @staabm.

@xabbuh xabbuh merged commit 9b0ca99 into symfony:7.2 Oct 29, 2024
@staabm staabm deleted the skip branch October 29, 2024 10:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants