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

Fixing #87 (again), #465 and similar ones #469

Merged
merged 3 commits into from
Apr 26, 2017
Merged

Fixing #87 (again), #465 and similar ones #469

merged 3 commits into from
Apr 26, 2017

Conversation

diemol
Copy link
Member

@diemol diemol commented Apr 23, 2017

Originally #87 was solved by #182 by adding ENV DBUS_SESSION_BUS_ADDRESS=/dev/null to the Chrome and Firefox Dockerfiles.

Nevertheless #358 was raised and due to that the command changed via #386 to RUN echo "DBUS_SESSION_BUS_ADDRESS=/dev/null" >> /etc/environment.

But after that, #87 started showing up again.

So this PR leaves the Dockerfiles in the same state as the commit made in #182 that fixed #87.

I tested the locally generated Chrome images it with the current tests but also with a simple test that help me to reproduce #465 and #87. Here is the simple test

I was not able to reproduce the error reported in #358, maybe @Sovetnikov or @a-k-g can provide a test case to reproduce it before this PR is considered to be merged?

@testphreak, maybe you can also generate the images locally based on this PR and test it?

@ddavison
Copy link
Member

if this truly fixes these issues (I thought I had fixed the dbus issue..)

then this probably needs to be reverted:

69707f0#diff-1454090f007c02ce89531779aca07244R13

I added the "dbus" package

@diemol
Copy link
Member Author

diemol commented Apr 24, 2017

Good point @ddavison,
I'll remove the package and test it first, to see if there are any differences. I'll let you know soon.

@diemol
Copy link
Member Author

diemol commented Apr 25, 2017

@ddavison I removed the dbus package and executed the tests in the repo and two times this test agains the locally generated containers. So it looks like it works also without the package.

Here are the commands I used to start the containers:

  • Generated the local images with:
 VERSION="local" make build
  • docker-compose (for normal and debug, remove the comments where it applies)
version: '2'
services:
  selenium-dev:
    image: selenium/hub:3.4.0-local
    ports:
      - 4444:4444/tcp
    environment:
      GRID_BROWSER_TIMEOUT: '180000'
      GRID_MAX_SESSION: '10'
      GRID_TIMEOUT: '180000'
  chrome-dev:
    image: selenium/node-chrome:3.4.0-local
    #image: selenium/node-chrome-debug:3.4.0-local
    #ports:
    #  - 5900:5900/tcp
    environment:
      HUB_PORT_4444_TCP_ADDR: selenium-dev
      HUB_PORT_4444_TCP_PORT: '4444'
      JAVA_OPTS: -Xmx4G
      shm_size: 1024MB
    volumes:
      - /dev/shm:/dev/shm
  • Standalone images
docker run --rm -p 4444:4444 -v /dev/shm:/dev/shm --shm-size=1g selenium/standalone-chrome:3.4.0-local
docker run --rm -p 4444:4444 -p 5900:5900 -v /dev/shm:/dev/shm --shm-size=1g selenium/standalone-chrome-debug:3.4.0-local

Any help testing this is appreciated.

@testphreak
Copy link
Contributor

@diemol, not sure I follow. @ddavison in his commit 12 days ago for 3.3.1-cesium, added dbus package to resolve the dbus issues. With the 3.4.0-bismuth, dbus package is no longer needed?

@diemol
Copy link
Member Author

diemol commented Apr 26, 2017

@testphreak the dbus package is installed since 3.3.1-cesium, and it is still there for the latest release (3.4.0-bismuth). It attempted to solve the dbus issues, but as we know, they are still there.

What I did in the last commit was to remove the package and see if it was enough to set DBUS_SESSION_BUS_ADDRESS=/dev/null. It seems it is enough, but I would be cool that someone else tests that as well.

I was talking about this with @elgalu, and in my opinion we could try first to just set the value as shown above, since it should solve the issue.
If after doing that, some people still report the issue (with a reproducible test case), we could install dbus and use it like @elgalu did in his image, see this comment. This would create an extra process in the container though.

@ddavison ddavison merged commit c256bbb into SeleniumHQ:master Apr 26, 2017
@testphreak
Copy link
Contributor

@diemol, build locally with your changes and ran a subset of my tests. Looks good to me.

@diemol
Copy link
Member Author

diemol commented Apr 27, 2017

Cool @testphreak, thanks for the feedback!

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

Successfully merging this pull request may close these issues.

Chromedriver frequently hangs when attempting to start a new session.
3 participants