laminas / laminas-di

Automated dependency injection for PSR-11 containers
https://docs.laminas.dev/laminas-di/
BSD 3-Clause "New" or "Revised" License
36 stars 20 forks source link

Drop PHP 7 support, provide PHP 8.2 compatibility #82

Closed AshishKumarPundeer closed 1 year ago

AshishKumarPundeer commented 1 year ago
Q A
Documentation no
Bugfix no
BC Break no
New Feature no
RFC no
QA yes
Ocramius commented 1 year ago

Hmm, we probably need to rewrite the tests that rely on phpspec/prophecy to use only PHPUnit mocking

Ocramius commented 1 year ago

@AshishKumarPundeer TODOs:

  1. a run of vendor/bin/phpcbf
  2. removal of ProphecyTrait from tests
AshishKumarPundeer commented 1 year ago

@Ocramius Suggested changes has beed done.

Ocramius commented 1 year ago

@AshishKumarPundeer please look at the CI failures 😁

AshishKumarPundeer commented 1 year ago

Hello @Ocramius Our PR is dependent on package phpspec/prophecy and they have added the support php 8.2 under this PR https://github.com/phpspec/prophecy/pull/571 which is not yet merged hence it seems once PR will be merged our PR will be green.

Please suggest way forward anything is required our end.

Thanks

Ocramius commented 1 year ago

Hey @AshishKumarPundeer, I already suggested to drop prophecy here, and use PHPUnit mocks instead.

AshishKumarPundeer commented 1 year ago

Hello @Ocramius ,

Hey @AshishKumarPundeer, I already suggested to drop prophecy here, and use PHPUnit mocks instead.

It may be the silly question for you just for clarification can we use this phpspec/prophecy-phpunit or phpunit only.

Ocramius commented 1 year ago

Let's please phase out prophecy overall: PHPUnit only 👍

AshishKumarPundeer commented 1 year ago

Let's please phase out prophecy overall: PHPUnit only 👍

We have already removed ProphecyTrait in tests file,Can you please suggest in which files we need to remove the prophecy now.

Ocramius commented 1 year ago

Effectively, ->prophesize() calls need to be replaced with ->createMock() (and associated API) in the test classes :)

AshishKumarPundeer commented 1 year ago

Effectively, ->prophesize() calls need to be replaced with ->createMock() (and associated API) in the test classes :)

we have used createMock() in place of prophesize() but below are some interfaces in which we are facing the issues. use Laminas\Di\InjectorInterface; use Psr\Log\LoggerInterface use Psr\Container\ContainerInterface;

eg. $container = $this->createMock(ContainerInterface::class)->reveal();

we have tried various things but it is not resolved yet please suggest what we should do for this.

Thanks

gsteel commented 1 year ago

(and associated API)

@AshishKumarPundeer you may find it helpful to read the chapter on PHPUnit mocks here. reveal() is part of the Prophecy api. There are some similarities between the mocking api of PHPUnit and prophecy, but you must review each mock individually and ensure that all of the expectations match the current test suite.

samsonasik commented 1 year ago

@AshishKumarPundeer instead of randomly push and wait CI to run test, you can verify locally first and make a commit only when all ok on your local dev:

./vendor/bin/phpcs -q --report=checkstyle
./vendor/bin/phpunit
glo82145 commented 1 year ago

@Ocramius @samsonasik All testcases are passing now, please review and merge the PR