-
Notifications
You must be signed in to change notification settings - Fork 61
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
Get browser version in local and container #215
Conversation
src/main/java/com/znsio/teswiz/runner/BrowserDriverManager.java
Outdated
Show resolved
Hide resolved
src/main/java/com/znsio/teswiz/runner/BrowserDriverManager.java
Outdated
Show resolved
Hide resolved
src/main/java/com/znsio/teswiz/runner/BrowserDriverManager.java
Outdated
Show resolved
Hide resolved
src/main/java/com/znsio/teswiz/runner/BrowserDriverManager.java
Outdated
Show resolved
Hide resolved
src/main/java/com/znsio/teswiz/runner/BrowserDriverManager.java
Outdated
Show resolved
Hide resolved
case "firefox": | ||
binaryPath = getFirefoxBinaryPath(); | ||
break; | ||
case "safari": |
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.
If you are adding break in chrome, firefox, why not after safari
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.
to fetch the version of safari, we are executing a separate method getSafariVersion() and skipping the next steps in current method
src/main/java/com/znsio/teswiz/runner/BrowserDriverManager.java
Outdated
Show resolved
Hide resolved
src/main/java/com/znsio/teswiz/runner/BrowserDriverManager.java
Outdated
Show resolved
Hide resolved
} | ||
} | ||
|
||
assert containerId != null; |
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.
What is the use of this assert here
and why are we passing it here
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.
if the container object does not find the matching id for container name it will return null. the assertion is added for the same.
src/main/java/com/znsio/teswiz/runner/BrowserDriverManager.java
Outdated
Show resolved
Hide resolved
src/main/java/com/znsio/teswiz/runner/BrowserDriverManager.java
Outdated
Show resolved
Hide resolved
import org.openqa.selenium.remote.RemoteWebDriver; | ||
import org.openqa.selenium.safari.SafariDriver; | ||
import org.openqa.selenium.safari.SafariOptions; | ||
import java.io.File; | ||
import org.yaml.snakeyaml.Yaml; | ||
import java.io.*; |
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.
are the total imports from java.io > 4, if not you can import specific classes.
same for java.util
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.
total imports from java.io are > 4
Map<String, Object> services = (Map<String, Object>) data.get("services"); | ||
Map<String, Object> selenium = (Map<String, Object>) services.get("chrome"); | ||
imageName = (String) selenium.get("image"); | ||
LOGGER.info("Selenium image: " + imageName); |
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.
String.format() can be used to maintain uniformity
default: | ||
throw new InvalidTestDataException("Invalid browser : " + browserType); | ||
} | ||
ChromeOptions chromeOptions = new ChromeOptions(); |
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 required we have helper methods to get binary paths.
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.
Chrome options was used to set the binary path, removed and updated as we have helper methods to get the path
The implementation for getting browser version is completed and merged to teswiz |
This PR covers the implementation for getting browser version in local. Safari,Chrome and Firefox
This PR covers the implementation for getting browser version in container
-Includes methods added for the same in browserDriverManager
The browser version fetched are stored in variable as of now in setupBrowserDriver().