minkphp / MinkZombieDriver

Zombie.js driver for Mink framework
41 stars 49 forks source link

Reanimate travis build #195

Closed aik099 closed 3 years ago

aik099 commented 4 years ago

@stof , Travis CI isn't picking this PR.

stof commented 4 years ago

According to https://travis-ci.org/github/minkphp/MinkZombieDriver/requests, it could not parse the .travis.yml file. You probably have an issue in it.

aik099 commented 4 years ago

@stof , as you can see from build logs older Zombie versions (even with older NodeJS versions) can't install properly. Should we really support them?

Maybe we'll just support latest Zombie version (6.x detected automatically) with latest NodeJS version from Travis + Zombie 5.x version with NodeJS 6.x?

stof commented 4 years ago

I'm all for dropping old Zombie versions.

Ideally, I'd like a way to specify the zombie dependency automatically, so that we would have control over it, but that would require that the JS part of our code is managed by a JS package manager (yarn/npm/...), which would be a big change

aik099 commented 4 years ago

Now build is working (at least not dying) and on every PHP version the PHPUnit tests that fail are failing the same. Though I have no idea why they are failing. I've kept Zombie 4.0 installed on Node 5.4.1 as a reference to show, that it was working before.

@stof , any ideas? You can safely change PR code directly too.

Removing usage of ProcessBuilder had a side effect, that older Process class used on PHP 5.3 and PHP 5.4 doesn't like arguments as array and expects them as an escaped string already.

Not sure if we should try fixing it or drop PHP 5.3 and PHP 5.4 support 😄

stof commented 4 years ago

Well, ideally, we should first do a release with the existing state before dropping old PHP versions. But you can detect support for arrays in the Process class by checking for existence of Process::fromShellCommandLine method (or something like that, I'm not sure about the exact name).

stof commented 4 years ago

For the failing test for files, we probably need to use browser.attach rather than browser.fill for file inputs.

aik099 commented 4 years ago

Well, ideally, we should first do a release with the existing state before dropping old PHP versions. But you can detect support for arrays in the Process class by checking for existence of Process::fromShellCommandLine method (or something like that, I'm not sure about the exact name).

The method doesn't exist anymore. I'll check for Process::escapeArgument method, that indicates support for argument array conversion ability to a command line.

aik099 commented 4 years ago

For the failing test for files, we probably need to use browser.attach rather than browser.fill for file inputs.

I've tried that. The \Behat\Mink\Tests\Driver\Js\ChangeEventTest::testSetValueChangeEvent is structured the way, that we first reset input value and then set new value. That approach doesn't work for upload fields due https://github.com/assaf/zombie/issues/1203, which I've just reported.

aik099 commented 4 years ago

The basic auth related tests fail because somehow on Travis CI PHP auth headers didn't reach PHP. Maybe due fact, that PHP_SAPI is CGI/FastCGI.

The \Behat\Mink\Tests\Driver\Js\ChangeEventTest::testSetValueChangeEvent tests fail, because new Zombie version fires 2 change events for 1 change for select/radio/checkbox elements. Reported as https://github.com/assaf/zombie/issues/1204.

Locally fixed \Behat\Mink\Tests\Driver\Js\EventsTest::testKeyboardEvents. The issue was with usage of deprecated (probably Zombie already dropped it) usage of e.keyCode event property. The correct way is to use e.charCode property of event.

aik099 commented 4 years ago

If using built-in PHP web server of PHP 7.4, then Behat\Mink\Tests\Driver\Basic\CookieTest::testCookie also fails with this message:

Failed asserting that 'Basic Form Previous cookie: client+cookie+set' contains "Previous cookie: client cookie set".

@stof , I remember seeing some change where urlencode (or decode) was replaced by rawurlencode (or decode), but I don't remember where so that I can replicate that in this driver.

aik099 commented 4 years ago

Strange, but \Behat\Mink\Tests\Driver\Js\EventsTest::testKeyboardEvents fix haven't worked on Travis CI, but worked locally.

aik099 commented 4 years ago

Out of curiosity I've also tested Zombie 5.x on Travis CI and all test did pass, except one that calls setValue on upload field twice, that fails due the same reason as for Zombie 6.

stof commented 3 years ago

Closing in favor of #196