MinkSelenium2Driver icon indicating copy to clipboard operation
MinkSelenium2Driver copied to clipboard

Add support of getStatus() and getBrowser() api.

Open rajeshtaneja opened this issue 9 years ago • 13 comments

With recent releases of Selenium we have been facing few fails because of click button (https://github.com/SeleniumHQ/selenium/issues/1202).

Also, as they have recently released lot of versions in last few months it's hard to keep up with the changes and direction they take. It will be helpful if there is a way to notify the user about selenium version being used and if that is supported.

Can we please have getSeleniumVersion and getBrowserName implementation for the same.

rajeshtaneja avatar Feb 08 '16 08:02 rajeshtaneja

And you plan to put added method calls all around test suite?

aik099 avatar Feb 08 '16 09:02 aik099

I am planning to use it in beforeScenario hook for the first JS sesssion, to notify if the selenium version is greater then supported version. If failure found then report them, and use the last supported version.

rajeshtaneja avatar Feb 08 '16 09:02 rajeshtaneja

But message would be misleading in case of 99% cases, because the particular problem that you've reported in https://github.com/SeleniumHQ/selenium/issues/1202 won't be the cause of each error.

aik099 avatar Feb 08 '16 09:02 aik099

The reason I am thinking of checking the selenium version is to inform user about the selenium version not being supported if greater then what we have tested with before release. To ensure the failure found is genuine, the dev should test it with recommended selenium version.

We have a custom progress format which display following before the execution starts: Moodle 3.1dev (Build: 20160129), pgsql, ead6c29bb58b194861b05379bff21b583106a13e Server OS "Linux", Browser: "firefox" You are not using recommended selenium version. Please refer https://docs.moodle.org/dev/Acceptance_testing/Browsers#Working_combinations_of_OS.2BBrowser.2Bselenium

Probably it will be nice to implement getStatus() rather then being specific about selenium and implement https://code.google.com/p/selenium/wiki/JsonWireProtocol#/status

Please let me know if that make sense to you and I will update the patch accordingly for getStatus() and getBrowser() in mink and here.

In addition to above it would be nice to have getWebDriver() as well.

rajeshtaneja avatar Feb 09 '16 02:02 rajeshtaneja

Not sure if there is status analog in other drivers.

aik099 avatar Feb 09 '16 07:02 aik099

Sorry for the late response on this @aik099

Do you think adding getStatus() in this driver is an option ? If yes, then I will be happy to create a patch for the same and rename this issue to just add getStatus()

rajeshtaneja avatar Feb 29 '16 05:02 rajeshtaneja

The getBrowserName might be a good addition to DriverInterface, but the getSeleniumVersion I'm not so sure anymore.

@stof , what you think about this?

aik099 avatar Feb 29 '16 07:02 aik099

@aik099 rather then having getSeleniumVersion, I think it will be nice to have getStatus() implementing https://github.com/SeleniumHQ/selenium/wiki/JsonWireProtocol#status

As far as getBrowserName(), it should only be implemented here, as CoreDriver or DriverInterface doesn't define it. It is only valid for MinkSelenium2Driver

rajeshtaneja avatar Feb 29 '16 08:02 rajeshtaneja

The more the talk about it the more it seems like a very specific edge case that you're having. Not sure if anybody have came across need for what you're suggesting in associated PR before.

aik099 avatar Feb 29 '16 09:02 aik099

@aik099 We have a formatter which override progress formatter and prints information about

  • Branch
  • PHP version
  • OS details
  • Browser (We test on FF, chrome, phantomjs, safari and IE)
  • database used.

In addition to above it will be helpful if we can add selenium version information, so when someone report a failure, it will be easy for us to identify and replicate the issue.

Since last Oct 2015 Selenium has released 9 versions and when someone report a failure it will be easy for us to replicate the problem and get to resolution.

Not sure if anyone else need this, but I will appreciate if you consider this request of implementing getStatus() and getBrowser() api. As it will be easy for us to use it in our formatter and replicate problems easily by knowing about environment used by user.

rajeshtaneja avatar Feb 29 '16 11:02 rajeshtaneja

@stof any thoughts on this request ?

rajeshtaneja avatar Mar 08 '16 03:03 rajeshtaneja

About the PR:

  • the getStatus method seems not be generally useful and you already can have it's functionality by calling $driver->$this->getWebDriver()->status() code
  • I like getBrowserName method, but it needs more work to be mergeable

Plan:

  1. send PR to the main Mink repo, that would do this:
    • add getBrowserName method to DriverInterface
    • add CoreDriver::getBrowserName that would throw an exception about non-implemented method (see other similar methods in there)
    • add Session::getBrowserName method, that would just return result of calling getBrowserName on it's driver
    • add new unit tests to main test suite and to driver test suite to cover new methods
  2. send PR to each driver, that use browsers (Selenium2 and Sahi if I'm not mistaken) where getBrowserName method is implemented

aik099 avatar Jul 03 '16 11:07 aik099

I'm -1 on adding getBrowserName in the Session and Driver APIs, as this would be specific to Selenium (SahiDriver is unsupported, and may not even have this API)

stof avatar Jan 03 '17 14:01 stof