From e4ba2773586a87379835a7d2bcd62cbbaab2408d Mon Sep 17 00:00:00 2001 From: David Kyle Date: Mon, 5 Jul 2021 13:54:02 +0100 Subject: [PATCH 1/2] Speed up the AbstractNativeProcessTests --- .../xpack/ml/process/AbstractNativeProcessTests.java | 9 --------- 1 file changed, 9 deletions(-) diff --git a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/process/AbstractNativeProcessTests.java b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/process/AbstractNativeProcessTests.java index fad1485931c5f..70a4f0cc8ffbc 100644 --- a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/process/AbstractNativeProcessTests.java +++ b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/process/AbstractNativeProcessTests.java @@ -88,7 +88,6 @@ public void testStart_DoNotDetectCrashWhenNoInputPipeProvided() throws Exception when(processPipes.getProcessInStream()).thenReturn(Optional.empty()); try (AbstractNativeProcess process = new TestNativeProcess()) { process.start(executorService); - } finally { mockNativeProcessLoggingStreamEnds.countDown(); // Not detecting a crash is confirmed in terminateExecutorService() } @@ -97,7 +96,6 @@ public void testStart_DoNotDetectCrashWhenNoInputPipeProvided() throws Exception public void testStart_DoNotDetectCrashWhenProcessIsBeingClosed() throws Exception { try (AbstractNativeProcess process = new TestNativeProcess()) { process.start(executorService); - } finally { mockNativeProcessLoggingStreamEnds.countDown(); // Not detecting a crash is confirmed in terminateExecutorService() } @@ -147,8 +145,6 @@ public void testWriteRecord() throws Exception { process.flushStream(); verify(inputStream).write(any(), anyInt(), anyInt()); - - } finally { mockNativeProcessLoggingStreamEnds.countDown(); } } @@ -158,7 +154,6 @@ public void testWriteRecord_FailWhenNoInputPipeProvided() throws Exception { try (AbstractNativeProcess process = new TestNativeProcess()) { process.start(executorService); expectThrows(NullPointerException.class, () -> process.writeRecord(new String[] {"a", "b", "c"})); - } finally { mockNativeProcessLoggingStreamEnds.countDown(); } } @@ -169,7 +164,6 @@ public void testFlush() throws Exception { process.flushStream(); verify(inputStream).flush(); - } finally { mockNativeProcessLoggingStreamEnds.countDown(); } } @@ -179,7 +173,6 @@ public void testFlush_FailWhenNoInputPipeProvided() throws Exception { try (AbstractNativeProcess process = new TestNativeProcess()) { process.start(executorService); expectThrows(NullPointerException.class, process::flushStream); - } finally { mockNativeProcessLoggingStreamEnds.countDown(); } } @@ -190,7 +183,6 @@ public void testIsReady() throws Exception { assertThat(process.isReady(), is(false)); process.setReady(); assertThat(process.isReady(), is(true)); - } finally { mockNativeProcessLoggingStreamEnds.countDown(); } } @@ -199,7 +191,6 @@ public void testConsumeAndCloseOutputStream_GivenNoOutputStream() throws Excepti when(processPipes.getProcessOutStream()).thenReturn(Optional.empty()); try (AbstractNativeProcess process = new TestNativeProcess()) { process.consumeAndCloseOutputStream(); - } finally { mockNativeProcessLoggingStreamEnds.countDown(); } } From ff958b80268f8d4182ef083d971e2f80f5ca9fe9 Mon Sep 17 00:00:00 2001 From: David Kyle Date: Mon, 5 Jul 2021 15:07:55 +0100 Subject: [PATCH 2/2] do not wait in case of failure --- .../process/AbstractNativeProcessTests.java | 82 ++++++++++++------- 1 file changed, 52 insertions(+), 30 deletions(-) diff --git a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/process/AbstractNativeProcessTests.java b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/process/AbstractNativeProcessTests.java index 70a4f0cc8ffbc..cc077ee4a2f8c 100644 --- a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/process/AbstractNativeProcessTests.java +++ b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/process/AbstractNativeProcessTests.java @@ -87,17 +87,23 @@ public void terminateExecutorService() { public void testStart_DoNotDetectCrashWhenNoInputPipeProvided() throws Exception { when(processPipes.getProcessInStream()).thenReturn(Optional.empty()); try (AbstractNativeProcess process = new TestNativeProcess()) { - process.start(executorService); - mockNativeProcessLoggingStreamEnds.countDown(); - // Not detecting a crash is confirmed in terminateExecutorService() + try { + process.start(executorService); + } finally { + mockNativeProcessLoggingStreamEnds.countDown(); + // Not detecting a crash is confirmed in terminateExecutorService() + } } } public void testStart_DoNotDetectCrashWhenProcessIsBeingClosed() throws Exception { try (AbstractNativeProcess process = new TestNativeProcess()) { - process.start(executorService); - mockNativeProcessLoggingStreamEnds.countDown(); - // Not detecting a crash is confirmed in terminateExecutorService() + try { + process.start(executorService); + } finally { + mockNativeProcessLoggingStreamEnds.countDown(); + // Not detecting a crash is confirmed in terminateExecutorService() + } } } @@ -140,58 +146,74 @@ public void testCrashReporting() throws Exception { public void testWriteRecord() throws Exception { try (AbstractNativeProcess process = new TestNativeProcess()) { - process.start(executorService); - process.writeRecord(new String[] {"a", "b", "c"}); - process.flushStream(); - - verify(inputStream).write(any(), anyInt(), anyInt()); - mockNativeProcessLoggingStreamEnds.countDown(); + try { + process.start(executorService); + process.writeRecord(new String[]{"a", "b", "c"}); + process.flushStream(); + verify(inputStream).write(any(), anyInt(), anyInt()); + } finally { + mockNativeProcessLoggingStreamEnds.countDown(); + } } } public void testWriteRecord_FailWhenNoInputPipeProvided() throws Exception { when(processPipes.getProcessInStream()).thenReturn(Optional.empty()); try (AbstractNativeProcess process = new TestNativeProcess()) { - process.start(executorService); - expectThrows(NullPointerException.class, () -> process.writeRecord(new String[] {"a", "b", "c"})); - mockNativeProcessLoggingStreamEnds.countDown(); + try { + process.start(executorService); + expectThrows(NullPointerException.class, () -> process.writeRecord(new String[]{"a", "b", "c"})); + } finally { + mockNativeProcessLoggingStreamEnds.countDown(); + } } } public void testFlush() throws Exception { try (AbstractNativeProcess process = new TestNativeProcess()) { - process.start(executorService); - process.flushStream(); - - verify(inputStream).flush(); - mockNativeProcessLoggingStreamEnds.countDown(); + try { + process.start(executorService); + process.flushStream(); + verify(inputStream).flush(); + } finally { + mockNativeProcessLoggingStreamEnds.countDown(); + } } } public void testFlush_FailWhenNoInputPipeProvided() throws Exception { when(processPipes.getProcessInStream()).thenReturn(Optional.empty()); try (AbstractNativeProcess process = new TestNativeProcess()) { - process.start(executorService); - expectThrows(NullPointerException.class, process::flushStream); - mockNativeProcessLoggingStreamEnds.countDown(); + try { + process.start(executorService); + expectThrows(NullPointerException.class, process::flushStream); + } finally { + mockNativeProcessLoggingStreamEnds.countDown(); + } } } public void testIsReady() throws Exception { try (AbstractNativeProcess process = new TestNativeProcess()) { - process.start(executorService); - assertThat(process.isReady(), is(false)); - process.setReady(); - assertThat(process.isReady(), is(true)); - mockNativeProcessLoggingStreamEnds.countDown(); + try { + process.start(executorService); + assertThat(process.isReady(), is(false)); + process.setReady(); + assertThat(process.isReady(), is(true)); + } finally { + mockNativeProcessLoggingStreamEnds.countDown(); + } } } public void testConsumeAndCloseOutputStream_GivenNoOutputStream() throws Exception { when(processPipes.getProcessOutStream()).thenReturn(Optional.empty()); try (AbstractNativeProcess process = new TestNativeProcess()) { - process.consumeAndCloseOutputStream(); - mockNativeProcessLoggingStreamEnds.countDown(); + try { + process.consumeAndCloseOutputStream(); + } finally { + mockNativeProcessLoggingStreamEnds.countDown(); + } } }