Skip to content

Selenium: Revise and update "Dashboard" selenium tests in "Workspaces" section ("Workspace List" and "New Workspace" pages).#9997

Merged
Ohrimenko1988 merged 74 commits intomasterfrom
selen-dashboard-ws
Jun 19, 2018
Merged

Selenium: Revise and update "Dashboard" selenium tests in "Workspaces" section ("Workspace List" and "New Workspace" pages).#9997
Ohrimenko1988 merged 74 commits intomasterfrom
selen-dashboard-ws

Conversation

@Ohrimenko1988
Copy link
Copy Markdown
Contributor

@Ohrimenko1988 Ohrimenko1988 commented Jun 11, 2018

What does this PR do?

Revise and update "Dashboard" selenium tests in "Workspaces" section ("Workspace List" and "New Workspace" pages).

What issues does this PR fix or reference?

Issue: #9652

Release Notes

Docs PR

Ohrimenko1988 and others added 30 commits May 10, 2018 14:34
newWorkspace.selectStack(DOT_NET);
newWorkspace.waitStackSelected(DOT_NET);
Assert.assertEquals(newWorkspace.getWorkspaceNameValue(), TEST_BLANK_WORKSPACE_NAME);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about static import?

}

public boolean equals(Object obj) {
WorkspaceListItem itemForCompare = (WorkspaceListItem) obj;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May be using Generic more preferable in this case?

Copy link
Copy Markdown
Contributor

@dmytro-ndp dmytro-ndp Jun 18, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generics hardly ever help to simplify the code so as we are overriding Object#equals(Object) method here.
But we could make this code safer if we are checking if obj is an instance of WorkspaceListItem as follow https://stackoverflow.com/a/2316239/2556329
And also @Override annotation is missed here as well.

@Ohrimenko1988: please, make sure there are no new warnings before merging into the master like the follow:

/home/ndp/projects/che/selenium/che-selenium-test/src/main/java/org/eclipse/che/selenium/pageobject/dashboard/workspaces/Workspaces.java:437: warning: [EqualsHashCode] Classes that override equals should also override hashCode.
    public boolean equals(Object obj) {
                   ^
    (see http://errorprone.info/bugpattern/EqualsHashCode)

Along with it, it would be grateful if you fix another warning as well:

/home/ndp/projects/che/selenium/che-selenium-test/src/test/java/org/eclipse/che/selenium/dashboard/workspaces/AddOrImportProjectFormTest.java:70: warning: [MutableConstantField] Constant field declarations should use the immutable type (such as ImmutableList) instead of the general collection interface type (such as List)
  private static final Map<String, String> EXPECTED_SAMPLES_WITH_DESCRIPTIONS =
                          ^
    (see http://errorprone.info/bugpattern/MutableConstantField)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dmytro-ndp Thank you for so complete clarification.

"zend");

private static final List<String> EXPECTED_ALL_STACKS =
Arrays.asList(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May be make sense put long lists into resources for making the code more shorter?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my opinion, in this case, of course, it makes a code shorter, but more complicated.

@codenvy-ci
Copy link
Copy Markdown

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

@Ohrimenko1988
Copy link
Copy Markdown
Contributor Author

ci-test

@codenvy-ci
Copy link
Copy Markdown

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

@Ohrimenko1988
Copy link
Copy Markdown
Contributor Author

ci-test

@codenvy-ci
Copy link
Copy Markdown

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

@Ohrimenko1988
Copy link
Copy Markdown
Contributor Author

ci-test

@codenvy-ci
Copy link
Copy Markdown

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

@Ohrimenko1988
Copy link
Copy Markdown
Contributor Author

ci-build

}

public void waitWorkspaceCheckboxEnabled(String workspaceName) {
webDriverWaitFactory
Copy link
Copy Markdown
Contributor

@dmytro-ndp dmytro-ndp Jun 19, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like we can simplify the code a bit if we will create and user method SeleniumWebDriverHelper#waitCondition(ExpectedCondition<Boolean> expectedCondition)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have created an issue for this task #10105

@codenvy-ci
Copy link
Copy Markdown

@Ohrimenko1988 Ohrimenko1988 merged commit 4ce5a6f into master Jun 19, 2018
@Ohrimenko1988 Ohrimenko1988 deleted the selen-dashboard-ws branch June 19, 2018 15:06
@benoitf 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 Jun 20, 2018
@benoitf benoitf added this to the 6.8.0 milestone Jun 20, 2018
hbhargav pushed a commit to hbhargav/che that referenced this pull request Dec 5, 2018
…" section ("Workspace List" and "New Workspace" pages). (eclipse-che#9997)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/task Internal things, technical debt, and to-do tasks to be performed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants