Codeception / module-yii2

Codeception module for Yii2 framework
MIT License
17 stars 35 forks source link

PhpBrowser::$client doesn't match it's parent InnerBrowser #58

Closed rafaelmiti closed 1 year ago

rafaelmiti commented 2 years ago

We just upgraded to PHP 8.0. We got an error when running the acceptance tests:

failure

wrong

We could fix it by removing the type of the $client attribute of the PhpBrowser class (it was changed for the annotation really):

success

right

The PR if this is the right solution: https://github.com/Codeception/module-phpbrowser/pull/22

Obs: We tried to update to the version 2.1 of the module, because we couldn't find the 2.0 branch (against which the PR maybe should be opened), but it wasn't possible. Looks like composer can't update to it. So we hope this fix will be tagged by 2.0.x

TavoNiievez commented 2 years ago

Hi @rafaelmiti,

The solution is not to remove the typing.

If you update to version 2.0.1 of codeception/lib-innerbrowser you should stop seeing this error.

Try removing your composer.lock file and run composer update -W, anyway, please run composer show after that and share here the console output.

rafaelmiti commented 2 years ago

Hi, Tavo!

Did what you say, but it remains with the innerbrowser 1.5.1:

composer show:

codeception/codeception 4.1.27 BDD-style testing framework codeception/lib-asserts 2.0.0 Assertion methods used by Codeception core and Asserts module codeception/lib-innerbrowser 1.5.1 Parent library for all Codeception framework modules and PhpBrowser codeception/module-asserts 2.0.1 Codeception module containing various assertions codeception/module-phpbrowser 2.0.2 Codeception module for testing web application over HTTP codeception/module-rest 2.0.0 REST module for Codeception codeception/module-yii2 1.1.4 Codeception module for Yii2 framework codeception/phpunit-wrapper 9.0.6 PHPUnit classes used by Codeception codeception/stub 4.0.0 Flexible Stub wrapper for PHPUnit's Mock Builder


Even with the phpbrowser requiring it to be above 2.0 (.lock created):

{ "name": "codeception/module-phpbrowser", "version": "2.0.2", "source": { "type": "git", "url": "https://github.com/Codeception/module-phpbrowser.git", "reference": "fda6d97253d0595843adb87ec0b9b7d6d89539ac" }, "dist": { "type": "zip", "url": "https://api.github.com/repos/Codeception/module-phpbrowser/zipball/fda6d97253d0595843adb87ec0b9b7d6d89539ac", "reference": "fda6d97253d0595843adb87ec0b9b7d6d89539ac", "shasum": "" }, "require": { "codeception/codeception": "^4.1 | @dev", "codeception/lib-innerbrowser": "^2.0 | *@dev", "ext-json": "", "guzzlehttp/guzzle": "^7.4", "php": "^7.4 | ^8.0" },

rafaelmiti commented 2 years ago

I even excluded the innerbrowser package directly in the vendor and ran the composer update -W, but it reinstalled the 1.5.1. I suppose the problem could be the module-rest because it requires the innerbrowser superior than 1.0:

"codeception/lib-innerbrowser": "^1.0",

What you think?

TavoNiievez commented 2 years ago

@rafaelmiti I just released version 2.0.1 of codeception/module-rest requiring version ^2.0 of codeception/lib-innerbrowser. If you do the composer.lock + composer update -W thing again this problem should be fixed this time.

Keep me posted and thank you :-)

rafaelmiti commented 2 years ago

@TavoNiievez , did it again (even deleting the innerbrowser in the vendor), the module-rest was updated to the 2.0.1 version with the requiring for innerbrowser ^2.0, but it installed the 1.5.1 again

btw, merry christmas! haha

TavoNiievez commented 2 years ago

@rafaelmiti merry christmas!

I tried to do from scratch an installation of the Codeception packages that you indicated in your comment that you are using.

composer req codeception/codeception codeception/lib-asserts codeception/lib-innerbrowser codeception/module-asserts codeception/module-phpbrowser codeception/module-rest codeception/module-yii2 codeception/phpunit-wrapper codeception/stub

The problem now seems to be that the yii2 module is not compatible with the 2.x versions of the other modules.

Your requirements could not be resolved to an installable set of packages.

  Problem 1
    - codeception/module-yii2[1.1.2, ..., 1.1.4] require codeception/lib-innerbrowser ^1.0 -> found codeception/lib-innerbrowser[1.0.0, ..., 1.5.1] but it conflicts with your root composer.json require (^2.0).
    - Root composer.json requires codeception/module-yii2 ^1.1 -> satisfiable by codeception/module-yii2[1.1.0, ..., 1.1.4].

I'm going to transfer this issue in case someone is interested in submitting a PR to add such support.

The workaround is to lock the dependencies in your composer.json to use version 1.x of the Codeception modules.

samdark commented 2 years ago

@TavoNiievez would using `"codeception/lib-innerbrowser": "^1.0|^2.0" do the job? I don't see any obvious incompatibilities in 7.4 upgrade commit.

TavoNiievez commented 2 years ago

A priori the necessary changes would be as follows:

- protected function clientRequest($method, $uri, array $parameters = [], array $files = [], array $server = [], $content = null, $changeHistory = true)
+ protected function clientRequest(string $method, string $uri,  array $parameters = [],  array $files = [], array $server = [], string $content = null, bool $changeHistory = true): \Symfony\Component\DomCrawler\Crawler

However, for this, support for any version of PHP lower than 7.4 should be removed, I commented via Slack that I wasn't sure if you would want to do this, or if you would reserve these changes for a hypothetical module-yii3.

Codeception 5 will require PHP 7.4 in addition, so you have to decide whether to keep backward compatibility with older versions of PHP or not.

SamMousa commented 2 years ago

We can just bump the module to major to 2? Since it is pretty stable and doesn't see a lot of bug reports version 2 could bump the required PHP version. If we do that, I'd personally prefer to just go to 8.1 though. Don't think it's worth it to bump the requirement to a version that's already / almost? EOL..

TavoNiievez commented 2 years ago

would using `"codeception/lib-innerbrowser": "^1.0|^2.0" do the job?

you cannot support versions 1.x and 2.x at the same time because of these changes. So in the composer.json...

- "codeception/***": "^1.0",
+ "codeception/***": "^2.0",

Technically, support for PHP 7.4 or higher would be required at this time.

Going straight for PHP 8.1 as @SamMousa suggests is an option too, but that's why I thought it would make more sense to do that by targeting yii3.

samdark commented 2 years ago

Bumping a version sounds alright but I think we should go 8.1 when Codeception will require it, not earlier.

Yii3 is a totally different framework so we can't reuse Yii2 module with it.

rafaelmiti commented 2 years ago

Got lost in the discussion, but the suggested solution of locking to the 1.x worked, @TavoNiievez . Thanks! I'll not close the issue because I don't know if you guys ended the conversation.

Patrick-Remy commented 2 years ago

I ran exactly in the same issue, if upgrading via -W to those versons

"codeception/module-phpbrowser": "^2.0",
"codeception/module-yii2": "^1.1",

the codeception/lib-innerbrowser gets bumped to v2 without any error. No idea why composer doesn't fail there.

I would welcome if you release a v2 of this module like the other codeception modules that targets PHP 7.4, as it's currently not possible to upgrade to PHP8 for us, and many php libraries still support 7.4 + 8.0 as it's not a big deal for maintainers, but has a great benefit for all users.

Thanks for your work!

TavoNiievez commented 1 year ago

I think this issue has been fixed and can be closed.