-
Notifications
You must be signed in to change notification settings - Fork 165
Description
@stof @aik099 I've got the newly-built driver passing all (24!) tests*!
While there are definitely some final changes required, I think it's in a good state to start a discussion/review on moving forward.
Here's the PR to my own repository: https://github.com/uuf6429/MinkPhpWebDriver/pull/1 (you can still see what changed)
The good parts
- supports Selenium 2, 3 and 4
- tested on Chrome and Firefox, and PHP 7.4-8.2
- bridges webdriver protocol incompatibilities, such as the case of window names vs window handles
- more functionality is tested, compared to MinkSelenium2Driver
- can be run through act
- fixture test server is now part of the test boostrap, so no need to care about starting it up
- tonne of other minor improvements:- syn getter script, license file, smaller composer archive, etc
The questionable parts
- I removed the changelog file (I wasn't thinking of contributing the driver to this project at that point, and I think it's pointless to have a changelog when github has clear release notes)
- similarly, some coding style was also changed for the same reason
- it now contains IDE-specific PHP attributes (which would have been more useful in the driver interface), but they are otherwise harmless
- requires PHP 7.4 (a requirement of php-webdriver)
- there's a leftover TODO.md which I'll make sure to remove soon
- I removed/disable the symfony phpunit bridge thing - I found it to cause more trouble rather than be useful
- to be able to run the tests, I had to make it run against a fork of driver-testsuite ('coz of Test was not remapping remotely-used fixture file driver-testsuite#67 and Fix tests failing from format ambiguities driver-testsuite#68)
So how shall we proceed? Do you see this as a "v2" of this repo, or as a different repo altogether? What are the main things you'd rather see changed/fixed? (what are the next steps?)
* events are not working well in Chrome + Selenium 2 (it seems to be a browser problem), but it's not new: I am able to prove that here, by having a fork testing against chrome instead of firefox.
Because of this problem, chrome+se2 tests are allowed to fail. Would be nice to have a proper fix, but I'm not sure this is possible.