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

Remove Test\AssertionFailed and Test\AssertionSucceeded events #5604

Merged
merged 1 commit into from
Dec 12, 2023

Conversation

sebastianbergmann
Copy link
Owner

When we introduced the event system with PHPUnit 10, we thought it would be nice to emit events, Test\AssertionFailed and Test\AssertionSucceeded, after an assertion is performed.

While PHPUnit itself does not need these events, it does not subscribe to them itself, they have been a constant source of problems: #5183, #5184, #5261, #5510, #5515, #5524, #5539.

The workarounds that were implemented do not seem to always yield the desired effect and, moreover, make the event system's implementation more complicated than it should be.

I therefore propose to remove these events ASAP, in PHPUnit 11. As far as I know, none of the widely used extensions for PHPUnit subscribe to these events, so this removal should not be a problem.

@sebastianbergmann sebastianbergmann added type/backward-compatibility Something will be/is intentionally broken type/performance Issues related to resource consumption (time and memory) labels Dec 8, 2023
@sebastianbergmann sebastianbergmann added this to the PHPUnit 11.0 milestone Dec 8, 2023
Copy link

codecov bot commented Dec 8, 2023

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (6efa643) 88.52% compared to head (1a59978) 88.46%.

Files Patch % Lines
src/Event/Facade.php 0.00% 1 Missing ⚠️
src/Framework/Constraint/Constraint.php 50.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main    #5604      +/-   ##
============================================
- Coverage     88.52%   88.46%   -0.07%     
+ Complexity     6293     6258      -35     
============================================
  Files           671      668       -3     
  Lines         20170    20047     -123     
============================================
- Hits          17855    17734     -121     
+ Misses         2315     2313       -2     

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

* Remove Extension\Facade::requireExportOfObjects()
@sebastianbergmann sebastianbergmann merged commit 6fa53c3 into main Dec 12, 2023
47 of 48 checks passed
@sebastianbergmann sebastianbergmann deleted the remove-assertion-events branch December 12, 2023 12:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/backward-compatibility Something will be/is intentionally broken type/performance Issues related to resource consumption (time and memory)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant