Fix leaking tests in HealthPeriodicLoggerTests (#134295)

At least one (`testClosingWhenRunInProgress`) test, but likely more,
were leaking into other tests in `HealthPeriodicLoggerTests`. By
triggering the health logger asynchronously in a separate thread, the
actual health log would be logged in a subsequent test.
`testOutputModeNoLogging` asserts that nothing is logged, so this
leaking log would result in a test failure.

We simply need to wait for the separate thread to finish before
proceeding to the next test.

Fixes #134200

Author: nielsbauman

muted-tests.yml

@@ -510,9 +510,6 @@ tests:
 - class: org.elasticsearch.xpack.esql.qa.single_node.GenerativeForkIT
   method: test {csv-spec:inlinestats.MultiIndexInlinestatsOfMultiTypedField}
   issue: https://github.com/elastic/elasticsearch/issues/133973
-- class: org.elasticsearch.health.HealthPeriodicLoggerTests
-  method: testOutputModeNoLogging
-  issue: https://github.com/elastic/elasticsearch/issues/134200
 - class: org.elasticsearch.xpack.esql.qa.single_node.GenerativeForkIT
   method: test {csv-spec:spatial_shapes.ConvertFromStringParseError}
   issue: https://github.com/elastic/elasticsearch/issues/134254

server/src/test/java/org/elasticsearch/health/HealthPeriodicLoggerTests.java

@@ -436,23 +436,24 @@ public void testTryToLogHealthConcurrencyControlWithResults() throws Exception {
         SchedulerEngine.Event event = new SchedulerEngine.Event(HealthPeriodicLogger.HEALTH_PERIODIC_LOGGER_JOB_NAME, 0, 0);
 
         // run it once, verify getHealth is called
-        {
-            Thread logHealthThread = new Thread(() -> testHealthPeriodicLogger.triggered(event));
-            logHealthThread.start();
-            // We wait to verify that the triggered even is in progress, then we block, so it will rename in progress
-            assertBusy(() -> assertThat(getHealthCalled.get(), equalTo(1)));
-            // We try to log again while it's in progress, we expect this run to be skipped
-            assertFalse(testHealthPeriodicLogger.tryToLogHealth());
-            // Unblock the first execution
-            waitForSecondRun.countDown();
-        }
+        Thread logHealthThread = new Thread(() -> testHealthPeriodicLogger.triggered(event));
+        logHealthThread.start();
+        // We wait to verify that the triggered even is in progress, then we block, so it will rename in progress
+        assertBusy(() -> assertThat(getHealthCalled.get(), equalTo(1)));
+        // We try to log again while it's in progress, we expect this run to be skipped
+        assertFalse(testHealthPeriodicLogger.tryToLogHealth());
+        // Unblock the first execution
+        waitForSecondRun.countDown();
 
         // run it again, verify getHealth is called, because we are calling the results listener
         {
             waitForRelease.await();
             testHealthPeriodicLogger.triggered(event);
             assertBusy(() -> assertThat(getHealthCalled.get(), equalTo(2)));
         }
+
+        // Wait for the log thread to finish, to ensure it logs during this test and doesn't pollute other tests.
+        logHealthThread.join();
     }
 
     public void testTryToLogHealthConcurrencyControl() throws Exception {
@@ -485,11 +486,9 @@ public void testTryToLogHealthConcurrencyControl() throws Exception {
         SchedulerEngine.Event event = new SchedulerEngine.Event(HealthPeriodicLogger.HEALTH_PERIODIC_LOGGER_JOB_NAME, 0, 0);
 
         // call it and verify that getHealth is called
-        {
-            Thread logHealthThread = new Thread(() -> testHealthPeriodicLogger.triggered(event));
-            logHealthThread.start();
-            assertBusy(() -> assertThat(getHealthCalled.get(), equalTo(1)));
-        }
+        Thread logHealthThread = new Thread(() -> testHealthPeriodicLogger.triggered(event));
+        logHealthThread.start();
+        assertBusy(() -> assertThat(getHealthCalled.get(), equalTo(1)));
 
         // run it again, verify that it's skipped because the other one is in progress
         {
@@ -504,6 +503,9 @@ public void testTryToLogHealthConcurrencyControl() throws Exception {
             testHealthPeriodicLogger.triggered(event);
             assertBusy(() -> assertThat(getHealthCalled.get(), equalTo(2)));
         }
+
+        // Wait for the log thread to finish, to ensure it logs during this test and doesn't pollute other tests.
+        logHealthThread.join();
     }
 
     public void testTryToLogHealthConcurrencyControlWithException() throws Exception {
@@ -609,11 +611,9 @@ public void testClosingWhenRunInProgress() throws Exception {
             SchedulerEngine.Event event = new SchedulerEngine.Event(HealthPeriodicLogger.HEALTH_PERIODIC_LOGGER_JOB_NAME, 0, 0);
 
             // call it and verify that getHealth is called
-            {
-                Thread logHealthThread = new Thread(() -> testHealthPeriodicLogger.triggered(event));
-                logHealthThread.start();
-                assertBusy(() -> assertTrue(testHealthPeriodicLogger.currentlyRunning()));
-            }
+            Thread logHealthThread = new Thread(() -> testHealthPeriodicLogger.triggered(event));
+            logHealthThread.start();
+            assertBusy(() -> assertTrue(testHealthPeriodicLogger.currentlyRunning()));
 
             // stop and close it
             {
@@ -626,6 +626,9 @@ public void testClosingWhenRunInProgress() throws Exception {
                 waitForCloseToBeTriggered.countDown();
                 assertBusy(() -> assertEquals(Lifecycle.State.CLOSED, testHealthPeriodicLogger.lifecycleState()));
             }
+
+            // Wait for the log thread to finish, to ensure it logs during this test and doesn't pollute other tests.
+            logHealthThread.join();
         }
     }