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

Add restart and/or ensure-running commands #3

Open
schlomo opened this issue Jul 21, 2017 · 6 comments
Open

Add restart and/or ensure-running commands #3

schlomo opened this issue Jul 21, 2017 · 6 comments

Comments

@schlomo
Copy link

schlomo commented Jul 21, 2017

I would like to be able to have

curl -sSL https://raw.githubusercontent.com/dosel/t/i/p | env USE_NET_HOST=true bash -s start

in my test runner scripts before starting the tests. ATM this fails if Zalenium is already running:

sschapiro@zalando-26723:~/src/schlomo/zalenium-demo$ time curl -sSL https://raw.githubusercontent.com/dosel/t/i/p | env USE_NET_HOST=true bash -s start ; echo RETURN_CODE: $?
Checking dependencies... Done Checking dependencies.
latest: Pulling from dosel/zalenium
Digest: sha256:f17d8ba75b3677a2d66a42417d2b011ca0ac080a1f4b21664d2a3c4030600368
Status: Image is up to date for dosel/zalenium:latest
latest: Pulling from elgalu/selenium
Digest: sha256:eb49a354d1b77d2e8fbcf43d5222745d14056e500ab47c5f78a749e096ff746f
Status: Image is up to date for elgalu/selenium:latest
Removing exited docker-selenium containers...
014b9781d6f7
Starting Zalenium in docker...
WARN: Sauce Labs will not be enabled because the var $SAUCE_USERNAME is NOT present
WARN: BrowserStack will not be enabled because the var $BROWSER_STACK_USER is NOT present
WARN: Testing Bot will not be enabled because the var $TESTINGBOT_SECRET is NOT present
docker: Error response from daemon: Conflict. The container name "/zalenium" is already in use by container "45916a8be9b6d45a699d9592b461525462ce7c9f1caea77389e19e4d189b3f37". You have to remove (or rename) that container to be able to reuse that name.
See 'docker run --help'.

real	0m6,333s
user	0m0,270s
sys	0m0,136s
RETURN_CODE: 125

I therefore kindly ask you to enhance the script so that it can be added everywhere as an "ensure Zalenium is running" script.

Maybe it is also enough to add a new command "restart" or "ensure" that first stops and then starts Zalenium.

@elgalu
Copy link
Member

elgalu commented Jul 21, 2017

Hi, we made the stop command idempotent precisely to cover this use case.

Just do the following before doing the start

curl -sSL https://raw.githubusercontent.com/dosel/t/i/p | bash -s stop

@schlomo
Copy link
Author

schlomo commented Jul 21, 2017

Yes, I did: https://gist.github.com/schlomo/8bd30a130bc28cfb1facf2d37ee25456#file-run-tests-sh

As this is probably a very common use-case I would prefer to have it in the tool itself. As you wish.

@elgalu
Copy link
Member

elgalu commented Jul 21, 2017

Nice!!! Even tweeted and all! https://twitter.com/schlomoschapiro/status/888394803377000448

Regarding this change request, it was like that at first but then we changed so we wouldn't kill any running Zalenium and therefore running tests by mistake.

idk what do you think @diemol ? should be keep it as it is or go with the stop first then start?

@schlomo
Copy link
Author

schlomo commented Jul 21, 2017

I wouldn't change the behavior of start but add a new command like restart or ensure-running. That way users will know for sure what it does.

@diemol
Copy link
Member

diemol commented Jul 22, 2017

I don't remember this :)

Regarding this change request, it was like that at first but then we changed so we wouldn't kill any running Zalenium and therefore running tests by mistake.

But yes, a start should be clean. At least we should have the restart option. We also need to document all the available parameters of the script.

@elgalu elgalu changed the title make Zalenium start idempotent Add restart and/or ensure-running commands Jul 22, 2017
@elgalu
Copy link
Member

elgalu commented Jul 22, 2017

I updated the issue. PR's are welcome either to add this and/or to refactor this script. It was created as an internal convenience but it has proved to be widely used and needed so maybe is time to do a proper rewrite.

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

No branches or pull requests

3 participants