diff --git a/modules/transport-netty4/src/internalClusterTest/java/org/elasticsearch/transport/netty4/ESLoggingHandlerIT.java b/modules/transport-netty4/src/internalClusterTest/java/org/elasticsearch/transport/netty4/ESLoggingHandlerIT.java index d0cef178dc920..aee0d313e4e00 100644 --- a/modules/transport-netty4/src/internalClusterTest/java/org/elasticsearch/transport/netty4/ESLoggingHandlerIT.java +++ b/modules/transport-netty4/src/internalClusterTest/java/org/elasticsearch/transport/netty4/ESLoggingHandlerIT.java @@ -10,7 +10,6 @@ import org.apache.logging.log4j.Level; import org.elasticsearch.ESNetty4IntegTestCase; -import org.elasticsearch.core.Releasable; import org.elasticsearch.core.TimeValue; import org.elasticsearch.test.ESIntegTestCase; import org.elasticsearch.test.MockLogAppender; @@ -24,16 +23,14 @@ public class ESLoggingHandlerIT extends ESNetty4IntegTestCase { private MockLogAppender appender; - private Releasable appenderRelease; public void setUp() throws Exception { super.setUp(); - appender = new MockLogAppender(); - appenderRelease = appender.capturing(ESLoggingHandler.class, TransportLogger.class, TcpTransport.class); + appender = MockLogAppender.capture(ESLoggingHandler.class, TransportLogger.class, TcpTransport.class); } public void tearDown() throws Exception { - appenderRelease.close(); + appender.close(); super.tearDown(); } diff --git a/qa/no-bootstrap-tests/src/test/java/org/elasticsearch/bootstrap/SpawnerNoBootstrapTests.java b/qa/no-bootstrap-tests/src/test/java/org/elasticsearch/bootstrap/SpawnerNoBootstrapTests.java index 08e3ac2cbce8c..99b2728ebfa3c 100644 --- a/qa/no-bootstrap-tests/src/test/java/org/elasticsearch/bootstrap/SpawnerNoBootstrapTests.java +++ b/qa/no-bootstrap-tests/src/test/java/org/elasticsearch/bootstrap/SpawnerNoBootstrapTests.java @@ -206,16 +206,16 @@ private void assertControllerSpawns(final Function pluginsDir String stdoutLoggerName = "test_plugin-controller-stdout"; String stderrLoggerName = "test_plugin-controller-stderr"; - MockLogAppender appender = new MockLogAppender(); Loggers.setLevel(LogManager.getLogger(stdoutLoggerName), Level.TRACE); Loggers.setLevel(LogManager.getLogger(stderrLoggerName), Level.TRACE); CountDownLatch messagesLoggedLatch = new CountDownLatch(2); - if (expectSpawn) { - appender.addExpectation(new ExpectedStreamMessage(stdoutLoggerName, "I am alive", messagesLoggedLatch)); - appender.addExpectation(new ExpectedStreamMessage(stderrLoggerName, "I am an error", messagesLoggedLatch)); - } - try (var ignore = appender.capturing(stdoutLoggerName, stderrLoggerName)) { + try (var appender = MockLogAppender.capture(stdoutLoggerName, stderrLoggerName)) { + if (expectSpawn) { + appender.addExpectation(new ExpectedStreamMessage(stdoutLoggerName, "I am alive", messagesLoggedLatch)); + appender.addExpectation(new ExpectedStreamMessage(stderrLoggerName, "I am an error", messagesLoggedLatch)); + } + Spawner spawner = new Spawner(); spawner.spawnNativeControllers(environment); diff --git a/server/src/internalClusterTest/java/org/elasticsearch/cluster/allocation/ClusterRerouteIT.java b/server/src/internalClusterTest/java/org/elasticsearch/cluster/allocation/ClusterRerouteIT.java index 3b9d3e133b63a..fcccc0051f0cd 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/cluster/allocation/ClusterRerouteIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/cluster/allocation/ClusterRerouteIT.java @@ -387,17 +387,16 @@ public void testMessageLogging() { ) .get(); - MockLogAppender dryRunMockLog = new MockLogAppender(); - dryRunMockLog.addExpectation( - new MockLogAppender.UnseenEventExpectation( - "no completed message logged on dry run", - TransportClusterRerouteAction.class.getName(), - Level.INFO, - "allocated an empty primary*" - ) - ); + try (var dryRunMockLog = MockLogAppender.capture(TransportClusterRerouteAction.class)) { + dryRunMockLog.addExpectation( + new MockLogAppender.UnseenEventExpectation( + "no completed message logged on dry run", + TransportClusterRerouteAction.class.getName(), + Level.INFO, + "allocated an empty primary*" + ) + ); - try (var ignored = dryRunMockLog.capturing(TransportClusterRerouteAction.class)) { AllocationCommand dryRunAllocation = new AllocateEmptyPrimaryAllocationCommand(indexName, 0, nodeName1, true); ClusterRerouteResponse dryRunResponse = clusterAdmin().prepareReroute() .setExplain(randomBoolean()) @@ -412,24 +411,23 @@ public void testMessageLogging() { dryRunMockLog.assertAllExpectationsMatched(); } - MockLogAppender allocateMockLog = new MockLogAppender(); - allocateMockLog.addExpectation( - new MockLogAppender.SeenEventExpectation( - "message for first allocate empty primary", - TransportClusterRerouteAction.class.getName(), - Level.INFO, - "allocated an empty primary*" + nodeName1 + "*" - ) - ); - allocateMockLog.addExpectation( - new MockLogAppender.UnseenEventExpectation( - "no message for second allocate empty primary", - TransportClusterRerouteAction.class.getName(), - Level.INFO, - "allocated an empty primary*" + nodeName2 + "*" - ) - ); - try (var ignored = allocateMockLog.capturing(TransportClusterRerouteAction.class)) { + try (var allocateMockLog = MockLogAppender.capture(TransportClusterRerouteAction.class)) { + allocateMockLog.addExpectation( + new MockLogAppender.SeenEventExpectation( + "message for first allocate empty primary", + TransportClusterRerouteAction.class.getName(), + Level.INFO, + "allocated an empty primary*" + nodeName1 + "*" + ) + ); + allocateMockLog.addExpectation( + new MockLogAppender.UnseenEventExpectation( + "no message for second allocate empty primary", + TransportClusterRerouteAction.class.getName(), + Level.INFO, + "allocated an empty primary*" + nodeName2 + "*" + ) + ); AllocationCommand yesDecisionAllocation = new AllocateEmptyPrimaryAllocationCommand(indexName, 0, nodeName1, true); AllocationCommand noDecisionAllocation = new AllocateEmptyPrimaryAllocationCommand("noexist", 1, nodeName2, true); diff --git a/test/framework/src/main/java/org/elasticsearch/test/MockLogAppender.java b/test/framework/src/main/java/org/elasticsearch/test/MockLogAppender.java index bc3723119afa9..dd7987642c58a 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/MockLogAppender.java +++ b/test/framework/src/main/java/org/elasticsearch/test/MockLogAppender.java @@ -31,13 +31,35 @@ /** * Test appender that can be used to verify that certain events were logged correctly */ -public class MockLogAppender { +public class MockLogAppender implements Releasable { private static final Map> mockAppenders = new ConcurrentHashMap<>(); private static final RealMockAppender parent = new RealMockAppender(); + // TODO: this can become final once the ctor is made private + private List loggers = List.of(); private final List expectations; private volatile boolean isAlive = true; + @Override + public void close() { + isAlive = false; + for (String logger : loggers) { + mockAppenders.compute(logger, (k, v) -> { + assert v != null; + v.remove(this); + return v.isEmpty() ? null : v; + }); + } + // check that all expectations have been evaluated before this is released + for (WrappedLoggingExpectation expectation : expectations) { + assertThat( + "Method assertMatched() not called on LoggingExpectation instance before release: " + expectation, + expectation.assertMatchedCalled, + is(true) + ); + } + } + private static class RealMockAppender extends AbstractAppender { RealMockAppender() { @@ -71,6 +93,11 @@ public MockLogAppender() { expectations = new CopyOnWriteArrayList<>(); } + private MockLogAppender(List loggers) { + this(); + this.loggers = loggers; + } + /** * Initialize the mock log appender with the log4j system. */ @@ -267,58 +294,57 @@ public String toString() { } } + public Releasable capturing(Class... classes) { + this.loggers = Arrays.stream(classes).map(Class::getCanonicalName).toList(); + addToMockAppenders(this, loggers); + return this; + } + + public Releasable capturing(String... names) { + this.loggers = Arrays.asList(names); + addToMockAppenders(this, loggers); + return this; + } + /** * Adds the list of class loggers to this {@link MockLogAppender}. * * Stops and runs some checks on the {@link MockLogAppender} once the returned object is released. */ - public Releasable capturing(Class... classes) { - return appendToLoggers(Arrays.stream(classes).map(Class::getCanonicalName).toList()); + public static MockLogAppender capture(Class... classes) { + return create(Arrays.stream(classes).map(Class::getCanonicalName).toList()); } /** * Same as above except takes string class names of each logger. */ - public Releasable capturing(String... names) { - return appendToLoggers(Arrays.asList(names)); + public static MockLogAppender capture(String... names) { + return create(Arrays.asList(names)); + } + + private static MockLogAppender create(List loggers) { + MockLogAppender appender = new MockLogAppender(loggers); + addToMockAppenders(appender, loggers); + return appender; } - private Releasable appendToLoggers(List loggers) { + private static void addToMockAppenders(MockLogAppender appender, List loggers) { for (String logger : loggers) { mockAppenders.compute(logger, (k, v) -> { if (v == null) { v = new CopyOnWriteArrayList<>(); } - v.add(this); + v.add(appender); return v; }); } - return () -> { - isAlive = false; - for (String logger : loggers) { - mockAppenders.compute(logger, (k, v) -> { - assert v != null; - v.remove(this); - return v.isEmpty() ? null : v; - }); - } - // check that all expectations have been evaluated before this is released - for (WrappedLoggingExpectation expectation : expectations) { - assertThat( - "Method assertMatched() not called on LoggingExpectation instance before release: " + expectation, - expectation.assertMatchedCalled, - is(true) - ); - } - }; } /** * Executes an action and verifies expectations against the provided logger */ public static void assertThatLogger(Runnable action, Class loggerOwner, MockLogAppender.LoggingExpectation expectation) { - MockLogAppender mockAppender = new MockLogAppender(); - try (var ignored = mockAppender.capturing(loggerOwner)) { + try (var mockAppender = MockLogAppender.capture(loggerOwner)) { mockAppender.addExpectation(expectation); action.run(); mockAppender.assertAllExpectationsMatched(); diff --git a/test/framework/src/main/java/org/elasticsearch/transport/AbstractSimpleTransportTestCase.java b/test/framework/src/main/java/org/elasticsearch/transport/AbstractSimpleTransportTestCase.java index ee7687398cf7b..89d10acb6ec45 100644 --- a/test/framework/src/main/java/org/elasticsearch/transport/AbstractSimpleTransportTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/transport/AbstractSimpleTransportTestCase.java @@ -1319,8 +1319,7 @@ public void handleException(TransportException exp) {} .build() ); - MockLogAppender appender = new MockLogAppender(); - try (var ignored = appender.capturing("org.elasticsearch.transport.TransportService.tracer")) { + try (var appender = MockLogAppender.capture("org.elasticsearch.transport.TransportService.tracer")) { //////////////////////////////////////////////////////////////////////// // tests for included action type "internal:test"