Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

customize driver polling interval in large Kubernetes nightwatch clusters #2193

Closed
wants to merge 12 commits into from

Conversation

taitelman
Copy link
Contributor

@taitelman taitelman commented Aug 29, 2019

with regards to #2192
in large container swarm we need longer timeout to see that web driver is up and responding.
added customized way to increase that timeout.
Actual stress tests showed that it might take the chrome driver few seconds (6) to responde for localhost:9515/status endpoint.

also: docker host machine will need to increase max file handlers due to the new spawn mechanism of the driver (used to be handled inside Selenium Java process)

@taitelman taitelman changed the title customize driver polling interval in large K8s nightwatch clusters customize driver polling interval in large Kubernets nightwatch clusters Aug 29, 2019
@taitelman taitelman changed the title customize driver polling interval in large Kubernets nightwatch clusters customize driver polling interval in large Kubernetes nightwatch clusters Aug 29, 2019
@beatfactor
Copy link
Member

Thanks for your contribution. There is also another related PR here #2144. Maybe these 2 could be combined?

Also it would be interesting to hear a bit more about your usage of Nightwatch. Would you consider contributing parts of your solution or maybe write a blog post?

If you could add a few lines about your project here #2154 that will be much appreciated.

@taitelman
Copy link
Contributor Author

taitelman commented Aug 29, 2019

Indeed my PR duplicates #2144 trying to merge them.
I have added much more info about my usage in defect #2192.
I am scaling nightwatch to the medium clusters (100+ VMs ) to automate parallel UI tests of a web product.

Also it would be interesting to hear a bit more about your usage of Nightwatch. Would you consider contributing parts of your solution or maybe write a blog post?
If you could add a few lines about your project here #2154 that will be much appreciated.

@taitelman
Copy link
Contributor Author

taitelman commented Aug 29, 2019

ok. I've pulled and enhanced #2144 into this PR.
you may accept this PR.

@@ -90,6 +90,9 @@ class BaseWDServer {
this.pollStarted = false;
this.processCreatedTimeout = null;
this.processStatusCheckTimeout = null;
// for multi docker container increase max status polling retries and intervals.
this.maxStatusPollTries = settings.max_status_poll_tries || BaseWDServer.maxStatusPollTries();
Copy link
Member

Choose a reason for hiding this comment

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

BaseWDServer.maxStatusPollTries is a static property, not a method and also I don't see maxStatusPollTries being converted to an instance property, it is still static: https://github.com/taitelman/nightwatch/blob/fixDriverPolling/lib/runner/wd-instances/base-wd-server.js#L60

Copy link
Contributor Author

@taitelman taitelman Sep 2, 2019

Choose a reason for hiding this comment

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

correct. fixing...
maxStatusPollTries is converted to instance property in line 94 and later we use it here

checkPingStatus() {
    if (this.statusPingTries < this.maxStatusPollTries) {...

however, it is a good question if we want to expose a getter for that instance member. I didn't see any external usage for such case.
I may have done a poor member name choice by using this.maxStatusPollTries and BaseWDServer.maxStatusPollTries in the same code

Copy link
Contributor Author

@taitelman taitelman Sep 2, 2019

Choose a reason for hiding this comment

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

I don't see any commits from #2144 and also there are some issues here so I'll go ahead and merge the initial PR.

indeed. I did it manually since I changed few items at once. you won't see any git line from #2144 into here

@beatfactor
Copy link
Member

I don't see any commits from #2144 and also there are some issues here so I'll go ahead and merge the initial PR.

@beatfactor beatfactor closed this Aug 30, 2019
@beatfactor beatfactor reopened this Aug 30, 2019
@taitelman
Copy link
Contributor Author

ok. let me check this again

@stale
Copy link

stale bot commented Dec 1, 2019

This issue has been automatically marked as stale because it has not had any recent activity.
If possible, please retry using the latest Nightwatch version and update the issue with any relevant details. If no further activity occurs, it will be closed. Thank you for your contribution.

@stale stale bot added the stale label Dec 1, 2019
@stale stale bot closed this Dec 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants