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

Add support for PHP 8.4, Upgrade PHPUnit to 10.x, remove Prophecy dependency and improve unit tests #174

Merged
merged 13 commits into from
Nov 14, 2024

Conversation

fezfez
Copy link
Contributor

@fezfez fezfez commented Nov 14, 2024

No description provided.

Copy link
Member

@gsteel gsteel left a comment

Choose a reason for hiding this comment

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

Thank you @fezfez :)

The code changes all look 👌 but can you please add to PHPUnit config failOnDeprecation along with failOn… PhpUnitDeprecation|Error|Warning|Notice and re-push - there are some deprecations in the tests that are currently going unnoticed and they may or may not be problematic

@fezfez
Copy link
Contributor Author

fezfez commented Nov 14, 2024

@gsteel done expect failOnPhpunitDeprecation because of

2 tests triggered 2 PHPUnit deprecations:

1) LaminasTest\Mvc\DispatchListenerTest::testWillNotDispatchWhenAnMvcEventResultIsAlreadySet
Data Provider method LaminasTest\Mvc\DispatchListenerTest::alreadySetMvcEventResultProvider() is not static

/var/www/test/DispatchListenerTest.php:95

2) LaminasTest\Mvc\Service\ViewHelperManagerFactoryTest::testBasePathHelperFactoryCanBeInvokedViaShortNameOrFullClassName
Data Provider method LaminasTest\Mvc\Service\ViewHelperManagerFactoryTest::basePathConfiguration() is not static

/var/www/test/Service/ViewHelperManagerFactoryTest.php:134

those two static method create a mock with $this->createMock(....) and cant be marked as static, it will take some effort to fix this

Signed-off-by: fezfez <[email protected]>
@fezfez
Copy link
Contributor Author

fezfez commented Nov 14, 2024

@gsteel fail on deprecation also fail on php 8.4 lowest job... Should a disable it too for now?

@gsteel
Copy link
Member

gsteel commented Nov 14, 2024

Fixing the non-static static data providers will have to be done at some point - can you move the mocks to the test and provide any required data for the mock in the provider?

It's also helpful to set displayDetailsOn{Deprecation|PhpUnitDepreaction|Notice|Warning} in phpunit config so that we can see in the logs what failed.

@fezfez
Copy link
Contributor Author

fezfez commented Nov 14, 2024

@gsteel this time i think it's good :)

Copy link
Member

@gsteel gsteel left a comment

Choose a reason for hiding this comment

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

Sorry @fezfez - I missed something, but other than that this is good to go. Thank you :)

Copy link
Member

@gsteel gsteel left a comment

Choose a reason for hiding this comment

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

🤦‍♂️ Forgot to add the review comment

src/ResponseSender/AbstractResponseSender.php Outdated Show resolved Hide resolved
Copy link
Member

@gsteel gsteel left a comment

Choose a reason for hiding this comment

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

Thanks again @fezfez

@gsteel gsteel changed the title Add php 8.4 support + Remove Prophecy from unit tests Add support for PHP 8.4, Upgrade PHPUnit to 10.x, remove Prophecy dependency and improve unit tests Nov 14, 2024
@gsteel gsteel self-assigned this Nov 14, 2024
@gsteel gsteel added Enhancement dependencies Pull requests that update a dependency file QA Quality assurance tasks such as static analysis improvements labels Nov 14, 2024
@gsteel gsteel added this to the 3.8.0 milestone Nov 14, 2024
@gsteel gsteel merged commit 7899bb5 into laminas:3.8.x Nov 14, 2024
15 checks passed
@gsteel gsteel mentioned this pull request Nov 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file Enhancement QA Quality assurance tasks such as static analysis improvements
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants