Skip to content
This repository has been archived by the owner on Jul 29, 2024. It is now read-only.

feat(driverProvider) Adding browserstackProxy param in BrowserStack driverProvider #4852

Merged
merged 5 commits into from
Jul 13, 2018

Conversation

raghuhit
Copy link
Contributor

Adding browserstackProxy configuration param to enable users behind proxy to run their tests, example value for this config : "http://proxy.example.com:1234".

@raghuhit raghuhit changed the title Browserstack proxy fix feat(driverProvider) Adding browserstackProxy param in BrowserStack driverProvider Jun 15, 2018
@raghuhit
Copy link
Contributor Author

@qiyigg Can you please review this?

@raghuhit
Copy link
Contributor Author

@qiyigg
Hi, can you please review this?

Copy link
Contributor

@qiyigg qiyigg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have no idea of browserstackProxy, just give a general review feedback.

this.browserstackClient = BrowserstackClient.createAutomateClient({
username: this.config_.browserstackUser,
password: this.config_.browserstackKey,
proxy: this.config_.browserstackProxy
Copy link
Contributor

@qiyigg qiyigg Jun 28, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this.config_.browserstackProxy is not set, will it break this code? It would be better to have a test for it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have added test for browserStack driverProvider's basic functionality check.
It will not break by this.
And tests are now failing in circleci with error "Error: does not exist in logs: WebDriverError: Sauce Labs Authentication Error." and in travis due to some reason. I think there is some issue with Sauce labs credentials, can you please check and review this PR.

@raghuhit
Copy link
Contributor Author

raghuhit commented Jul 4, 2018

@qiyigg , Any comments on this? And can you re run the travis and circleci builds, as travis build ran successfully, when I ran it under my account from my repo.

@qiyigg
Copy link
Contributor

qiyigg commented Jul 6, 2018

Rebuilding..

@raghuhit
Copy link
Contributor Author

raghuhit commented Jul 9, 2018

@qiyigg I have fixed a minor issue in test cases in "scripts/errorTest.js" file with saucelabs' driver, after which the previously failing test in circleci is getting passed now but now the build is failing in circleci because it was not able to create GeckoDriverService, which is an intermittent issue and I don't think it has anything to do with code.
Also previously failing travisci builds are now passing. They were failing earlier because of timeout issues, which again is intermittent as you can see.
So can you please review and merge this PR.

@qiyigg
Copy link
Contributor

qiyigg commented Jul 13, 2018

merged, thanks

@qiyigg qiyigg merged commit 03e2209 into angular:master Jul 13, 2018
@raghuhit
Copy link
Contributor Author

Thank you :)

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

Successfully merging this pull request may close these issues.

3 participants