diff --git a/server/src/test/java/org/elasticsearch/cluster/coordination/JoinValidationServiceTests.java b/server/src/test/java/org/elasticsearch/cluster/coordination/JoinValidationServiceTests.java index 6df9260b2bccf..79203899b665d 100644 --- a/server/src/test/java/org/elasticsearch/cluster/coordination/JoinValidationServiceTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/coordination/JoinValidationServiceTests.java @@ -60,6 +60,7 @@ import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; +import static org.elasticsearch.action.support.ActionTestUtils.assertNoSuccessListener; import static org.hamcrest.Matchers.allOf; import static org.hamcrest.Matchers.containsString; @@ -295,17 +296,9 @@ protected void onSendRequest(long requestId, String action, TransportRequest req assertSame(node, joiningNode); assertEquals(JoinValidationService.JOIN_VALIDATE_ACTION_NAME, action); - final var listener = new ActionListener() { - @Override - public void onResponse(TransportResponse transportResponse) { - fail("should not succeed"); - } - - @Override - public void onFailure(Exception e) { - handleError(requestId, new RemoteTransportException(node.getName(), node.getAddress(), action, e)); - } - }; + final ActionListener listener = assertNoSuccessListener( + e -> handleError(requestId, new RemoteTransportException(node.getName(), node.getAddress(), action, e)) + ); try (var ignored = NamedWriteableRegistryTests.ignoringUnknownNamedWriteables(); var out = new BytesStreamOutput()) { request.writeTo(out); diff --git a/server/src/test/java/org/elasticsearch/cluster/service/MasterServiceTests.java b/server/src/test/java/org/elasticsearch/cluster/service/MasterServiceTests.java index 50030143ec354..617e1cb09c353 100644 --- a/server/src/test/java/org/elasticsearch/cluster/service/MasterServiceTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/service/MasterServiceTests.java @@ -80,6 +80,7 @@ import java.util.stream.Collectors; import static java.util.Collections.emptySet; +import static org.elasticsearch.action.support.ActionTestUtils.assertNoSuccessListener; import static org.elasticsearch.cluster.service.MasterService.MAX_TASK_DESCRIPTION_CHARS; import static org.hamcrest.Matchers.allOf; import static org.hamcrest.Matchers.contains; @@ -1041,30 +1042,22 @@ public void onFailure(Exception e) { threadContext.putHeader(testContextHeaderName, testContextHeaderValue); final var expectFailure = randomBoolean(); final var taskComplete = new AtomicBoolean(); - final var task = new Task(expectFailure, testResponseHeaderValue, new ActionListener<>() { - @Override - public void onResponse(ClusterState clusterState) { - throw new AssertionError("should not succeed"); + final var task = new Task(expectFailure, testResponseHeaderValue, assertNoSuccessListener(e -> { + assertEquals(testContextHeaderValue, threadContext.getHeader(testContextHeaderName)); + assertEquals(List.of(testResponseHeaderValue), threadContext.getResponseHeaders().get(testResponseHeaderName)); + assertThat(e, instanceOf(FailedToCommitClusterStateException.class)); + assertThat(e.getMessage(), equalTo(publicationFailedExceptionMessage)); + if (expectFailure) { + assertThat(e.getSuppressed().length, greaterThan(0)); + var suppressed = e.getSuppressed()[0]; + assertThat(suppressed, instanceOf(ElasticsearchException.class)); + assertThat(suppressed.getMessage(), equalTo(taskFailedExceptionMessage)); } - - @Override - public void onFailure(Exception e) { - assertEquals(testContextHeaderValue, threadContext.getHeader(testContextHeaderName)); - assertEquals(List.of(testResponseHeaderValue), threadContext.getResponseHeaders().get(testResponseHeaderName)); - assertThat(e, instanceOf(FailedToCommitClusterStateException.class)); - assertThat(e.getMessage(), equalTo(publicationFailedExceptionMessage)); - if (expectFailure) { - assertThat(e.getSuppressed().length, greaterThan(0)); - var suppressed = e.getSuppressed()[0]; - assertThat(suppressed, instanceOf(ElasticsearchException.class)); - assertThat(suppressed.getMessage(), equalTo(taskFailedExceptionMessage)); - } - assertNotNull(publishedState.get()); - assertNotSame(stateBeforeFailure, publishedState.get()); - assertTrue(taskComplete.compareAndSet(false, true)); - publishFailureCountdown.countDown(); - } - }); + assertNotNull(publishedState.get()); + assertNotSame(stateBeforeFailure, publishedState.get()); + assertTrue(taskComplete.compareAndSet(false, true)); + publishFailureCountdown.countDown(); + })); queue.submitTask("test", task, null); } diff --git a/test/framework/src/main/java/org/elasticsearch/action/support/ActionTestUtils.java b/test/framework/src/main/java/org/elasticsearch/action/support/ActionTestUtils.java index 187a8b6e4eab2..023305101f4c4 100644 --- a/test/framework/src/main/java/org/elasticsearch/action/support/ActionTestUtils.java +++ b/test/framework/src/main/java/org/elasticsearch/action/support/ActionTestUtils.java @@ -22,6 +22,9 @@ import java.util.Map; import java.util.concurrent.TimeUnit; +import java.util.function.Consumer; + +import static org.elasticsearch.test.ESTestCase.fail; public class ActionTestUtils { @@ -77,6 +80,27 @@ public static ActionListener assertNoFailureListener(CheckedConsumer ActionListener assertNoSuccessListener(Consumer consumer) { + return new ActionListener<>() { + @Override + public void onResponse(T result) { + fail(null, "unexpected success with result [%s] while expecting to handle failure with [%s]", result, consumer); + } + + @Override + public void onFailure(Exception e) { + try { + consumer.accept(e); + } catch (Exception e2) { + if (e2 != e) { + e2.addSuppressed(e); + } + fail(e2, "unexpected failure in onFailure handler for [%s]", consumer); + } + } + }; + } + public static ResponseListener wrapAsRestResponseListener(ActionListener listener) { return new ResponseListener() { @Override diff --git a/test/framework/src/main/java/org/elasticsearch/action/support/CancellableActionTestPlugin.java b/test/framework/src/main/java/org/elasticsearch/action/support/CancellableActionTestPlugin.java index 115ea63fb243e..dad0e3b613efb 100644 --- a/test/framework/src/main/java/org/elasticsearch/action/support/CancellableActionTestPlugin.java +++ b/test/framework/src/main/java/org/elasticsearch/action/support/CancellableActionTestPlugin.java @@ -26,6 +26,7 @@ import static org.elasticsearch.ExceptionsHelper.unwrapCause; import static org.elasticsearch.action.support.ActionTestUtils.assertNoFailureListener; +import static org.elasticsearch.action.support.ActionTestUtils.assertNoSuccessListener; import static org.elasticsearch.test.ESIntegTestCase.internalCluster; import static org.elasticsearch.test.ESTestCase.asInstanceOf; import static org.elasticsearch.test.ESTestCase.randomInt; @@ -37,7 +38,6 @@ import static org.junit.Assert.assertNull; import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; /** * Utility plugin that captures the invocation of an action on a node after the task has been registered with the {@link TaskManager}, @@ -128,19 +128,11 @@ public void app if (capturingListener != null) { final var cancellableTask = asInstanceOf(CancellableTask.class, task); capturingListener.addListener(assertNoFailureListener(captured -> { - cancellableTask.addListener(() -> chain.proceed(task, action, request, new ActionListener<>() { - @Override - public void onResponse(Response response) { - fail("cancelled action should not succeed, but got " + response); - } - - @Override - public void onFailure(Exception e) { - assertThat(unwrapCause(e), instanceOf(TaskCancelledException.class)); - listener.onFailure(e); - captured.countDownLatch().countDown(); - } - })); + cancellableTask.addListener(() -> chain.proceed(task, action, request, assertNoSuccessListener(e -> { + assertThat(unwrapCause(e), instanceOf(TaskCancelledException.class)); + listener.onFailure(e); + captured.countDownLatch().countDown(); + }))); assertFalse(cancellableTask.isCancelled()); captured.doCancel().run(); })); diff --git a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/dataframe/DestinationIndexTests.java b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/dataframe/DestinationIndexTests.java index 2f3ccaa313b0d..f0f7dec448d99 100644 --- a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/dataframe/DestinationIndexTests.java +++ b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/dataframe/DestinationIndexTests.java @@ -61,6 +61,7 @@ import java.util.Map; import static java.util.Collections.singletonMap; +import static org.elasticsearch.action.support.ActionTestUtils.assertNoSuccessListener; import static org.elasticsearch.common.xcontent.support.XContentMapValues.extractValue; import static org.elasticsearch.xpack.ml.DefaultMachineLearningExtension.ANALYTICS_DEST_INDEX_ALLOWED_SETTINGS; import static org.hamcrest.Matchers.arrayContaining; @@ -334,10 +335,7 @@ private Map testCreateDestinationIndex(DataFrameAnalysis analysi clock, config, ANALYTICS_DEST_INDEX_ALLOWED_SETTINGS, - ActionListener.wrap( - response -> fail("should not succeed"), - e -> assertThat(e.getMessage(), Matchers.matchesRegex(finalErrorMessage)) - ) + assertNoSuccessListener(e -> assertThat(e.getMessage(), Matchers.matchesRegex(finalErrorMessage))) ); return null; @@ -578,8 +576,7 @@ public void testCreateDestinationIndex_ResultsFieldsExistsInSourceIndex() { clock, config, ANALYTICS_DEST_INDEX_ALLOWED_SETTINGS, - ActionListener.wrap( - response -> fail("should not succeed"), + assertNoSuccessListener( e -> assertThat( e.getMessage(), equalTo("A field that matches the dest.results_field [ml] already exists; please set a different results_field")