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

fix(error): avoid phpunit crash by triggering side-effect via count on testcase #1577

Merged
merged 2 commits into from
Jun 17, 2024

Conversation

simPod
Copy link
Collaborator

@simPod simPod commented Jun 17, 2024

@mfn this is a followup of sebastianbergmann/phpunit#5866 (comment)

What do you think about the fix? I don't see any other option since we analyze the whole stacktrace.

@simPod simPod requested a review from spawnia June 17, 2024 06:59
@mfn
Copy link
Contributor

mfn commented Jun 17, 2024

I get it but…

It just feels wrong from phpunit: they don't uphold the \Countable contract.

Then some other project also does this and we would add another exception case?

I feel like we should re-open the phpunit issue, as I don't have the feeling this has been fully discussed.

@mfn
Copy link
Contributor

mfn commented Jun 17, 2024

… and tests are failing anyway 😬

@simPod
Copy link
Collaborator Author

simPod commented Jun 17, 2024

As I know Sebastian he won't discuss it anymore :D He does not want any 3rd party to mess with phpunit internals.

Tests are fixed.

Co-authored-by: Benedikt Franke <benedikt@franke.tech>
@spawnia spawnia added the bug label Jun 17, 2024
@spawnia spawnia merged commit a7532a2 into webonyx:master Jun 17, 2024
15 checks passed
@spawnia
Copy link
Collaborator

spawnia commented Jun 17, 2024

@simPod simPod deleted the fix-phpunit branch June 17, 2024 09:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants