Skip to content

Selenium: Create E2E test for checking Hot update feature for wsmaster#10176

Merged
Ohrimenko1988 merged 33 commits intomasterfrom
CHE-9988
Jun 26, 2018
Merged

Selenium: Create E2E test for checking Hot update feature for wsmaster#10176
Ohrimenko1988 merged 33 commits intomasterfrom
CHE-9988

Conversation

@Ohrimenko1988
Copy link
Copy Markdown
Contributor

What does this PR do?

Create E2E test for checking Hot update feature for wsmaster

What issues does this PR fix or reference?

Issue: #9646

Release Notes

Docs PR

@Ohrimenko1988 Ohrimenko1988 added status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. kind/task Internal things, technical debt, and to-do tasks to be performed. labels Jun 25, 2018
@Ohrimenko1988 Ohrimenko1988 requested a review from vparfonov as a code owner June 25, 2018 13:33

private void waitRevision(int expectedRevision) {
webDriverWaitFactory
.get(100)
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.

100 in unclear here. It's better to use dedicated constant to express timeout.

+ "import java.util.Random;\n"
+ "\n";

private static final String EXPECTED_CHANGED_TEXT =
Copy link
Copy Markdown
Contributor

@dmytro-ndp dmytro-ndp Jun 25, 2018

Choose a reason for hiding this comment

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

looks like EXPECTED_CHANGED_TEXT = TEXT_FOR_TYPING + EXPECTED_DEFAULT_TEXT
If so, it would be clearer and simpler to defined that constant in such way.


projectExplorer.quickExpandWithJavaScript();

projectExplorer.openItemByVisibleNameInExplorer(NAME_OF_CHECKED_CLASS + ".java");
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.

We can avoid duplication if we will have dedicated constant to describe NAME_OF_CHECKED_CLASS_FILE = NAME_OF_CHECKED_CLASS + ".java"

executeRolloutUpdateCommand();

// check editor availability during rollout updating
Assert.assertEquals(cheTestSystemClient.getStatus(), SystemStatus.RUNNING);
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.

Static import

@Ohrimenko1988
Copy link
Copy Markdown
Contributor Author

ci-test

private static final int TIMEOUT_FOR_ROLLING_UPDATE_FINISH = 100;
private static final int RESTORE_IDE_AFTER_REFRESH_TIMEOUT = 10;
private static final int EXPECTED_PREFERENCES_RESPONCE_CODE = 200;
private static final String PROJECT_NAME = "default-spring-project";
Copy link
Copy Markdown
Contributor

@musienko-maxim musienko-maxim Jun 26, 2018

Choose a reason for hiding this comment

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

May be we will use all HTTP statuses from single entry point. I propose from this: javax.ws.rs.core.Response.Status.ACCEPTED. In this case we won't need any fields for storing HTTP statuses
WDYT?

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.

Good catch!

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.

Even more - we doesn't need to check response code so as in case of error HttpJsonResponse#request() throws one of the next type of exception: ServerException, UnauthorizedException, ForbiddenException, NotFoundException, ConflictException, BadRequestException.
So, just calling TestUserPreferencesServiceClient#getPreferences() would be enough to check if everything is okay with WS master server.

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.

Totally agree

@dmytro-ndp
Copy link
Copy Markdown
Contributor

@Ohrimenko1988: to run tests regularly on CI we need to divide Rollout strategy tests from RecreateUpdateStrategyTest* to make it possible to run all kind of tests separately by one command: selenium-tests.sh --test=....

The simplest way is to move those tests in dedicated packages:

  • org.eclipse.che.selenium.hotupdate.recreate
  • org.eclipse.che.selenium.hotupdate.rollout

@ghost
Copy link
Copy Markdown

ghost commented Jun 26, 2018

What is rollout update strategy btw?

@Ohrimenko1988
Copy link
Copy Markdown
Contributor Author

@eivantsov Good catch, thank you. Of course "Rolling update".

@ghost
Copy link
Copy Markdown

ghost commented Jun 26, 2018

I think it's be nice to refactor a little bit to respect K8S spec :)

Copy link
Copy Markdown
Contributor

@dmytro-ndp dmytro-ndp left a comment

Choose a reason for hiding this comment

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

fixup looks good

@Ohrimenko1988 Ohrimenko1988 merged commit ca695ee into master Jun 26, 2018
@Ohrimenko1988 Ohrimenko1988 deleted the CHE-9988 branch June 26, 2018 13:33
@Ohrimenko1988 Ohrimenko1988 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 26, 2018
@benoitf benoitf added this to the 6.8.0 milestone Jun 26, 2018
@codenvy-ci
Copy link
Copy Markdown

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

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.

5 participants