BuildMutators handle filtering

- remove the key filtering from the ScenarioLoader outside of the BuildMutators - update existing BuildMutators to return NOOP when key doesn't exist in config, allowing the BuildMutators to be responsible for enabling/disabling functionality. This will allow BuildMutators to work with specific Build types (ie Bazel) or all of them depending on the configuration. Signed-off-by: Alex Beggs <sunyal@gmail.com>
gradle · Feb 8, 2021 · c7db586 · c7db586
1 parent c10a294
commit c7db586
Show file tree

Hide file tree

Showing 7 changed files with 63 additions and 25 deletions.
diff --git a/src/main/java/org/gradle/profiler/ScenarioLoader.java b/src/main/java/org/gradle/profiler/ScenarioLoader.java
@@ -212,7 +212,6 @@ static List<ScenarioDefinition> loadScenarios(File scenarioFile, InvocationSetti
             String title = scenario.hasPath(TITLE) ? scenario.getString(TITLE) : null;
 
             List<BuildMutator> mutators = BUILD_MUTATOR_CONFIGURATORS.entrySet().stream()
-                .filter(entry -> scenario.hasPath(entry.getKey()))
                 .map(entry -> entry.getValue().configure(scenario, scenarioName, settings, entry.getKey()))
                 .filter(mutator -> mutator != BuildMutator.NOOP)
                 .collect(Collectors.toList());

diff --git a/...in/java/org/gradle/profiler/mutations/AbstractBuildMutatorWithoutOptionsConfigurator.java b/...in/java/org/gradle/profiler/mutations/AbstractBuildMutatorWithoutOptionsConfigurator.java
@@ -10,7 +10,9 @@ public abstract class AbstractBuildMutatorWithoutOptionsConfigurator implements
 
     @Override
     public BuildMutator configure(Config scenario, String scenarioName, InvocationSettings settings, String key) {
-        boolean enabled = scenario.getBoolean(key);
-        return enabled ? createBuildMutator(settings) : BuildMutator.NOOP;
+        return new HasPathBuildMutatorConfigurator(() -> {
+            boolean enabled = scenario.getBoolean(key);
+            return enabled ? createBuildMutator(settings) : BuildMutator.NOOP;
+        }).configure(scenario,scenarioName,settings,key);
     }
 }
diff --git a/src/main/java/org/gradle/profiler/mutations/AbstractCleanupMutator.java b/src/main/java/org/gradle/profiler/mutations/AbstractCleanupMutator.java
@@ -66,11 +66,14 @@ protected static void delete(File f) {
     protected static abstract class Configurator implements BuildMutatorConfigurator {
         @Override
         public BuildMutator configure(Config scenario, String scenarioName, InvocationSettings settings, String key) {
-            CleanupSchedule schedule = ConfigUtil.enumValue(scenario, key, CleanupSchedule.class, null);
-            if (schedule == null) {
-                throw new IllegalArgumentException("Schedule for cleanup is not specified");
-            }
-            return newInstance(scenario, scenarioName, settings, key, schedule);
+            return new HasPathBuildMutatorConfigurator(() -> {
+                CleanupSchedule schedule = ConfigUtil
+                    .enumValue(scenario, key, CleanupSchedule.class, null);
+                if (schedule == null) {
+                    throw new IllegalArgumentException("Schedule for cleanup is not specified");
+                }
+                return newInstance(scenario, scenarioName, settings, key, schedule);
+            }).configure(scenario, scenarioName, settings, key);
         }
 
         protected abstract BuildMutator newInstance(Config scenario, String scenarioName, InvocationSettings settings, String key, CleanupSchedule schedule);

diff --git a/src/main/java/org/gradle/profiler/mutations/FileChangeMutatorConfigurator.java b/src/main/java/org/gradle/profiler/mutations/FileChangeMutatorConfigurator.java
@@ -22,12 +22,15 @@ public FileChangeMutatorConfigurator(Class<? extends AbstractFileChangeMutator>
 
 	@Override
 	public BuildMutator configure(Config scenario, String scenarioName, InvocationSettings settings, String key) {
-		List<BuildMutator> mutatorsForKey = new ArrayList<>();
-		for (File sourceFileToChange : sourceFiles(scenario, scenarioName, settings.getProjectDir(), key)) {
-			mutatorsForKey.add(getBuildMutatorForFile(sourceFileToChange));
-		}
+        return new HasPathBuildMutatorConfigurator(() -> {
+            List<BuildMutator> mutatorsForKey = new ArrayList<>();
+            for (File sourceFileToChange : sourceFiles(scenario, scenarioName,
+                settings.getProjectDir(), key)) {
+                mutatorsForKey.add(getBuildMutatorForFile(sourceFileToChange));
+            }
 
-		return new CompositeBuildMutator(mutatorsForKey);
+            return new CompositeBuildMutator(mutatorsForKey);
+        }).configure(scenario, scenarioName, settings, key);
 	}
 
 	private BuildMutator getBuildMutatorForFile(File sourceFileToChange) {

diff --git a/src/main/java/org/gradle/profiler/mutations/GitCheckoutMutator.java b/src/main/java/org/gradle/profiler/mutations/GitCheckoutMutator.java
@@ -63,13 +63,16 @@ private void checkout(String target) {
 	public static class Configurator implements BuildMutatorConfigurator {
 		@Override
 		public BuildMutator configure(Config scenario, String scenarioName, InvocationSettings settings, String key) {
-			Config config = scenario.getConfig(key);
-			String cleanup = ConfigUtil.string(config, "cleanup", null);
-			String build = ConfigUtil.string(config, "build", null);
-			if (build == null) {
-				throw new IllegalArgumentException("No git-checkout target specified for build");
-			}
-			return new GitCheckoutMutator(settings.getProjectDir(), cleanup, build);
+            return new HasPathBuildMutatorConfigurator(() -> {
+                Config config = scenario.getConfig(key);
+                String cleanup = ConfigUtil.string(config, "cleanup", null);
+                String build = ConfigUtil.string(config, "build", null);
+                if (build == null) {
+                    throw new IllegalArgumentException(
+                        "No git-checkout target specified for build");
+                }
+                return new GitCheckoutMutator(settings.getProjectDir(), cleanup, build);
+            }).configure(scenario, scenarioName, settings, key);
 		}
 	}
 

diff --git a/src/main/java/org/gradle/profiler/mutations/GitRevertMutator.java b/src/main/java/org/gradle/profiler/mutations/GitRevertMutator.java
@@ -60,11 +60,13 @@ private void abortRevert() {
     public static class Configurator implements BuildMutatorConfigurator {
 		@Override
 		public BuildMutator configure(Config scenario, String scenarioName, InvocationSettings settings, String key) {
-			List<String> commits = ConfigUtil.strings(scenario, key);
-			if (commits.isEmpty()) {
-				throw new IllegalArgumentException("No commits specified for git-revert");
-			}
-			return new GitRevertMutator(settings.getProjectDir(), commits);
+            return new HasPathBuildMutatorConfigurator(() -> {
+                List<String> commits = ConfigUtil.strings(scenario, key);
+                if (commits.isEmpty()) {
+                    throw new IllegalArgumentException("No commits specified for git-revert");
+                }
+                return new GitRevertMutator(settings.getProjectDir(), commits);
+            }).configure(scenario, scenarioName, settings, key);
 		}
 	}
 

diff --git a/src/main/java/org/gradle/profiler/mutations/HasPathBuildMutatorConfigurator.java b/src/main/java/org/gradle/profiler/mutations/HasPathBuildMutatorConfigurator.java
@@ -0,0 +1,26 @@
+package org.gradle.profiler.mutations;
+
+import com.typesafe.config.Config;
+import java.util.function.Supplier;
+import org.gradle.profiler.BuildMutator;
+import org.gradle.profiler.InvocationSettings;
+
+class HasPathBuildMutatorConfigurator implements BuildMutatorConfigurator {
+
+    private Supplier<BuildMutator> configurator;
+
+    HasPathBuildMutatorConfigurator(Supplier<BuildMutator> configurator) {
+        this.configurator = configurator;
+    }
+
+    @Override
+    public BuildMutator configure(Config scenario, String scenarioName, InvocationSettings settings, String key) {
+        BuildMutator buildMutator;
+        if (scenario.hasPath(key)) {
+            buildMutator = configurator.get();
+        } else {
+            buildMutator = BuildMutator.NOOP;
+        }
+        return buildMutator;
+    }
+}