minkphp / Mink

PHP web browser emulator abstraction
https://mink.behat.org/
MIT License
1.6k stars 280 forks source link

Reset in Selenium2Driver must validate driver before reset #727

Open murphy83 opened 7 years ago

murphy83 commented 7 years ago

Currently there is no check in mink-selenium2-driver/src/Selenium2Driver.php: 357 wether the wdSession has been properly initialized before. Which leads to errors when the driver is used by a setup-Routine such as in PHPUnit->setUp() to automatically initialize the browser before each test. According to DriverInterface it is supported to call reset on a fresh driver.

stof commented 7 years ago

a fresh driver is a driver which has been started but has not visited any page yet. It is not the same than a driver which is not started

stof commented 7 years ago

and the interface tells you that the only supported action on a stopped driver (which is the initial state of a driver) is to start it: https://github.com/minkphp/Mink/blob/v1.7.1/src/Driver/DriverInterface.php#L65

murphy83 commented 7 years ago

However using reset() should not result in a fatal error, I would agree to raise an exception (eg. "unsupported action on non initialized driver") - but currently a fatal error occurs which tells that an operation on a non-object (null) was attempted.

stof commented 7 years ago

Well, then we would have to check all methods to ensure that the driver was started. I'm not sure this is worth it, considering that it is an unsupported usage anyway (and the doc states that drivers are not required to handle it)

aik099 commented 7 years ago

I agree with @stof on this one. Better to avoid extra check against unsupported class usage in favor of speeding up program execution time (no checks against stuff, that rarely happens = program runs faster).