Selenium: Rework "ProjectExplorer" page object with "SeleniumWebDriverHelper" and "WebDriverWaitFactory" #8865

Ohrimenko1988 · 2018-02-21T15:53:37Z

What does this PR do?

Rework "ProjectExplorer" page object with "SeleniumWebDriverHelper" and "WebDriverWaitFactory"

What issues does this PR fix or reference?

Issue: #8854
Issue: #8902

Release Notes

Docs PR

…nd 'WebDriverWaitFactory' classes

Ohrimenko1988 · 2018-02-21T16:01:39Z

ci-test

codenvy-ci · 2018-02-21T21:28:18Z

ci-test build report:
Build details
Test report
selenium tests report data
docker image: eclipseche/che-server:8865
https://github.com/orgs/eclipse/teams/eclipse-che-qa please check this report.

…work-projectexplorer

Ohrimenko1988 · 2018-02-22T07:01:52Z

ci-test

codenvy-ci · 2018-02-22T12:04:29Z

ci-test build report:
Build details
Test report
selenium tests report data
docker image: eclipseche/che-server:8865
https://github.com/orgs/eclipse/teams/eclipse-che-qa please check this report.

…work they to enumerations

…work-projectexplorer

Ohrimenko1988 · 2018-02-22T15:54:49Z

ci-test

codenvy-ci · 2018-02-22T21:44:33Z

ci-test build report:
Build details
Test report
selenium tests report data
docker image: eclipseche/che-server:8865
https://github.com/orgs/eclipse/teams/eclipse-che-qa please check this report.

…work-projectexplorer

dmytro-ndp · 2018-02-23T13:07:18Z

...ium/che-selenium-test/src/main/java/org/eclipse/che/selenium/pageobject/ProjectExplorer.java

-    this.seleniumWebDriverHelper = seleniumWebDriverHelper;
-    loadPageTimeout = new WebDriverWait(seleniumWebDriver, LOAD_PAGE_TIMEOUT_SEC);
-    redrawUiElementsWait = new WebDriverWait(seleniumWebDriver, REDRAW_UI_ELEMENTS_TIMEOUT_SEC);
+    this.helper = seleniumWebDriverHelper;


helper is too short name for the field, because it doesn't explain what kind of help it proposes.

I agree with you this is not very convenient but this is my point - this object uses almost in each project explorer's method, and that's why I did it shorter, and you just need one time used "F4" to see what it means and remember it for all cases.

I understood why you had shorten the name. From the same reason it could be named as just "h" :-)
But it is not good idea to force readers to find the type of field to understand what it means.
Also, it allows to involve another type of helpers in future.

Ok, I will change it

But we can get rid of SeleniumWebDriverHelper helper by moving its staff to the SeleniumWebDriver class itself. It also will make operation with page objects with different web driver instances more clear.

It also could fix issue #8902.

dmytro-ndp · 2018-02-23T13:15:23Z

There are ~2000 changes in 128 files. Those are too many changes for the one pull request with a number of different refactoring operations. As a rule, each refactoring like extracting code into the SeleniumWebDriverHelper class, or encapsulating constants into the enumeration, ought to be rearranged by separate commit to make it possible to review correctness of each refactoring operation one by one.

…work-projectexplorer

Ohrimenko1988 · 2018-02-23T17:45:43Z

ci-test

dmytro-ndp · 2018-02-27T15:11:42Z

...ium/che-selenium-test/src/main/java/org/eclipse/che/selenium/pageobject/ProjectExplorer.java

  }

-  /** select external maven Library relative module */
+  /** Select external maven Library relative module */


Select > Selects

dmytro-ndp · 2018-02-27T15:24:00Z

...ium/che-selenium-test/src/main/java/org/eclipse/che/selenium/pageobject/ProjectExplorer.java

   *
-   * @param item item form {@code SubMenuNew}
+   * @param item item from {@link


Link is outdated.

Ohrimenko1988 · 2018-02-27T16:05:35Z

ci-test

dmytro-ndp · 2018-02-27T16:14:19Z

...selenium-test/src/main/java/org/eclipse/che/selenium/pageobject/SeleniumWebDriverHelper.java

+   * @param elementLocator
+   * @param timeout waiting time in seconds
+   * @return {@code true} - if not visible,
+   *     <p>throws {@link org.openqa.selenium.TimeoutException} - if visible.


- if visible > - if visible after timeout

dmytro-ndp · 2018-02-27T16:22:01Z

...selenium-test/src/main/java/org/eclipse/che/selenium/pageobject/SeleniumWebDriverHelper.java

+   *
+   * @param elementLocator
+   * @param timeout waiting time in seconds
+   * @return {@code true} - if not visible,


returning boolean type is useless here because it can only return true, or throw timeout exception.

In this case all usages of this method need check with Assert.assertTrue()

dmytro-ndp · 2018-02-27T16:22:31Z

...selenium-test/src/main/java/org/eclipse/che/selenium/pageobject/SeleniumWebDriverHelper.java

+   * @return {@code true} - if not visible,
+   *     <p>throws {@link org.openqa.selenium.TimeoutException} - if visible.
+   */
+  public boolean waitInvisibility(By elementLocator, int timeout) {


returning boolean type is useless here because it can only return true, or throw timeout exception.

In this case all usages of this method need check with Assert.assertTrue()

dmytro-ndp · 2018-02-27T16:22:47Z

...selenium-test/src/main/java/org/eclipse/che/selenium/pageobject/SeleniumWebDriverHelper.java

+   * @return {@code true} - if not visible,
+   *     <p>throws {@link org.openqa.selenium.TimeoutException} - if visible.
+   */
+  public boolean waitInvisibility(By elementLocator) {


returning boolean type is useless here because it can only return true, or throw timeout exception.

In this case all usages of this method need check with Assert.assertTrue()

dmytro-ndp · 2018-02-27T16:23:03Z

...selenium-test/src/main/java/org/eclipse/che/selenium/pageobject/SeleniumWebDriverHelper.java

+   * @return {@code true} - if not visible,
+   *     <p>throws {@link org.openqa.selenium.TimeoutException} - if visible.
+   */
+  public boolean waitInvisibility(WebElement webElement, int timeout) {


returning boolean type is useless here because it can only return true, or throw timeout exception.

In this case all usages of this method need check with Assert.assertTrue()

What is the benefit from Assert.assertTrue(true)? It checks nothing.

Assert.assertTrue(waitInvisibility)

Actually, we can simply replace Assert.assertTrue(waitInvisibility()) by waitInvisibility() without regression because in our case Assert.assertTrue(waitInvisibility()) <-> Assert.assertTrue(true).

dmytro-ndp · 2018-02-27T16:23:19Z

...selenium-test/src/main/java/org/eclipse/che/selenium/pageobject/SeleniumWebDriverHelper.java

+   * @return {@code true} - if not visible,
+   *     <p>throws {@link org.openqa.selenium.TimeoutException} - if visible.
+   */
+  public boolean waitInvisibility(WebElement webElement) {


returning boolean type is useless here because it can only return true, or throw timeout exception.

In this case all usages of this method need check with Assert.assertTrue()

…ipse/che into selen-rework-projectexplorer

dmytro-ndp · 2018-02-27T17:36:20Z

...ium/che-selenium-test/src/main/java/org/eclipse/che/selenium/pageobject/ProjectExplorer.java

  }

  /** Clicks on project explorer tab and waits its appearance. */
-  public void openPanel() {
-    clickOnProjectExplorerTabInTheLeftPanel();
+  public void openAndWaitProjectExplorer() {


openAndWait is enough because openAndWaitProjectExplorer duplicates class name ProjectExplorer.

dmytro-ndp · 2018-02-27T17:38:36Z

...ium/che-selenium-test/src/main/java/org/eclipse/che/selenium/pageobject/ProjectExplorer.java

   *
-   * @param timeout user timeout
+   * @param timeout waiting timeout in seconds
   */
  public void waitProjectExplorer(int timeout) {


Let's name it just wait, because waitProjectExplorer duplicates class name ProjectExplorer.

Sorry, wait() can't be used indeed. waitAppearance() is possible variant instead, IMHO

dmytro-ndp · 2018-02-27T18:22:34Z

...selenium-test/src/main/java/org/eclipse/che/selenium/pageobject/SeleniumWebDriverHelper.java

@@ -156,45 +156,37 @@ public WebElement waitVisibility(WebElement webElement) {
   *
   * @param elementLocator
   * @param timeout waiting time in seconds
-   * @return {@code true} - if not visible,
-   *     <p>throws {@link org.openqa.selenium.TimeoutException} - if visible after timeout.


@throws {@link org.openqa.selenium.TimeoutException} - if is visible during timeout. could be still useful for reader.

dmytro-ndp · 2018-02-27T18:28:44Z

ci-build

dmytro-ndp · 2018-02-27T18:29:46Z

ci-test

codenvy-ci · 2018-02-27T20:12:30Z

Build success. https://ci.codenvycorp.com/job/che-pullrequests-build/4484/

codenvy-ci · 2018-02-27T23:31:08Z

ci-test build report:
Build details
Test report
selenium tests report data
docker image: eclipseche/che-server:8865
https://github.com/orgs/eclipse/teams/eclipse-che-qa please check this report.

Ohrimenko1988 added 2 commits February 21, 2018 17:40

Rework 'ProjectExplorer' page object with 'SeleniumWebDriverHelper' a…

adfcd0e

…nd 'WebDriverWaitFactory' classes

merge master and resolve conflicts

9bfa627

Ohrimenko1988 added the kind/task Internal things, technical debt, and to-do tasks to be performed. label Feb 21, 2018

Ohrimenko1988 requested review from dmytro-ndp, musienko-maxim, artaleks9 and SkorikSergey February 21, 2018 15:53

Ohrimenko1988 requested a review from vparfonov as a code owner February 21, 2018 15:53

Merge branch 'master' of https://github.com/eclipse/che into selen-re…

bc40428

…work-projectexplorer

Ohrimenko1988 added 3 commits February 22, 2018 15:01

Update javadocs to actual state and remove duplicated methods

9fe465f

Replace project explorer's context menu constants to one place and re…

32c7d60

…work they to enumerations

Merge branch 'master' of https://github.com/eclipse/che into selen-re…

7cad6f9

…work-projectexplorer

Merge branch 'master' of https://github.com/eclipse/che into selen-re…

81054dc

…work-projectexplorer

benoitf added the status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. label Feb 23, 2018

Update javadocs to actual state and remove duplicated methods

e97e893

Ohrimenko1988 changed the title ~~[WIP] Selenium: Rework "ProjectExplorer" page object with "SeleniumWebDriverHelper" and "WebDriverWaitFactory"~~ Selenium: Rework "ProjectExplorer" page object with "SeleniumWebDriverHelper" and "WebDriverWaitFactory" Feb 23, 2018

dmytro-ndp reviewed Feb 23, 2018

View reviewed changes

musienko-maxim approved these changes Feb 23, 2018

View reviewed changes

SkorikSergey approved these changes Feb 23, 2018

View reviewed changes

Ohrimenko1988 added 3 commits February 23, 2018 16:12

Remove unused objects

458be99

Rename 'helper' to 'seleniumWebDriverHelper'

6767c2a

Merge branch 'master' of https://github.com/eclipse/che into selen-re…

27a8288

…work-projectexplorer

dmytro-ndp reviewed Feb 27, 2018

View reviewed changes

Ohrimenko1988 added 3 commits February 27, 2018 17:34

Merge master and resolve conflicts

ea25b18

Add changes according to review

ded25a3

Merge master and resolve conflicts

181b2e4

dmytro-ndp reviewed Feb 27, 2018

View reviewed changes

Merge branch 'selen-rework-projectexplorer' of https://github.com/ecl…

83905a4

…ipse/che into selen-rework-projectexplorer

dmytro-ndp reviewed Feb 27, 2018

View reviewed changes

Ohrimenko1988 added 2 commits February 27, 2018 19:55

Do changes according to review

0883ed7

Do changes according to review

0f4d6a2

dmytro-ndp reviewed Feb 27, 2018

View reviewed changes

Do changes according to review

90853e4

dmytro-ndp approved these changes Feb 28, 2018

View reviewed changes

Ohrimenko1988 merged commit 1475ff9 into master Feb 28, 2018

Ohrimenko1988 deleted the selen-rework-projectexplorer branch February 28, 2018 07:48

benoitf removed the status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. label Feb 28, 2018

benoitf added this to the 6.2.0 milestone Feb 28, 2018

This was referenced Feb 28, 2018

Rework "ProjectExplorer" page object with "SeleniumWebDriverHelper" and "WebDriverWaitFactory" #8854

Closed

EditFilesWithTabsTest opens redundant chromium window which doesn't close #8902

Closed

Selenium: Rework "ProjectExplorer" page object with "SeleniumWebDriverHelper" and "WebDriverWaitFactory" #8865

Selenium: Rework "ProjectExplorer" page object with "SeleniumWebDriverHelper" and "WebDriverWaitFactory" #8865

Conversation

Ohrimenko1988 commented Feb 21, 2018 • edited Loading

What does this PR do?

What issues does this PR fix or reference?

Release Notes

Docs PR

Ohrimenko1988 commented Feb 21, 2018

codenvy-ci commented Feb 21, 2018

Ohrimenko1988 commented Feb 22, 2018

codenvy-ci commented Feb 22, 2018

Ohrimenko1988 commented Feb 22, 2018

codenvy-ci commented Feb 22, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dmytro-ndp Feb 23, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dmytro-ndp Feb 23, 2018 • edited Loading

Choose a reason for hiding this comment

dmytro-ndp commented Feb 23, 2018

Ohrimenko1988 commented Feb 23, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Ohrimenko1988 commented Feb 27, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dmytro-ndp Feb 27, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dmytro-ndp Feb 27, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dmytro-ndp commented Feb 27, 2018

dmytro-ndp commented Feb 27, 2018

codenvy-ci commented Feb 27, 2018

codenvy-ci commented Feb 27, 2018

Ohrimenko1988 commented Feb 21, 2018 •

edited

Loading

dmytro-ndp Feb 23, 2018 •

edited

Loading

dmytro-ndp Feb 23, 2018 •

edited

Loading

dmytro-ndp Feb 27, 2018 •

edited

Loading

dmytro-ndp Feb 27, 2018 •

edited

Loading