From d75571ff0f2994f26dfdf3d910e4bf0e57df1fb6 Mon Sep 17 00:00:00 2001 From: Luca Cavanna Date: Wed, 1 Apr 2020 11:21:42 +0200 Subject: [PATCH] [TEST] rename AsyncSearchActionTests to IT and move it out of unit tests (#54520) `AsyncSearchActionTests` currently fails quite often. That is since the introduction of `RestSubmitAsyncSearchActionTests` which indirectly manipulates the channels being tracked in `RestCancellableNodeClient`. There are channels left in the map after `RestSubmitAsyncSearchActionTests` is run, and later `AsyncSearchActionTests` checks that there are no channels in the map which makes each test method fail. This is particularily hard to reproduce as the order in which tests are run appears to be platform dependent. The test cluster assertion that there are no channels in the map only makes sense in the context of internal cluster tests, while there may be collisions with unit tests that register http channels as part of their testing. This can be solved by renaming `AsyncSearchActionTests` to `AsyncSearchActionIT`. This way it won't be run as part of unit tests but rather within another JVM where the number of channels is `0` and such assumption holds, because there are no expected manual manipulation of the channels. Relates to #54180 --- x-pack/plugin/async-search/build.gradle | 15 +++++++++++++-- ...hActionTests.java => AsyncSearchActionIT.java} | 2 +- .../search/RestSubmitAsyncSearchActionTests.java | 8 ++++---- 3 files changed, 18 insertions(+), 7 deletions(-) rename x-pack/plugin/async-search/src/test/java/org/elasticsearch/xpack/search/{AsyncSearchActionTests.java => AsyncSearchActionIT.java} (99%) diff --git a/x-pack/plugin/async-search/build.gradle b/x-pack/plugin/async-search/build.gradle index 8aac2b9f885..156b6374e82 100644 --- a/x-pack/plugin/async-search/build.gradle +++ b/x-pack/plugin/async-search/build.gradle @@ -18,8 +18,6 @@ archivesBaseName = 'x-pack-async-search' compileJava.options.compilerArgs << "-Xlint:-rawtypes" compileTestJava.options.compilerArgs << "-Xlint:-rawtypes" -integTest.enabled = false - // add all sub-projects of the qa sub-project gradle.projectsEvaluated { project.subprojects @@ -40,3 +38,16 @@ dependencies { dependencyLicenses { ignoreSha 'x-pack-core' } + +integTest.enabled = false + +// Instead we create a separate task to run the +// tests based on ESIntegTestCase +task internalClusterTest(type: Test) { + description = 'Java fantasy integration tests' + mustRunAfter test + + include '**/*IT.class' +} + +check.dependsOn internalClusterTest diff --git a/x-pack/plugin/async-search/src/test/java/org/elasticsearch/xpack/search/AsyncSearchActionTests.java b/x-pack/plugin/async-search/src/test/java/org/elasticsearch/xpack/search/AsyncSearchActionIT.java similarity index 99% rename from x-pack/plugin/async-search/src/test/java/org/elasticsearch/xpack/search/AsyncSearchActionTests.java rename to x-pack/plugin/async-search/src/test/java/org/elasticsearch/xpack/search/AsyncSearchActionIT.java index a3fc60b79f2..e507ca0e70f 100644 --- a/x-pack/plugin/async-search/src/test/java/org/elasticsearch/xpack/search/AsyncSearchActionTests.java +++ b/x-pack/plugin/async-search/src/test/java/org/elasticsearch/xpack/search/AsyncSearchActionIT.java @@ -35,7 +35,7 @@ import static org.hamcrest.Matchers.greaterThanOrEqualTo; import static org.hamcrest.Matchers.lessThanOrEqualTo; // TODO: add tests for keepAlive and expiration -public class AsyncSearchActionTests extends AsyncSearchIntegTestCase { +public class AsyncSearchActionIT extends AsyncSearchIntegTestCase { private String indexName; private int numShards; diff --git a/x-pack/plugin/async-search/src/test/java/org/elasticsearch/xpack/search/RestSubmitAsyncSearchActionTests.java b/x-pack/plugin/async-search/src/test/java/org/elasticsearch/xpack/search/RestSubmitAsyncSearchActionTests.java index 748c6d12acc..579e16282d2 100644 --- a/x-pack/plugin/async-search/src/test/java/org/elasticsearch/xpack/search/RestSubmitAsyncSearchActionTests.java +++ b/x-pack/plugin/async-search/src/test/java/org/elasticsearch/xpack/search/RestSubmitAsyncSearchActionTests.java @@ -72,10 +72,10 @@ public class RestSubmitAsyncSearchActionTests extends ESTestCase { dispatchRequest(submitAsyncRestRequest); SubmitAsyncSearchRequest submitRequest = (SubmitAsyncSearchRequest) lastCapturedRequest; assertEquals(TimeValue.timeValueSeconds(1), submitRequest.getWaitForCompletionTimeout()); - assertEquals(false, submitRequest.isKeepOnCompletion()); + assertFalse(submitRequest.isKeepOnCompletion()); assertEquals(TimeValue.timeValueDays(5), submitRequest.getKeepAlive()); // check parameters we implicitly set in the SubmitAsyncSearchRequest ctor - assertEquals(false, submitRequest.getSearchRequest().isCcsMinimizeRoundtrips()); + assertFalse(submitRequest.getSearchRequest().isCcsMinimizeRoundtrips()); assertEquals(5, submitRequest.getSearchRequest().getBatchedReduceSize()); assertEquals(true, submitRequest.getSearchRequest().requestCache()); assertEquals(1, submitRequest.getSearchRequest().getPreFilterShardSize().intValue()); @@ -92,8 +92,8 @@ public class RestSubmitAsyncSearchActionTests extends ESTestCase { boolean requestCache = randomBoolean(); doTestParameter("request_cache", Boolean.toString(requestCache), requestCache, r -> r.getSearchRequest().requestCache()); - Integer batchReduceSize = randomIntBetween(2, 50); - doTestParameter("batched_reduce_size", Integer.toString(batchReduceSize), batchReduceSize, + int batchedReduceSize = randomIntBetween(2, 50); + doTestParameter("batched_reduce_size", Integer.toString(batchedReduceSize), batchedReduceSize, r -> r.getSearchRequest().getBatchedReduceSize()); }