-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Selenium: add method for switching from Dashboard to IDE frame in SeleniumWebDriverHelper class #9262
Conversation
…omDashboardToIdeFrame
…omDashboardToIdeFrame
…omDashboardToIdeFrame
@@ -57,6 +58,7 @@ | |||
@Inject private Dashboard dashboard; | |||
@Inject private WorkspaceDetails workspaceDetails; | |||
@Inject private SeleniumWebDriver seleniumWebDriver; |
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.
seleniumWebDriver looks redundant in class.
@@ -44,6 +45,7 @@ | |||
private String gitHubUsername; | |||
|
|||
@Inject private SeleniumWebDriver seleniumWebDriver; |
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.
seleniumWebDriver looks redundant in class.
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.
SeleniumWebDriver is needed in some factory tests for factory url opening(TestFactory.open(seleniumWebDriver)).
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.
Looks like an issue. It seems we can easily reuse field TestFactory#seleniumWebDriver
inside the open()
method instead of argument seleniumWebDriver
.
@@ -85,7 +87,7 @@ public void checkOpenFileFeatureTest() throws Exception { | |||
String currentWin = seleniumWebDriver.getWindowHandle(); | |||
seleniumWebDriver.switchToNoneCurrentWindow(currentWin); |
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.
We can move SeleniumWebDriver#switchToNoneCurrentWindow(currentWin)
method to the SeleniumWebDriverHelper as well, can't we?
SeleniumWebDriver seleniumWebDriver; field becomes redundant in the most test classes after refactoring. Please, remove such fields. |
It worth to move |
@@ -766,4 +768,21 @@ public void waitAndSwitchToFrame(WebElement frame, int timeout) { | |||
public void waitAndSwitchToFrame(WebElement frame) { | |||
waitAndSwitchToFrame(frame, DEFAULT_TIMEOUT); | |||
} | |||
|
|||
/** Switch to IDE frame and wait that Project Explorer is visible */ | |||
public void switchFromDashboardAndWaitProjectExplorer() { |
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.
Please create an override of this method with a custom timeout in argument
public void switchFromDashboardAndWaitProjectExplorer(int timeout)
@@ -766,4 +768,21 @@ public void waitAndSwitchToFrame(WebElement frame, int timeout) { | |||
public void waitAndSwitchToFrame(WebElement frame) { | |||
waitAndSwitchToFrame(frame, DEFAULT_TIMEOUT); | |||
} | |||
|
|||
/** Switch to IDE frame and wait that Project Explorer is visible */ |
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.
Switch -> Switches ; wait -> waits
@@ -766,4 +768,21 @@ public void waitAndSwitchToFrame(WebElement frame, int timeout) { | |||
public void waitAndSwitchToFrame(WebElement frame) { | |||
waitAndSwitchToFrame(frame, DEFAULT_TIMEOUT); | |||
} | |||
|
|||
/** Switch to IDE frame and wait that Project Explorer is visible */ | |||
public void switchFromDashboardAndWaitProjectExplorer() { |
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.
And about method name. What do you think about
switchToIdeFrameAndWaitAvailability
…omDashboardToIdeFrame
…deFrameAndWaitAvailability
ci-test |
ci-test build report: |
ci-test |
ci-test build report: |
ci-test |
ci-test build report: |
@@ -83,7 +85,7 @@ public void createAndDeleteProjectTest() throws ExecutionException, InterruptedE | |||
newWorkspace.clickOnCreateButtonAndOpenInIDE(); | |||
|
|||
String dashboardWindow = seleniumWebDriver.getWindowHandle(); |
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.
We could rework seleniumWebDriverHelper.switchToIdeFrameAndWaitAvailability()
to return dashboardWindow
* | ||
* @param currentWindowHandler | ||
*/ | ||
public void switchToNoneCurrentWindow(String currentWindowHandler) { |
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.
Let's rename the method to make its propose more obvious: switchToNextWindow()
.
Argument currentWindowHandler
is also misleading so as it seems could be any non-current window handler. So, lets rename the argument to be more precise, for example: windowHandlerToSwitchFrom
.
ci-test |
ci-test build report: |
What does this PR do?
This PR adds switchFromDashboardAndWaitProjectExplorer method for switching from Dashboard to IDE frame in SeleniumWebDriverHelper class.
What issues does this PR fix or reference?
#9100