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

Make the library phpunit 10 compatible #15

Merged
merged 10 commits into from
Nov 29, 2023

Conversation

MarijnKoesen
Copy link
Contributor

Since phpunit 10.4 the toString() method is called when sending out events. This causes any test to fail with the 'NotImplementedException', so it needs to be implemented now.

It would be nice if we can tag this as version 3.

Here is a little snippet of the stacktrace with the new phpunit where you can see that the event trigger is causing the toString() to be called:

#0 /myproject/vendor/phpunit/phpunit/src/Event/Emitter/DispatchingEmitter.php(487): Asynchronicity\PHPUnit\Eventually->toString(false)
#1 /myproject/vendor/phpunit/phpunit/src/Framework/Assert.php(1881): PHPUnit\Event\DispatchingEmitter->testAssertionSucceeded(Object(Closure), Object(Asynchronicity\PHPUnit\Eventually), '')
#2 /myproject/vendor/matthiasnoback/phpunit-asynchronicity/src/Asynchronicity/PHPUnit/Asynchronicity.php(20): PHPUnit\Framework\Assert::assertThat(Object(Closure), Object(Asynchronicity\PHPUnit\Eventually))
#3 /myproject/tests/MyTest.php(228): MyProject\MyTest::assertEventually(Object(Closure), 10000)

Since phpunit 10.4 the toString method is called when sending out
events, so it needs to be implemented now.
@matthiasnoback
Copy link
Owner

Thanks, looks good. The CI still needs fixing: https://github.com/matthiasnoback/phpunit-asynchronicity/blob/master/.github/workflows/code_analysis.yaml Can you look into that?

Since phpunit 10.4 the toString method is called when sending out
events, so it needs to be implemented now.
@MarijnKoesen
Copy link
Contributor Author

Looks like tuffz has already beaten me to it: #17 :)

@matthiasnoback
Copy link
Owner

Okay, shall we go with that PR then?

@MarijnKoesen
Copy link
Contributor Author

I'm actually fixing the phpstan warnings that were ignored there, give me a sec, than we can merge it a cleaner state 👍

@MarijnKoesen
Copy link
Contributor Author

MarijnKoesen commented Nov 29, 2023

It should be all green now. I've included the fixes from stuffz and my additional ones to resolve the new warnings :)

As I can't run the workflow in this repo I'm not 100% sure, but it's all green on my machine 😅

@matthiasnoback matthiasnoback merged commit ae4f6ac into matthiasnoback:master Nov 29, 2023
@matthiasnoback
Copy link
Owner

Thanks, released as v3.0.0

@MarijnKoesen
Copy link
Contributor Author

Thanks! 🥳

@tuffz
Copy link
Contributor

tuffz commented Nov 29, 2023

Great teamwork 🤝

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants