Tests: Ensure watcher is enabled/disabled during tests (elastic/x-pack-elasticsearch#2392)

The method to check if watcher was enabled was returning
`randomBoolean()` and thus could change during test runs.

This fixes the test to ensure that always the same value
is returned and documents this requirement.

relates elastic/x-pack-elasticsearch#1783

Original commit: elastic/x-pack-elasticsearch@97bf3cfc29

plugin/src/test/java/org/elasticsearch/xpack/monitoring/resolver/node/NodeStatsTests.java

@@ -28,6 +28,8 @@ import static org.hamcrest.Matchers.greaterThan;
 @ClusterScope(scope = Scope.TEST, numClientNodes = 0, transportClientRatio = 0.0)
 public class NodeStatsTests extends MonitoringIntegTestCase {
 
+    private static Boolean WATCHER_ENABLED = null;
+
     @Override
     protected Settings nodeSettings(int nodeOrdinal) {
         return Settings.builder()
@@ -67,7 +69,7 @@ public class NodeStatsTests extends MonitoringIntegTestCase {
         for (SearchHit searchHit : response.getHits().getHits()) {
             Map<String, Object> fields = searchHit.getSourceAsMap();
 
-            for (String filter : nodeStatsFilters(watcherEnabled)) {
+            for (String filter : nodeStatsFilters()) {
                 if (Constants.WINDOWS) {
                     // load average is unavailable on Windows
                     if (filter.startsWith("node_stats.os.cpu.load_average")) {
@@ -94,12 +96,10 @@ public class NodeStatsTests extends MonitoringIntegTestCase {
     /**
      * Optionally exclude {@link NodeStatsResolver#FILTERS} that require Watcher to be enabled.
      *
-     * @param includeWatcher {@code true} to keep watcher filters.
      * @return Never {@code null} or empty.
-     * @see #watcherEnabled
      */
-    private static Set<String> nodeStatsFilters(boolean includeWatcher) {
-        if (includeWatcher) {
+    private Set<String> nodeStatsFilters() {
+        if (enableWatcher()) {
             return NodeStatsResolver.FILTERS;
         }
 
@@ -108,8 +108,11 @@ public class NodeStatsTests extends MonitoringIntegTestCase {
 
     @Override
     protected boolean enableWatcher() {
+        if (WATCHER_ENABLED == null) {
+            WATCHER_ENABLED = randomBoolean();
+        }
         // currently this is the only Monitoring test that expects Watcher to be enabled.
         // Once this becomes the default, then this should be removed.
-        return randomBoolean();
+        return WATCHER_ENABLED;
     }
 }

plugin/src/test/java/org/elasticsearch/xpack/monitoring/test/MonitoringIntegTestCase.java

@@ -85,18 +85,6 @@ public abstract class MonitoringIntegTestCase extends ESIntegTestCase {
      * Per test run this is enabled or disabled.
      */
     protected static Boolean securityEnabled;
-    /**
-     * Enables individual tests to control the behavior.
-     * <p>
-     * Control this by overriding {@link #enableWatcher()}, which defaults to disabling it (this will change!).
-     */
-    protected Boolean watcherEnabled;
-
-    private void randomizeSettings() {
-        if (watcherEnabled == null) {
-            watcherEnabled = enableWatcher();
-        }
-    }
 
     @Override
     protected TestCluster buildTestCluster(Scope scope, long seed) throws IOException {
@@ -109,11 +97,9 @@ public abstract class MonitoringIntegTestCase extends ESIntegTestCase {
 
     @Override
     protected Settings nodeSettings(int nodeOrdinal) {
-        randomizeSettings();
-
         Settings.Builder builder = Settings.builder()
                 .put(super.nodeSettings(nodeOrdinal))
-                .put(XPackSettings.WATCHER_ENABLED.getKey(), watcherEnabled)
+                .put(XPackSettings.WATCHER_ENABLED.getKey(), enableWatcher())
                 // Disable native ML autodetect_process as the c++ controller won't be available
                 .put(MachineLearning.AUTODETECT_PROCESS.getKey(), false)
                 .put(XPackSettings.MACHINE_LEARNING_ENABLED.getKey(), false)
@@ -152,8 +138,6 @@ public abstract class MonitoringIntegTestCase extends ESIntegTestCase {
 
     @Override
     protected Settings transportClientSettings() {
-        randomizeSettings();
-
         if (securityEnabled) {
             return Settings.builder()
                     .put(super.transportClientSettings())
@@ -161,12 +145,12 @@ public abstract class MonitoringIntegTestCase extends ESIntegTestCase {
                     .put(Security.USER_SETTING.getKey(), "test:" + SecuritySettings.TEST_PASSWORD)
                     .put(NetworkModule.TRANSPORT_TYPE_KEY, Security.NAME4)
                     .put(NetworkModule.HTTP_TYPE_KEY, Security.NAME4)
-                    .put(XPackSettings.WATCHER_ENABLED.getKey(), watcherEnabled)
+                    .put(XPackSettings.WATCHER_ENABLED.getKey(), enableWatcher())
                     .build();
         }
         return Settings.builder().put(super.transportClientSettings())
                 .put(XPackSettings.SECURITY_ENABLED.getKey(), false)
-                .put(XPackSettings.WATCHER_ENABLED.getKey(), watcherEnabled)
+                .put(XPackSettings.WATCHER_ENABLED.getKey(), enableWatcher())
                 .build();
     }
 
@@ -222,7 +206,7 @@ public abstract class MonitoringIntegTestCase extends ESIntegTestCase {
 
     @After
     public void tearDown() throws Exception {
-        if (watcherEnabled != null && watcherEnabled) {
+        if (enableWatcher()) {
             internalCluster().getInstances(WatcherLifeCycleService.class)
                     .forEach(w -> w.stop("tearing down watcher as part of monitoring test case"));
         }
@@ -232,6 +216,9 @@ public abstract class MonitoringIntegTestCase extends ESIntegTestCase {
 
     /**
      * Override and return {@code false} to force running without Watcher.
+     *
+     * Ensure that this method always returns the same value during a test run, do not put randomBoolean() in here
+     * as it is called more than once
      */
     protected boolean enableWatcher() {
         // Once randomDefault() becomes the default again, then this should only be actively disabled when