mucsi96 / nightwatch-api

[DEPRECATED] Cross-runner API for Nightwatch.js
https://nightwatch-api.netlify.com/
MIT License
84 stars 64 forks source link

Browserstack Local Integration #825

Closed reidj32 closed 4 years ago

reidj32 commented 4 years ago

The goal of this PR is to allow users of Browserstack Local to continue using this package, with the only requirements being installation of the option dependency, and passing the the proper configuration variables.

To enable support for this, I have added the needed hooks to the CliRunner creation so we setup the browserstack.Local instance. I converted much of the code to be promise based since browserstack local utilizes a callback for setup. I also added the shutdown step after the webdriver has been closed. The new interfaces and exported declarations are all in support of this new feature.

This issue was originally raised in #42, and I did leave a comment there with some initial details.

mucsi96 commented 4 years ago

Hi @reidj32! Thanks a lot for you contribution! I don't get one think. Wouldn`t it possible to do the integration in the test runner setup? Personally I would start this server in cucumber, mocha or jest hooks instead of this package.

reidj32 commented 4 years ago

Hi @mucsi96, thanks for your reply. I did start by trying that, but I found there to be an issue with the runner setup. Particularly the isWebDriverManaged callback. Because this is set to true in the current implementation, it always tries to start a grid server locally. We don't want this to happen with browserstack-local installed (or BrowserStack at all really). If you take a look at my changes there, you can see I conditionally return true when BrowserStack is not used. Something to note, I do have the following in my nightwatch configuration file:

selenium: {
    start_process: false,
    host: 'hub-cloud.browserstack.com',
    port: 80
},

I see you have a Cucumber + Browserstack example in your repo, so I'm curious why this is an issue for me. Another note, I was able to setup the local server outside of this package, but I had to open up node_modules/nightwatch-api/lib/client.js and change the isWebDriverManaged callback to the following.

runner.isWebDriverManaged = function () {
  // if (this.baseSettings.selenium) {
  //   this.baseSettings.selenium.start_process = true;
  // }

  return true;
};

After doing that, my test setup works just fine, so perhaps that is the actual issue here?

reidj32 commented 4 years ago

I have updated my code to remove all the bs-local integration and just looking to fix the managed web driver issue. Can you help me understand why we need to set start process to true for the base settings?

mucsi96 commented 4 years ago

Hi @reidj32! That's great news. We set start process to true. To be able to start the web river manually using startWebDriver function. With that if you call startWebDriver the webdriver will be started without the need to change the configuration. Can you explain why you think it's causing issues for you?

reidj32 commented 4 years ago

I was finally able to work around this issue. It turns out the main problem is calling the startWebDriver() function when connecting to BrowserStack. After updating my test suite to only call startSession() when the BrowserStack credentials are detected (and setting up the BrowserStackLocal server myself), everything is now working. Thanks for the information you provided to help me figure this out.