Skip to content
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

89 [QA] lead reinition integration test #233

Merged
merged 3 commits into from
May 2, 2017
Merged

89 [QA] lead reinition integration test #233

merged 3 commits into from
May 2, 2017

Conversation

ex00
Copy link
Collaborator

@ex00 ex00 commented Apr 28, 2017

fixed #89

@ex00
Copy link
Collaborator Author

ex00 commented Apr 28, 2017

will merge after #235

@@ -72,6 +72,12 @@ public void initializeResources() throws SQLException {
@AfterMethod
public void cleanupResources() throws SQLException {
TEST_NUMBER++;
//todo issue #232 fix increment topic partition name for tests on milty jvm
Copy link
Collaborator

@Desperus Desperus May 2, 2017

Choose a reason for hiding this comment

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

Typo: should be "multi" JVM

processes = processes.stream().filter(Process::isAlive).collect(Collectors.toList());
try {
for (int gridNumber = processes.size(); gridNumber < clusterSize; gridNumber++) {
processes.add(startJVM("node-" + gridNumber, IgniteStarter.class));
for (int gridNumber = 0; gridNumber < count && processes.size() < clusterSize; gridNumber++) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

In which cases count != clusterSize ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

it is restriction on top for number of waking up server nodes

writeToCache.stop();
long expectedLastDenseCommitted = writeToCache.getCount() - 1;

//wait what lead state saved
Copy link
Collaborator

Choose a reason for hiding this comment

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

"wait that lead state was saved" will be more correct

Copy link
Collaborator

Choose a reason for hiding this comment

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

"Wait for lead state to be saved" possibly?

@@ -72,6 +72,12 @@ public void initializeResources() throws SQLException {
@AfterMethod
public void cleanupResources() throws SQLException {
TEST_NUMBER++;
//todo issue #232 fix increment topic partition name for tests on milty jvm
/* possible solution
Copy link
Collaborator

Choose a reason for hiding this comment

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

May be remove this commented out code?

public void waitStartServerNodes() {
IgniteCluster cluster = clientNode.cluster();
do {
Uninterruptibles.sleepUninterruptibly(AWAIT_TIME, TimeUnit.MILLISECONDS);
Copy link
Collaborator

@YevIgn YevIgn May 2, 2017

Choose a reason for hiding this comment

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

May be consider using Awaitility for such tasks - https://github.com/awaitility/awaitility? - as we already have a bunch of them - Discussion.

writeTask.interrupt();
}

public long getCount() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why long if count was declared as int?

// this test work if run it separate only
//@Test(timeOut = TEST_TIMEOUT)
public void leadChangedStateAfterDied() throws InterruptedException {
awaitStartAllServerNodes();
Copy link
Collaborator

Choose a reason for hiding this comment

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

May be add a couple of newlines in this method to split it in steps?

@ex00 ex00 merged commit 4260679 into develop May 2, 2017
@ex00 ex00 removed the in progress label May 2, 2017
@ex00 ex00 deleted the feature/89 branch May 2, 2017 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants