-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Modify Selenium container startup detection #351
Conversation
…ess string observed in v3.x of Selenium containers.
|
||
@Test | ||
public void testAdditionalStartupString() { | ||
BrowserWebDriverContainer chrome = new BrowserWebDriverContainer("selenium/standalone-chrome-debug:" + tag) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can use try-with-resources here
* Simple test to check that readiness detection works correctly across major versions of the containers. | ||
*/ | ||
@RunWith(Parameterized.class) | ||
public class Selenium3xTest extends BaseWebDriverContainerTest { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can remove "extends BaseWebDriverContainerTest" because we don't use anything from it
@@ -56,7 +56,7 @@ | |||
*/ | |||
public BrowserWebDriverContainer() { | |||
this.waitStrategy = new LogMessageWaitStrategy() | |||
.withRegEx(".*RemoteWebDriver instances should connect to.*\n") | |||
.withRegEx(".*Selenium Server is up and running.*\n") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this String available in all versions? Because I thought you wanted something like
".*(RemoteWebDriver instances should connect to)|(Selenium Server is up and running).*\n"
... using alternative readiness string observed in v3.x of Selenium containers.
It seems that somewhere along the line the string that the selenium containers use to signal readiness has changed. This commit fixes that issue by allowing either string to be detected. The test tries two tag versions with differing outputs.
FYI @bsideup @kiview