Use the Safari 10 WebDriver to run tests

RLovelett commented 7 years ago

For all the reasons addressed in a WebKit blog post, https://webkit.org/blog/6901/webdriver-support-in-safari-10/, this method should be preferred over the previous method.

How does this work?

It launches /usr/bin/safaridriver on a specified port (defaults to 4444).
It then "ping"s the safaridriver until it starts accepting incoming connections.
Creates a new WebDriver session (e.g., opens Safari).
Navigates to the Karma provided URL.

Caveats

By going this route the launcher named Safari now only supports Safari 10 on OS X El Capitan and newer. SafariLegacy works as it currently does.

Notes

I'm not thrilled with the retry method. It just does not feel right. Though I'm not sure how else to know when the safaridriver is fully launched and accepting incoming connections.

This should address the concerns of both #6 and #20.

googlebot commented 7 years ago

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

:memo: Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.

If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
If you signed the CLA as a corporation, please let us know the company's name.

RLovelett commented 7 years ago

I signed it!

googlebot commented 7 years ago

CLAs look good, thanks!

dignifiedquire commented 7 years ago

To make it possible to test both old and new safari versions could you keep the current one as a legacy launcher? similiar to how the chrome-launcher exposes multiple different launchers

RLovelett commented 7 years ago

I think I understand how that works in the chrome-launcher; so I can give that a try. Do you have any suggestions for names?

The reason why I ask is that in the chrome-launcher it truly launches different applications. Yet it this instance it's really launching different versions of the same browser. Technically the "legacy" version covers all versions of Safari that the new one does while the new one only covers Safari 10 and newer.

Suggestions:

SafariWebDriver and Safari
Safari and SafariLegacy
???

dignifiedquire commented 7 years ago

I think Safari and SafariLegacy are pretty good

RLovelett commented 6 years ago

@dignifiedquire better late than never right? 😏

I've incorporated the feedback. There are now two exported launchers Safari and SafariLegacy.

shinnn commented 6 years ago

When I tried to use this branch with macOS Mojave 10.14 and Safari 12.0, Karma threw the following error.

Error: [init({"browserName":"safari"})] The environment you requested was unavailable.
Request body does not contain required parameter 'capabilities'.

https://github.com/SeleniumHQ/selenium/issues/6026#issuecomment-423599780 is the reason why it stopped working:

This issue happens because Safari 12 uses a new (W3C) webdriver protocol which appears incompatible with selenium-webdriver v3.6

We should pass --legacy flag to safaridriver somehow, I'm not familiar with the API of wd module though.

RLovelett commented 6 years ago

The biggest problem is that is not a backwards compatible solution. We cannot globally just pass --legacy because on Safari 10/11 it does not understand that flag and exits with a non-zero status code. That leaves needing to gate the flag based on Safari version.

Not thrilled with that. Better yet would be to update the wd library to support W3C or to use a new library all-together that supports W3C.

RLovelett commented 5 years ago

@shinnn I've added a commit, 5b77a2f605d1dd84080f5f36c81d54609cacb691, in my environment I am now able to support Safari 10-12 with this change.

mgol commented 5 years ago

One concern with using safaridriver is that the window is not interactive so you cannot click Debug, open the inspector etc. That may sometimes be necessary so I wonder if we should support running new Safari the old way as well (not as a default)?

For example, jQuery's Karma setup requires clicking Debug to actually run the tests.

RLovelett commented 5 years ago

... so I wonder if we should support running new Safari the old way as well (not as a default)?

This change does just that. The old way is now called SafariLegacy as discussed above.

One concern with using safaridriver is that the window is not interactive so you cannot click Debug, open the inspector etc.

It is still possible in the new one. I have personally made use of "breaking the glass pane" over the automation window as discussed in the WebKit.org article: WebDriver Support in Safari 10. Granted it is a bit more clicking and as I recall required me to use some debugger statements but it was possible.

pdesjardins90 commented 5 years ago

@dignifiedquire can you merge this PR?

gergo-papp commented 5 years ago

Hey @RLovelett, I tried to test this change on Travis and I can't seem to get it to work. I've seen from previous comments that this is a known issue but I think we should make sure this works without requiring any manual configuration. The most important use case should be supporting this on a CI (such as on OSX running on TravisCI) - at least that's my opinion...

Some further info in case you need it:

I used "karma-safari-launcher": "git://github.com/RLovelett/karma-safari-launcher.git#safari-webdriver" in my package.json file and tried to launch Safari 11.0.3 (Mac OS X 10.13.3) on Travis CI.

I get the following error:

15 02 2019 10:08:11.884:INFO [karma-server]: Karma v3.1.1 server started at http://0.0.0.0:8082/
15 02 2019 10:08:11.897:INFO [launcher]: Launching browsers Safari with concurrency unlimited
15 02 2019 10:08:11.988:INFO [launcher]: Starting browser Safari via WebDriver at http://127.0.0.1:4444/

Could not create a session: You must enable the 'Allow Remote Automation' option in Safari's Develop menu to control Safari via WebDriver.
    at /Users/travis/build/ni-kismet/helium-webserver/content/node_modules/wd/lib/webdriver.js:148:15
    at Request._callback (/Users/travis/build/ni-kismet/helium-webserver/content/node_modules/wd/lib/http-utils.js:89:7)
    at Request.self.callback (/Users/travis/build/ni-kismet/helium-webserver/content/node_modules/request/request.js:185:22)
    at Request.emit (events.js:197:13)
    at Request.EventEmitter.emit (domain.js:446:20)
    at Request.<anonymous> (/Users/travis/build/ni-kismet/helium-webserver/content/node_modules/request/request.js:1161:10)
    at Request.emit (events.js:197:13)
    at Request.EventEmitter.emit (domain.js:446:20)
    at IncomingMessage.<anonymous> (/Users/travis/build/ni-kismet/helium-webserver/content/node_modules/request/request.js:1083:12)
    at Object.onceWrapper (events.js:285:13)
    at IncomingMessage.emit (events.js:202:15)
    at IncomingMessage.EventEmitter.emit (domain.js:446:20)
    at endReadableNT (_stream_readable.js:1129:12)
    at processTicksAndRejections (internal/process/next_tick.js:76:17)
  data:
   `{"status":33,"sessionId":"","value":{"message":"Could not create a session: You must enable the 'Allow Remote Automation' option in Safari's Develop menu to control Safari via WebDriver."}}` }
15 02 2019 10:09:12.041:WARN [launcher]: Safari via WebDriver at http://127.0.0.1:4444/ have not captured in 60000 ms, killing.

I used karma start karma.conf.js --single-run --browsers Safari to start my karma tests.

As far as I understand webdriver support is not enabled by default but I don't know how to set it programatically. I also think this should be set by karma-safari-launcher (if it's possible).

Or am I missing something? Thanks!

RLovelett commented 5 years ago

@gergo-papp this is the relevant part of the error message:

Could not create a session: You must enable the 'Allow Remote Automation' option in Safari's Develop menu to control Safari via WebDriver.

That seems to indicate that you have not configured Safari to enable support for the web driver. There are a few articles out there: Testing with WebDriver in Safari: Configure Safari to Enable WebDriver Support and WebDriver Support in Safari 10.

The steps are different depending on the version of Safari you have but on the latest versions it is effectively just safaridriver --enable.

Hopefully that helps.

gergo-papp commented 5 years ago

@RLovelett thanks for the response. In this case I’m happy with this change. Is this repo still maintained? I haven’t seen too much activity

RLovelett commented 5 years ago

Is this repo still maintained?

That is a good question. I think so but I could be wrong.

LeviticusMB commented 5 years ago

Can somebody please merge this and get it out on NPM ASAP?

mgol commented 5 years ago

@dignifiedquire Any chance of merging this?

mgol commented 4 years ago

@dignifiedquire Could you merge the PR?

LeviticusMB commented 3 years ago

FYI: I've published @onslip/karma-safari-launcher from https://github.com/Onslip/karma-safari-launcher/tree/onslip.

taymoork2 commented 3 years ago

So I've been using the code from this PR for a while now and it runs fine. I use npm so I've been using patch-package to patch the changes made in this PR to the package installed in my repo. You can manually create the patch using the package or just copy the patch I've linked below into a patches folder. I believe yarn has built in patching

https://gist.github.com/taymoork2/8eb2be290579467f0d0090671e031ca8

leobalter commented 3 years ago

Seconding @mgol requests...

@dignifiedquire Could you merge the PR, please?

johnjbarton commented 3 years ago

Any volunteers to maintain this repo?

leobalter commented 3 years ago

I mean, I'd be happy to merge and publish this if I have access to the npm package. I'd prefer to share this with @mgol if he's +1 and has the bandwidth, considering the work he's done at the jQuery foundation.

mgol commented 3 years ago

I'd prefer to share this with @mgol if he's +1 and has the bandwidth

I'm not able to commit to a timely feedback to issues but at least looking at available PRs and an occasional publish should be fine, especially if shared with you, @leobalter.

LeviticusMB commented 3 years ago

FYI: I've published @onslip/karma-safari-launcher from https://github.com/Onslip/karma-safari-launcher/tree/onslip.

FYI again: We've switched to Onslip/karma-playwright-launcher#v0.2.0 and I don't think we'll ever go back to testing against real Safari (or any other browser, for that matter). Playwright as been rock solid for us and I can't say I miss all the communication errors, hangups or failed unit tests because of Safaris aggressive low-power mode ...

karma-runner / karma-safari-launcher