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

Proposal: add seleniumServerStartTimeout in Local DriverProvider #3791

Merged
merged 1 commit into from
Dec 13, 2016

Conversation

akirakoyasu
Copy link
Contributor

@akirakoyasu akirakoyasu commented Dec 4, 2016

It's nice to specify how long time the Local DriverProvider waits for the selenium-webdriver server to start.

The Local DriverProvider starts local selenium-webdriver server, and waits for the server in 30,000ms now. That's because DriverService#start() of selenium-webdriver/remote has an arg opt_timeoutMs default 30,000ms.

30,000ms is almost enough long, but that depends on environment.

@googlebot
Copy link

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

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

1 similar comment
@googlebot
Copy link

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

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

@akirakoyasu
Copy link
Contributor Author

@googlebot I signed it!

@googlebot
Copy link

CLAs look good, thanks!

1 similar comment
@googlebot
Copy link

CLAs look good, thanks!

@googlebot googlebot added cla: yes and removed cla: no labels Dec 4, 2016
@@ -29,6 +29,13 @@ export interface Config {
seleniumServerJar?: string;

/**
* The timeout milliseconds waiting for standalone Selenium Server to start.
Copy link
Contributor

Choose a reason for hiding this comment

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

Add a note that it's only for local selenium servers

Copy link
Contributor Author

@akirakoyasu akirakoyasu Dec 6, 2016

Choose a reason for hiding this comment

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

@sjelin
Thanks, I fixed it. But...
Or other expression is better?

Copy link
Contributor

@sjelin sjelin left a comment

Choose a reason for hiding this comment

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

In your last comment you asked: "Or other expression is better?"

What does that mean?

@@ -29,6 +29,13 @@ export interface Config {
seleniumServerJar?: string;

/**
* The timeout milliseconds waiting for the local standalone Selenium Server to start.
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: should be "waiting for a local Selenium Standalone Server

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 fixed it.

@akirakoyasu
Copy link
Contributor Author

I'm sorry for my broken English. 😢

@akirakoyasu akirakoyasu force-pushed the patch-add-start-timeout branch from cdfc5ad to 141136a Compare December 8, 2016 13:25
@akirakoyasu
Copy link
Contributor Author

akirakoyasu commented Dec 8, 2016

And I squashed all commits.

@sjelin sjelin merged commit b337a8e into angular:master Dec 13, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants