Rework testing of Find Text Feature results presentation #7355

SkorikSergey · 2017-11-14T10:41:38Z

What does this PR do?

We need rework the FindTextFeatureTest selenium test:

Because checkTextResultsPagination test method of FindTextFeatureTest selenium test fails by Results of the Find Text feature are displayed in different order in CHE and OCP #7055 in start of the method and don't check other part of feature we need rework it to check count of a page occurrences in the end of the test method.
Checking that sums of found occurrences and files from all pages are correct.

What issues does this PR fix or reference?

#7327

… occurrences and files

…orer(checkTextResultsPagination())

tolusha · 2017-11-14T10:52:54Z

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

@@ -491,10 +494,12 @@ public void clickOnNextPageButton() {
  }

  public Boolean checkNextPageButtonIsEnabled() {
+    WaitUtils.sleepQuietly(1);


If possible it is better to avoid WaitUtils.sleepQuietly(1);

visibilityOfElementLocated would work well here

The next and previous page buttons are always visible in the Find Text Info panel and timeout is needed for waiting that the buttons can change their status after clicking(enabled or disabled).

I guess there is a way :)

dmytro-ndp · 2017-11-14T10:56:41Z

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

@@ -506,4 +511,21 @@ public void openFileNodeByDoubleClick(String pathToFile) {
        .click();
    actionsFactory.createAction(seleniumWebDriver).doubleClick().perform();
  }
+
+  /** get number of found occurrences in a page */


I suggest you to create data value class to encapsulate each number:

SearchFileResult(String result) ----------------------- private final int foundOnPage; private final int occurrenceOnPage; private final int totalFound; ----------------------- public int getFoundOnPage() public int getOccurranceOnPage() public int getTotalFound() -----------------------

This object SearchFileResult being returned from the dedicated method of FindText class allows to get all required data by one method call, and to have logic to parse the search results string in one most suitable place, that is to comply the Single Responsibility Principle.

dmytro-ndp · 2017-11-14T10:59:47Z

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

+
+  /** get number of found occurrences in a page */
+  public int getFoundOccurrencesNumberOnPage() {
+    List<String> results = new ArrayList<>(Arrays.asList(getResults().split(" ")));


We don't need to convert from array to list and back here, so as split() command returns String[].
You can use this command here instead:

return Integer.parseInt(getResults().split(" ")[0]);

dmytro-ndp · 2017-11-14T11:02:24Z

...-selenium-test/src/test/java/org/eclipse/che/selenium/miscellaneous/FindTextFeatureTest.java

-    // check results on second page and the previous page button is enabled
-    Assert.assertEquals(findText.getResults(), resultsOnSecondPage);
+    // Check move page buttons status on the second page
+    Assert.assertTrue(findText.checkNextPageButtonIsEnabled());


It's better to use static import here to improve readability.

…ndTextFeatureTest

dmytro-ndp · 2017-11-16T14:54:39Z

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

-    return Integer.parseInt(getResults().split(" ")[0]);
-  }
+  public static class SearchFileResult {
+    private int occurrencesFoundOnPage;


don't forget to make them final

dmytro-ndp · 2017-11-16T15:26:39Z

...-selenium-test/src/test/java/org/eclipse/che/selenium/miscellaneous/FindTextFeatureTest.java

-    sumOfFoundOccurrences += findText.getFoundOccurrencesNumberOnPage();
+    Assert.assertTrue(findText.checkNextPageButtonIsEnabled());
+    Assert.assertFalse(findText.checkPreviousPageButtonIsEnabled());
+    sumOfFoundFiles += findText.getResults().getFoundFilesOnPage();


It's better to set the returning by findText.getResults() object to the variable and then use it to get the numbers, to avoid multiple execution of commands inside the constructor SearchFileResult().

dmytro-ndp · 2017-11-16T15:29:47Z

...-selenium-test/src/test/java/org/eclipse/che/selenium/miscellaneous/FindTextFeatureTest.java

-    sumOfFoundOccurrences += findText.getFoundOccurrencesNumberOnPage();
+    Assert.assertTrue(findText.checkNextPageButtonIsEnabled());
+    Assert.assertFalse(findText.checkPreviousPageButtonIsEnabled());
+    sumOfFoundFiles += findText.getResults().getFoundFilesOnPage();


What about renaming findText variable to the more suitable one like "testFinder", "findTextPage"?

…ndTextFeatureTest

dmytro-ndp · 2017-11-17T12:53:31Z

ci-build

codenvy-ci · 2017-11-17T13:42:33Z

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

Sergey Skorik added 3 commits November 13, 2017 18:11

in checkTextResultPagination test method added checking sums of found…

01a4544

… occurrences and files

added comments

7d575f6

fixed problem with selecting project after import in the Project Expl…

1faca97

…orer(checkTextResultsPagination())

SkorikSergey added kind/task Internal things, technical debt, and to-do tasks to be performed. status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. team/production labels Nov 14, 2017

SkorikSergey requested review from dmytro-ndp and tolusha November 14, 2017 10:41

SkorikSergey requested a review from vparfonov as a code owner November 14, 2017 10:41

tolusha reviewed Nov 14, 2017

View reviewed changes

dmytro-ndp reviewed Nov 14, 2017

View reviewed changes

Sergey Skorik added 4 commits November 15, 2017 10:39

Merge branch 'master' of https://github.com/eclipse/che into reworkFi…

03e043d

…ndTextFeatureTest

reworked methods for getting fint text results

55a0535

reworked to use static import for Assert

2ee8146

reworked method for getting find text results

460f089

dmytro-ndp reviewed Nov 16, 2017

View reviewed changes

refactoring

fabb519

dmytro-ndp reviewed Nov 16, 2017

View reviewed changes

SkorikSergey added 2 commits November 17, 2017 11:45

Merge branch 'master' of https://github.com/eclipse/che into reworkFi…

0bb146b

…ndTextFeatureTest

renamed variable of FindText selenium page object

1df48d7

dmytro-ndp approved these changes Nov 17, 2017

View reviewed changes

tolusha approved these changes Nov 17, 2017

View reviewed changes

SkorikSergey merged commit 940e848 into master Nov 17, 2017

SkorikSergey deleted the reworkFindTextFeatureTest branch November 17, 2017 13:47

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 Nov 17, 2017

benoitf added this to the 5.21.0 milestone Nov 17, 2017

SkorikSergey mentioned this pull request Nov 17, 2017

[CHE 6] Rework testing of Find Text Feature results presentation #7436

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rework testing of Find Text Feature results presentation #7355

Rework testing of Find Text Feature results presentation #7355

SkorikSergey commented Nov 14, 2017

tolusha Nov 14, 2017

tolusha Nov 14, 2017

SkorikSergey Nov 15, 2017

tolusha Nov 15, 2017

dmytro-ndp Nov 14, 2017 •

edited

Loading

dmytro-ndp Nov 15, 2017 •

edited

Loading

dmytro-ndp Nov 14, 2017

dmytro-ndp Nov 14, 2017

dmytro-ndp Nov 16, 2017

dmytro-ndp Nov 16, 2017 •

edited

Loading

dmytro-ndp Nov 16, 2017 •

edited

Loading

dmytro-ndp commented Nov 17, 2017

codenvy-ci commented Nov 17, 2017

Rework testing of Find Text Feature results presentation #7355

Rework testing of Find Text Feature results presentation #7355

Conversation

SkorikSergey commented Nov 14, 2017

What does this PR do?

What issues does this PR fix or reference?

tolusha Nov 14, 2017

Choose a reason for hiding this comment

tolusha Nov 14, 2017

Choose a reason for hiding this comment

SkorikSergey Nov 15, 2017

Choose a reason for hiding this comment

tolusha Nov 15, 2017

Choose a reason for hiding this comment

dmytro-ndp Nov 14, 2017 • edited Loading

Choose a reason for hiding this comment

dmytro-ndp Nov 15, 2017 • edited Loading

Choose a reason for hiding this comment

dmytro-ndp Nov 14, 2017

Choose a reason for hiding this comment

dmytro-ndp Nov 14, 2017

Choose a reason for hiding this comment

dmytro-ndp Nov 16, 2017

Choose a reason for hiding this comment

dmytro-ndp Nov 16, 2017 • edited Loading

Choose a reason for hiding this comment

dmytro-ndp Nov 16, 2017 • edited Loading

Choose a reason for hiding this comment

dmytro-ndp commented Nov 17, 2017

codenvy-ci commented Nov 17, 2017

dmytro-ndp Nov 14, 2017 •

edited

Loading

dmytro-ndp Nov 15, 2017 •

edited

Loading

dmytro-ndp Nov 16, 2017 •

edited

Loading

dmytro-ndp Nov 16, 2017 •

edited

Loading