From faaa671fb624651c9d92b38c8de62ea552ba15ec Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Thu, 22 Dec 2016 20:08:02 -0500 Subject: [PATCH] Enable assertions in integration tests When starting a standalone cluster, we do not able assertions. This is problematic because it means that we miss opportunities to catch bugs. This commit enables assertions for standalone integration tests, and fixes a couple bugs that were uncovered by enabling these. Relates #22334 --- .../gradle/test/ClusterConfiguration.groovy | 8 +++++--- .../elasticsearch/index/engine/InternalEngine.java | 11 +++++------ .../org/elasticsearch/rest/BytesRestResponse.java | 7 ++++--- .../action/admin/indices/RestGetMappingAction.java | 2 ++ 4 files changed, 16 insertions(+), 12 deletions(-) diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/ClusterConfiguration.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/ClusterConfiguration.groovy index ca4957f7a6c..57adaa2576d 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/ClusterConfiguration.groovy +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/ClusterConfiguration.groovy @@ -72,9 +72,11 @@ class ClusterConfiguration { boolean useMinimumMasterNodes = true @Input - String jvmArgs = "-Xms" + System.getProperty('tests.heap.size', '512m') + - " " + "-Xmx" + System.getProperty('tests.heap.size', '512m') + - " " + System.getProperty('tests.jvm.argline', '') + String jvmArgs = "-ea" + + " " + "-Xms" + System.getProperty('tests.heap.size', '512m') + + " " + "-Xmx" + System.getProperty('tests.heap.size', '512m') + + " " + System.getProperty('tests.jvm.argline', '') + /** * A closure to call which returns the unicast host to connect to for cluster formation. diff --git a/core/src/main/java/org/elasticsearch/index/engine/InternalEngine.java b/core/src/main/java/org/elasticsearch/index/engine/InternalEngine.java index e142ac0f6f4..058ed0a19fc 100644 --- a/core/src/main/java/org/elasticsearch/index/engine/InternalEngine.java +++ b/core/src/main/java/org/elasticsearch/index/engine/InternalEngine.java @@ -592,15 +592,14 @@ public class InternalEngine extends Engine { private boolean assertSequenceNumber(final Engine.Operation.Origin origin, final long seqNo) { if (engineConfig.getIndexSettings().getIndexVersionCreated().before(Version.V_6_0_0_alpha1_UNRELEASED) && origin == Operation.Origin.LOCAL_TRANSLOG_RECOVERY) { // legacy support - assert seqNo == SequenceNumbersService.UNASSIGNED_SEQ_NO : "old op recovering but it already has a seq no." + - " index version: " + engineConfig.getIndexSettings().getIndexVersionCreated() + ". seq no: " + seqNo; + assert seqNo == SequenceNumbersService.UNASSIGNED_SEQ_NO : "old op recovering but it already has a seq no.;" + + " index version: " + engineConfig.getIndexSettings().getIndexVersionCreated() + ", seqNo: " + seqNo; } else if (origin == Operation.Origin.PRIMARY) { // sequence number should not be set when operation origin is primary - assert seqNo == SequenceNumbersService.UNASSIGNED_SEQ_NO : "primary ops should never have an assigned seq no. got: " + seqNo; - } else { + assert seqNo == SequenceNumbersService.UNASSIGNED_SEQ_NO : "primary ops should never have an assigned seq no.; seqNo: " + seqNo; + } else if (engineConfig.getIndexSettings().getIndexVersionCreated().onOrAfter(Version.V_6_0_0_alpha1_UNRELEASED)) { // sequence number should be set when operation origin is not primary - assert seqNo >= 0 : "recovery or replica ops should have an assigned seq no. origin: " + origin + - " index version: " + engineConfig.getIndexSettings().getIndexVersionCreated(); + assert seqNo >= 0 : "recovery or replica ops should have an assigned seq no.; origin: " + origin; } return true; } diff --git a/core/src/main/java/org/elasticsearch/rest/BytesRestResponse.java b/core/src/main/java/org/elasticsearch/rest/BytesRestResponse.java index 7af8249bf2e..ba952e23c23 100644 --- a/core/src/main/java/org/elasticsearch/rest/BytesRestResponse.java +++ b/core/src/main/java/org/elasticsearch/rest/BytesRestResponse.java @@ -89,9 +89,10 @@ public class BytesRestResponse extends RestResponse { this.content = BytesArray.EMPTY; this.contentType = TEXT_CONTENT_TYPE; } else { - XContentBuilder builder = convert(channel, status, e); - this.content = builder.bytes(); - this.contentType = builder.contentType().mediaType(); + try (final XContentBuilder builder = convert(channel, status, e)) { + this.content = builder.bytes(); + this.contentType = builder.contentType().mediaType(); + } } if (e instanceof ElasticsearchException) { copyHeaders(((ElasticsearchException) e)); diff --git a/core/src/main/java/org/elasticsearch/rest/action/admin/indices/RestGetMappingAction.java b/core/src/main/java/org/elasticsearch/rest/action/admin/indices/RestGetMappingAction.java index 684a54b7f7e..69bbe47ecd4 100644 --- a/core/src/main/java/org/elasticsearch/rest/action/admin/indices/RestGetMappingAction.java +++ b/core/src/main/java/org/elasticsearch/rest/action/admin/indices/RestGetMappingAction.java @@ -72,8 +72,10 @@ public class RestGetMappingAction extends BaseRestHandler { if (indices.length != 0 && types.length != 0) { return new BytesRestResponse(OK, builder.startObject().endObject()); } else if (indices.length != 0) { + builder.close(); return new BytesRestResponse(channel, new IndexNotFoundException(indices[0])); } else if (types.length != 0) { + builder.close(); return new BytesRestResponse(channel, new TypeMissingException("_all", types[0])); } else { return new BytesRestResponse(OK, builder.startObject().endObject());