Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reset in Selenium2Driver must validate driver before reset #727

Open
murphy83 opened this issue Dec 2, 2016 · 5 comments
Open

Reset in Selenium2Driver must validate driver before reset #727

murphy83 opened this issue Dec 2, 2016 · 5 comments

Comments

@murphy83
Copy link

murphy83 commented Dec 2, 2016

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
Copy link
Member

stof commented Dec 2, 2016

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
Copy link
Member

stof commented Dec 2, 2016

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
Copy link
Author

murphy83 commented Dec 2, 2016

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
Copy link
Member

stof commented Dec 2, 2016

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
Copy link
Member

aik099 commented Dec 2, 2016

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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants