Fix testInvalidJSON (#118398)

* Fix testInvalidJSON

* CURSE YOU SPOTLESS

Author: prdoyle

muted-tests.yml

@@ -123,9 +123,6 @@ tests:
 - class: org.elasticsearch.xpack.downsample.ILMDownsampleDisruptionIT
   method: testILMDownsampleRollingRestart
   issue: https://github.com/elastic/elasticsearch/issues/114233
-- class: org.elasticsearch.reservedstate.service.FileSettingsServiceTests
-  method: testInvalidJSON
-  issue: https://github.com/elastic/elasticsearch/issues/116521
 - class: org.elasticsearch.reservedstate.service.RepositoriesFileSettingsIT
   method: testSettingsApplied
   issue: https://github.com/elastic/elasticsearch/issues/116694

server/src/test/java/org/elasticsearch/reservedstate/service/FileSettingsServiceTests.java

@@ -74,8 +74,10 @@
 import static org.hamcrest.Matchers.hasEntry;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.argThat;
+import static org.mockito.ArgumentMatchers.contains;
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.doAnswer;
+import static org.mockito.Mockito.doNothing;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.times;
@@ -288,63 +290,54 @@ public void testProcessFileChanges() throws Exception {
         verifyNoMoreInteractions(healthIndicatorService);
     }
 
-    @SuppressWarnings("unchecked")
     public void testInvalidJSON() throws Exception {
-        doAnswer((Answer<Void>) invocation -> {
-            invocation.getArgument(1, XContentParser.class).map(); // Throw if JSON is invalid
-            ((Consumer<Exception>) invocation.getArgument(3)).accept(null);
-            return null;
-        }).when(controller).process(any(), any(XContentParser.class), any(), any());
-
-        CyclicBarrier fileChangeBarrier = new CyclicBarrier(2);
-        fileSettingsService.addFileChangedListener(() -> awaitOrBust(fileChangeBarrier));
+        // Chop off the functionality so we don't run too much of the actual cluster logic that we're not testing
+        doNothing().when(controller).updateErrorState(any());
+        doAnswer(
+            (Answer<Void>) invocation -> { throw new AssertionError("Parse error should happen before this process method is called"); }
+        ).when(controller).process(any(), any(ReservedStateChunk.class), any(), any());
 
+        // Don't really care about the initial state
         Files.createDirectories(fileSettingsService.watchedFileDir());
-        // contents of the JSON don't matter, we just need a file to exist
-        writeTestFile(fileSettingsService.watchedFile(), "{}");
+        doNothing().when(fileSettingsService).processInitialFileMissing();
+        fileSettingsService.start();
+        fileSettingsService.clusterChanged(new ClusterChangedEvent("test", clusterService.state(), ClusterState.EMPTY_STATE));
 
+        // Now break the JSON and wait
+        CyclicBarrier fileChangeBarrier = new CyclicBarrier(2);
         doAnswer((Answer<?>) invocation -> {
-            boolean returnedNormally = false;
             try {
-                var result = invocation.callRealMethod();
-                returnedNormally = true;
-                return result;
-            } catch (XContentParseException e) {
-                // We're expecting a parse error. processFileChanges specifies that this is supposed to throw ExecutionException.
-                throw new ExecutionException(e);
-            } catch (Throwable e) {
-                throw new AssertionError("Unexpected exception", e);
+                return invocation.callRealMethod();
             } finally {
-                if (returnedNormally == false) {
-                    // Because of the exception, listeners aren't notified, so we need to activate the barrier ourselves
-                    awaitOrBust(fileChangeBarrier);
-                }
+                awaitOrBust(fileChangeBarrier);
             }
         }).when(fileSettingsService).processFileChanges();
-
-        // Establish the initial valid JSON
-        fileSettingsService.start();
-        fileSettingsService.clusterChanged(new ClusterChangedEvent("test", clusterService.state(), ClusterState.EMPTY_STATE));
-        awaitOrBust(fileChangeBarrier);
-
-        // Now break the JSON
         writeTestFile(fileSettingsService.watchedFile(), "test_invalid_JSON");
         awaitOrBust(fileChangeBarrier);
 
-        verify(fileSettingsService, times(1)).processFileOnServiceStart(); // The initial state
-        verify(fileSettingsService, times(1)).processFileChanges(); // The changed state
         verify(fileSettingsService, times(1)).onProcessFileChangesException(
-            argThat(e -> e instanceof ExecutionException && e.getCause() instanceof XContentParseException)
+            argThat(e -> unwrapException(e) instanceof XContentParseException)
         );
 
         // Note: the name "processFileOnServiceStart" is a bit misleading because it is not
         // referring to fileSettingsService.start(). Rather, it is referring to the initialization
         // of the watcher thread itself, which occurs asynchronously when clusterChanged is first called.
 
-        verify(healthIndicatorService, times(2)).changeOccurred();
-        verify(healthIndicatorService, times(1)).successOccurred();
-        verify(healthIndicatorService, times(1)).failureOccurred(argThat(s -> s.startsWith(IllegalArgumentException.class.getName())));
-        verifyNoMoreInteractions(healthIndicatorService);
+        verify(healthIndicatorService).failureOccurred(contains(XContentParseException.class.getName()));
+    }
+
+    /**
+     * Looks for the ultimate cause of {@code e} by stripping off layers of bookkeeping exception wrappers.
+     */
+    private Throwable unwrapException(Throwable e) {
+        while (e != null) {
+            if (e instanceof ExecutionException || e instanceof IllegalStateException) {
+                e = e.getCause();
+            } else {
+                break;
+            }
+        }
+        return e;
     }
 
     private static void awaitOrBust(CyclicBarrier barrier) {