Test: Reset all cluster if a test hit a failure #6734

s1monw · 2014-07-04T12:59:02Z

No description provided.

bleskes · 2014-07-04T13:06:26Z

src/test/java/org/elasticsearch/test/ElasticsearchIntegrationTest.java

@@ -27,6 +27,7 @@
 import com.google.common.collect.Lists;
 import org.apache.lucene.store.StoreRateLimiting;
 import org.apache.lucene.util.AbstractRandomizedTest;
+import org.apache.lucene.util.FailureMarker;


I think this is not needed

I will fix that..

bleskes · 2014-07-04T13:08:19Z

LGTM but I'd love someone who knowns the test failure semantics / clean up code flow to have a look. I don't know that area.

s1monw · 2014-07-04T13:22:04Z

I think this is the only way to do it actually so not sure if we have much of a choice here..

Closes elastic#6734

Closes #6734

dadoonet · 2014-08-01T07:48:00Z

src/test/java/org/elasticsearch/test/ElasticsearchIntegrationTest.java

@@ -543,16 +543,19 @@ protected final void afterInternal() throws IOException {
            }
            throw e;
        } finally {
-            if (!success) {
+            if (!success || suiteFailureMarker.hadFailures()) {


@s1monw With that change, when running a suite which had a lot of tests, ElasticsearchRestTests, when you have a failure in one test, all other remaining tests will restart a full cluster.
We should only restart the cluster once until we get another issue.

I'm experiencing the same @s1monw . I had missed this change and I would like to better understand the rationale behind it. Why do we have to restart the global cluster if a test fails within the suite? Also, this seems wrong as if there's one failure in the suite we restart the global cluster for all the subsequent tests...this slows down all the subsequent tests quite a lot (e.g. think of REST tests in network mode).

@dadoonet @javanna this is due to failures on a node-level. The circiutBreaker for instance will fail for all subsequent tests. So I rather take the slowdown than a false positive

In elastic#7723 we removed the `updateAddresses` method from `RestClient` under the assumption that the addresses never change during the suite execution, as REST tests rely on the global cluster. Due to elastic#6734 we restart the global cluster though before each test if there was a failure in the suite. If that happens we do need to make sure that the REST client points to the proper nodes. What was missing before was the http call to verify the es version every time the addresses change, which we do now since we effectively re-initialize the REST client when needed (if the http addresses have changed).

In #7723 we removed the `updateAddresses` method from `RestClient` under the assumption that the addresses never change during the suite execution, as REST tests rely on the global cluster. Due to #6734 we restart the global cluster though before each test if there was a failure in the suite. If that happens we do need to make sure that the REST client points to the proper nodes. What was missing before was the http call to verify the es version every time the addresses change, which we do now since we effectively re-initialize the REST client when needed (if the http addresses have changed). Closes #7737

In elastic#7723 we removed the `updateAddresses` method from `RestClient` under the assumption that the addresses never change during the suite execution, as REST tests rely on the global cluster. Due to elastic#6734 we restart the global cluster though before each test if there was a failure in the suite. If that happens we do need to make sure that the REST client points to the proper nodes. What was missing before was the http call to verify the es version every time the addresses change, which we do now since we effectively re-initialize the REST client when needed (if the http addresses have changed). Closes elastic#7737

s1monw added test labels Jul 4, 2014

bleskes reviewed Jul 4, 2014
View reviewed changes

s1monw removed the review label Jul 4, 2014

[TEST] Reset all cluster if a test hit a failure

1e0506b

Closes elastic#6734

s1monw added a commit that referenced this pull request Jul 4, 2014

[TEST] Reset all cluster if a test hit a failure

943c4a9

Closes #6734

s1monw merged commit 1e0506b into elastic:master Jul 4, 2014

clintongormley changed the title ~~[TEST] Reset all cluster if a test hit a failure~~ Test: Reset all cluster if a test hit a failure Jul 16, 2014

dadoonet reviewed Aug 1, 2014
View reviewed changes

javanna mentioned this pull request Sep 16, 2014

[TEST] Update REST client before each test in our REST tests #7737

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Test: Reset all cluster if a test hit a failure #6734

Test: Reset all cluster if a test hit a failure #6734

s1monw commented Jul 4, 2014

bleskes Jul 4, 2014

s1monw Jul 4, 2014

bleskes commented Jul 4, 2014

s1monw commented Jul 4, 2014

dadoonet Aug 1, 2014

javanna Sep 15, 2014

s1monw Sep 16, 2014

Test: Reset all cluster if a test hit a failure #6734

Test: Reset all cluster if a test hit a failure #6734

Conversation

s1monw commented Jul 4, 2014

bleskes Jul 4, 2014

Choose a reason for hiding this comment

s1monw Jul 4, 2014

Choose a reason for hiding this comment

bleskes commented Jul 4, 2014

s1monw commented Jul 4, 2014

dadoonet Aug 1, 2014

Choose a reason for hiding this comment

javanna Sep 15, 2014

Choose a reason for hiding this comment

s1monw Sep 16, 2014

Choose a reason for hiding this comment