Skip to content

Allow docker image used for selenium to be specified#249

Merged
rnorth merged 2 commits into
masterfrom
override-selenium-image
Jan 16, 2017
Merged

Allow docker image used for selenium to be specified#249
rnorth merged 2 commits into
masterfrom
override-selenium-image

Conversation

@rnorth
Copy link
Copy Markdown
Member

@rnorth rnorth commented Dec 4, 2016

... instead of always trying to figure out from classpath dependencies. Refs #171

…ys trying to figure out from classpath dependencies. Refs #171
public SELF withDesiredCapabilities(DesiredCapabilities desiredCapabilities) {
super.setDockerImageName(getImageForCapabilities(desiredCapabilities));

if (! imageNameIsSet) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

formatting issue?

*/
public BrowserWebDriverContainer(String dockerImageName) {
super.setDockerImageName(dockerImageName);
this.imageNameIsSet = true;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

can't we just check that getDockerImageName() is not null?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Unfortunately, no; GenericContainer sets a default image name (alpine:3.2) on the no-args constructor. I agree with the principle, though...

public BrowserWebDriverContainer firefox = new BrowserWebDriverContainer("selenium/standalone-firefox-debug:2.53.1-beryllium")
.withDesiredCapabilities(DesiredCapabilities.firefox());

@Test
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

сan't we just inherit this tests from BaseWebDriverContainerTest?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The @Test methods? We've not done this yet in the other subclasses. I guess this would require making the container field be a protected member of the parent class and initialize it in the subclass constructors. Or is there a simpler way?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

you can make the parent class abstract with abstract protected BrowserWebDriverContainer getContainer() method. We were using something like that in Zipkin's tests:
https://github.com/openzipkin/zipkin/blob/master/zipkin-storage/mysql/src/test/java/zipkin/storage/mysql/MySQLTest.java

@bsideup bsideup added this to the 1.1.8 milestone Jan 15, 2017
@bsideup
Copy link
Copy Markdown
Member

bsideup commented Jan 15, 2017

LGTM, we can change the tests later, all-at-once

@rnorth rnorth merged commit 398c6b6 into master Jan 16, 2017
@rnorth rnorth deleted the override-selenium-image branch January 22, 2017 07:59
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.

2 participants