upthewaterspout closed pull request #9: Allowing JVM options to be set by test
URL: https://github.com/apache/geode-benchmarks/pull/9
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/harness/src/main/java/org/apache/geode/perftest/TestConfig.java 
b/harness/src/main/java/org/apache/geode/perftest/TestConfig.java
index 1d5a6f6..4386c77 100644
--- a/harness/src/main/java/org/apache/geode/perftest/TestConfig.java
+++ b/harness/src/main/java/org/apache/geode/perftest/TestConfig.java
@@ -19,6 +19,9 @@
 
 import java.io.Serializable;
 import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashMap;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
@@ -36,6 +39,7 @@
 
   private final WorkloadConfig workloadConfig = new WorkloadConfig();
   private Map<String, Integer> roles = new LinkedHashMap<>();
+  private Map<String, List<String>> jvmArgs = new HashMap<>();
   private List<TestStep> before = new ArrayList<>();
   private List<TestStep> workload = new ArrayList<>();
   private List<TestStep> after = new ArrayList<>();
@@ -153,11 +157,29 @@ public int getTotalJVMs() {
     return roles.values().stream().mapToInt(Integer::intValue).sum();
   }
 
+  /**
+   * Add JVM arguments used to launch JVMs for a particular role
+   *
+   * If multiple calls to this method are made for the same role, the new JVM 
arguments
+   * are appended to the existing JVM args
+   */
+  public void jvmArgs(String role, String ... jvmArgs) {
+    List<String> roleArgs = this.jvmArgs.computeIfAbsent(role, key -> new 
ArrayList<>());
+    roleArgs.addAll(Arrays.asList(jvmArgs));
+  }
+
+  public Map<String, List<String>> getJvmArgs() {
+    return Collections.unmodifiableMap(jvmArgs);
+  }
+
   public static class TestStep {
     private final Task task;
     private final String[] roles;
 
     public TestStep(Task task, String[] roles) {
+      if(roles == null || roles.length == 0) {
+        throw new IllegalStateException("Task " + task + " must be assigned to 
at least one role");
+      }
       this.task = task;
       this.roles = roles;
     }
diff --git 
a/harness/src/main/java/org/apache/geode/perftest/jvms/JVMLauncher.java 
b/harness/src/main/java/org/apache/geode/perftest/jvms/JVMLauncher.java
index eb26879..bc09436 100644
--- a/harness/src/main/java/org/apache/geode/perftest/jvms/JVMLauncher.java
+++ b/harness/src/main/java/org/apache/geode/perftest/jvms/JVMLauncher.java
@@ -40,28 +40,27 @@
       throws UnknownHostException {
     List<CompletableFuture<Void>> futures = new 
ArrayList<CompletableFuture<Void>>();
     for (JVMMapping entry : mapping) {
-      futures.add(launchWorker(infra, rmiPort, entry.getId(), entry.getNode(), 
libDir, entry.getOutputDir()));
+      CompletableFuture<Void> future = launchWorker(infra, rmiPort, libDir, 
entry);
+      futures.add(future);
     }
     return CompletableFuture.allOf(futures.toArray(new CompletableFuture[0]));
   }
 
-  CompletableFuture<Void> launchWorker(Infrastructure infra, int rmiPort,
-                                       int id, final Infrastructure.Node node, 
String libDir,
-                                       String outputDir)
+  CompletableFuture<Void> launchWorker(Infrastructure infra, int rmiPort, 
String libDir, JVMMapping jvmConfig)
       throws UnknownHostException {
-    String[] shellCommand = 
buildCommand(InetAddress.getLocalHost().getHostAddress(), rmiPort, id,
-        libDir, outputDir);
+    String[] shellCommand = 
buildCommand(InetAddress.getLocalHost().getHostAddress(), rmiPort, libDir, 
jvmConfig);
+
     CompletableFuture<Void> future = new CompletableFuture<Void>();
-    Thread thread = new Thread("Worker " + node.getAddress()) {
+    Thread thread = new Thread("Worker " + jvmConfig.getNode().getAddress()) {
       public void run() {
 
         try {
-          int result = infra.onNode(node, shellCommand);
+          int result = infra.onNode(jvmConfig.getNode(), shellCommand);
           if (result != 0) {
             logger.error("ChildJVM exited with error code " + result);
           }
         } catch (Throwable t) {
-          logger.error("Launching " + String.join(" ", shellCommand) + " on " 
+ node + "Failed.", t);
+          logger.error("Launching " + String.join(" ", shellCommand) + " on " 
+ jvmConfig.getNode() + "Failed.", t);
         } finally {
           future.complete(null);
         }
@@ -72,8 +71,7 @@ public void run() {
     return future;
   }
 
-  String[] buildCommand(String rmiHost, int rmiPort, int id, String libDir,
-                        String outputDir) {
+  String[] buildCommand(String rmiHost, int rmiPort, String libDir, JVMMapping 
jvmConfig) {
 
     List<String> command = new ArrayList<String>();
     command.add("java");
@@ -81,8 +79,9 @@ public void run() {
     command.add(libDir + "/*");
     command.add("-D" + RemoteJVMFactory.RMI_HOST + "=" + rmiHost);
     command.add("-D" + RemoteJVMFactory.RMI_PORT_PROPERTY + "=" + rmiPort);
-    command.add("-D" + RemoteJVMFactory.JVM_ID + "=" + id);
-    command.add("-D" + RemoteJVMFactory.OUTPUT_DIR + "=" + outputDir);
+    command.add("-D" + RemoteJVMFactory.JVM_ID + "=" + jvmConfig.getId());
+    command.add("-D" + RemoteJVMFactory.OUTPUT_DIR + "=" + 
jvmConfig.getOutputDir());
+    command.addAll(jvmConfig.getJvmArgs());
     command.add(ChildJVM.class.getName());
 
     return command.toArray(new String[0]);
diff --git 
a/harness/src/main/java/org/apache/geode/perftest/jvms/JVMMapping.java 
b/harness/src/main/java/org/apache/geode/perftest/jvms/JVMMapping.java
index 0ae6236..43ade44 100644
--- a/harness/src/main/java/org/apache/geode/perftest/jvms/JVMMapping.java
+++ b/harness/src/main/java/org/apache/geode/perftest/jvms/JVMMapping.java
@@ -18,6 +18,7 @@
 package org.apache.geode.perftest.jvms;
 
 import java.io.Serializable;
+import java.util.List;
 
 import org.apache.geode.perftest.infrastructure.Infrastructure;
 
@@ -25,11 +26,14 @@
   private final Infrastructure.Node node;
   private final String role;
   private final int id;
+  private final List<String> jvmArgs;
 
-  public JVMMapping(Infrastructure.Node node, String role, int id) {
+  public JVMMapping(Infrastructure.Node node, String role, int id,
+                    List<String> jvmArgs) {
     this.node = node;
     this.role = role;
     this.id = id;
+    this.jvmArgs = jvmArgs;
   }
 
   public Infrastructure.Node getNode() {
@@ -47,4 +51,8 @@ public int getId() {
   public String getOutputDir() {
     return "output/" + role + "-" + id;
   }
+
+  public List<String> getJvmArgs() {
+    return jvmArgs;
+  }
 }
diff --git 
a/harness/src/main/java/org/apache/geode/perftest/jvms/RemoteJVMFactory.java 
b/harness/src/main/java/org/apache/geode/perftest/jvms/RemoteJVMFactory.java
index af18ddc..63eb5ad 100644
--- a/harness/src/main/java/org/apache/geode/perftest/jvms/RemoteJVMFactory.java
+++ b/harness/src/main/java/org/apache/geode/perftest/jvms/RemoteJVMFactory.java
@@ -18,6 +18,7 @@
 package org.apache.geode.perftest.jvms;
 
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
@@ -74,9 +75,11 @@ public RemoteJVMFactory(InfrastructureFactory 
infrastructureFactory) {
    * @param roles The JVMs to start. Keys a roles and values are the number
    * of JVMs in that role.
    *
+   * @param jvmArgs
    * @return a {@link RemoteJVMs} object used to access the JVMs through RMI
    */
-  public RemoteJVMs launch(Map<String, Integer> roles) throws Exception {
+  public RemoteJVMs launch(Map<String, Integer> roles,
+                           Map<String, List<String>> jvmArgs) throws Exception 
{
     int numWorkers = roles.values().stream().mapToInt(Integer::intValue).sum();
 
     Infrastructure infra = infrastructureFactory.create(numWorkers);
@@ -87,7 +90,7 @@ public RemoteJVMs launch(Map<String, Integer> roles) throws 
Exception {
       throw new IllegalStateException("Too few nodes for test. Need " + 
numWorkers + ", have " + nodes.size());
     }
 
-    List<JVMMapping> mapping = mapRolesToNodes(roles, nodes);
+    List<JVMMapping> mapping = mapRolesToNodes(roles, nodes, jvmArgs);
 
     Controller controller = controllerFactory.createController(new 
SharedContext(mapping), numWorkers);
 
@@ -108,9 +111,8 @@ public InfrastructureFactory getInfrastructureFactory() {
   }
 
   private List<JVMMapping> mapRolesToNodes(Map<String, Integer> roles,
-                                           Set<Infrastructure.Node> nodes) {
-
-
+                                           Set<Infrastructure.Node> nodes,
+                                           Map<String, List<String>> jvmArgs) {
     List<JVMMapping> mapping = new ArrayList<>();
     Iterator<Infrastructure.Node> nodeItr = nodes.iterator();
 
@@ -118,7 +120,9 @@ public InfrastructureFactory getInfrastructureFactory() {
     for(Map.Entry<String, Integer> roleEntry : roles.entrySet()) {
       for(int i = 0; i < roleEntry.getValue(); i++) {
         Infrastructure.Node node = nodeItr.next();
-        mapping.add(new JVMMapping(node, roleEntry.getKey(), id++));
+        String role = roleEntry.getKey();
+        List<String> roleArgs = jvmArgs.getOrDefault(role, 
Collections.emptyList());
+        mapping.add(new JVMMapping(node, role, id++, roleArgs));
       }
 
     }
diff --git 
a/harness/src/main/java/org/apache/geode/perftest/runner/DefaultTestRunner.java 
b/harness/src/main/java/org/apache/geode/perftest/runner/DefaultTestRunner.java
index 07ac6b5..56ce3a0 100644
--- 
a/harness/src/main/java/org/apache/geode/perftest/runner/DefaultTestRunner.java
+++ 
b/harness/src/main/java/org/apache/geode/perftest/runner/DefaultTestRunner.java
@@ -73,10 +73,11 @@ protected void runTest(TestConfig config)
 
 
     Map<String, Integer> roles = config.getRoles();
+    Map<String, List<String>> jvmArgs = config.getJvmArgs();
 
     logger.info("Lauching JVMs...");
     //launch JVMs in parallel, hook them up
-    try (RemoteJVMs remoteJVMs = remoteJvmFactory.launch(roles)) {
+    try (RemoteJVMs remoteJVMs = remoteJvmFactory.launch(roles, jvmArgs)) {
 
       logger.info("Starting before tasks...");
       runTasks(config.getBefore(), remoteJVMs);
diff --git 
a/harness/src/test/java/org/apache/geode/perftest/TestRunnerIntegrationTest.java
 
b/harness/src/test/java/org/apache/geode/perftest/TestRunnerIntegrationTest.java
index 12bdbe0..e49a63a 100644
--- 
a/harness/src/test/java/org/apache/geode/perftest/TestRunnerIntegrationTest.java
+++ 
b/harness/src/test/java/org/apache/geode/perftest/TestRunnerIntegrationTest.java
@@ -85,6 +85,19 @@ public void generatesOutputDirectoryPerBenchmark() throws 
Exception {
     assertEquals(1, outputFiles.count());
   }
 
+  @Test
+  public void configuresJVMOptions() throws Exception {
+    runner.runTest(testConfig -> {
+      testConfig.name(SAMPLE_BENCHMARK);
+      testConfig.role("all", 1);
+      testConfig.jvmArgs("all", "-Dprop1=true", "-Dprop2=5");
+      testConfig.before(context -> {
+        assertTrue("Expecting system property to be set in launched JVM, but 
it was not present.", Boolean.getBoolean("prop1"));
+        assertEquals("Expecting system property to be set in launched JVM, but 
it was not present.", 5, Integer.getInteger("prop2").intValue());
+      }, "all");
+    });
+  }
+
   private Predicate<Path> nameMatches(String sensorOutputFile) {
     return path -> path.toString().contains(sensorOutputFile);
   }
diff --git 
a/harness/src/test/java/org/apache/geode/perftest/TestRunnerJUnitTest.java 
b/harness/src/test/java/org/apache/geode/perftest/TestRunnerJUnitTest.java
index 2b1027b..1349927 100644
--- a/harness/src/test/java/org/apache/geode/perftest/TestRunnerJUnitTest.java
+++ b/harness/src/test/java/org/apache/geode/perftest/TestRunnerJUnitTest.java
@@ -18,22 +18,20 @@
 package org.apache.geode.perftest;
 
 import static org.mockito.Matchers.any;
-import static org.mockito.Matchers.anyInt;
 import static org.mockito.Matchers.eq;
 import static org.mockito.Mockito.inOrder;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
-import java.io.File;
+import java.util.Collections;
 
+import org.assertj.core.api.Assertions;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.TemporaryFolder;
 import org.mockito.InOrder;
 
-import org.apache.geode.perftest.infrastructure.InfrastructureFactory;
-import org.apache.geode.perftest.infrastructure.Infrastructure;
 import org.apache.geode.perftest.jvms.RemoteJVMFactory;
 import org.apache.geode.perftest.jvms.RemoteJVMs;
 import org.apache.geode.perftest.runner.DefaultTestRunner;
@@ -49,7 +47,7 @@ public void testRunnerRunsBeforeAndAfterTasks() throws 
Exception {
     RemoteJVMFactory remoteJvmFactory = mock(RemoteJVMFactory.class);
 
     RemoteJVMs remoteJVMs = mock(RemoteJVMs.class);
-    when(remoteJvmFactory.launch(any())).thenReturn(remoteJVMs);
+    when(remoteJvmFactory.launch(any(), any())).thenReturn(remoteJVMs);
 
     TestRunner runner = new DefaultTestRunner(remoteJvmFactory,
         folder.newFolder());
@@ -73,4 +71,25 @@ public void testRunnerRunsBeforeAndAfterTasks() throws 
Exception {
     inOrder.verify(remoteJVMs).execute(eq(after), any());
   }
 
+  @Test
+  public void requiresAtLeastOneRole() throws Exception {
+
+    RemoteJVMFactory remoteJvmFactory = mock(RemoteJVMFactory.class);
+
+    RemoteJVMs remoteJVMs = mock(RemoteJVMs.class);
+    when(remoteJvmFactory.launch(any(), any())).thenReturn(remoteJVMs);
+
+    TestRunner runner = new DefaultTestRunner(remoteJvmFactory,
+        folder.newFolder());
+
+    Task before = mock(Task.class);
+
+    PerformanceTest test = config -> {
+      config.name("SampleBenchmark");
+      config.role("before", 1);
+
+      config.before(before);
+    };
+    Assertions.assertThatThrownBy(() -> 
runner.runTest(test)).isInstanceOf(IllegalStateException.class);
+  }
 }
\ No newline at end of file
diff --git 
a/harness/src/test/java/org/apache/geode/perftest/jvms/RemoteJVMFactoryIntegrationTest.java
 
b/harness/src/test/java/org/apache/geode/perftest/jvms/RemoteJVMFactoryIntegrationTest.java
index 03e2931..b9476bb 100644
--- 
a/harness/src/test/java/org/apache/geode/perftest/jvms/RemoteJVMFactoryIntegrationTest.java
+++ 
b/harness/src/test/java/org/apache/geode/perftest/jvms/RemoteJVMFactoryIntegrationTest.java
@@ -27,7 +27,6 @@
 import org.junit.Test;
 import org.junit.rules.TemporaryFolder;
 
-import org.apache.geode.perftest.infrastructure.local.LocalInfrastructure;
 import 
org.apache.geode.perftest.infrastructure.local.LocalInfrastructureFactory;
 
 public class RemoteJVMFactoryIntegrationTest {
@@ -38,7 +37,7 @@
   public void canExecuteCodeOnWorker() throws Exception {
     RemoteJVMFactory remoteJvmFactory = new RemoteJVMFactory(new 
LocalInfrastructureFactory());
     Map<String, Integer> roles = Collections.singletonMap("worker", 1);
-    try (RemoteJVMs jvms = remoteJvmFactory.launch(roles)) {
+    try (RemoteJVMs jvms = remoteJvmFactory.launch(roles, 
Collections.emptyMap())) {
       File tempFile = new File(temporaryFolder.newFolder(), 
"tmpfile").getAbsoluteFile();
       jvms.execute(context -> {
         tempFile.createNewFile();
diff --git 
a/harness/src/test/java/org/apache/geode/perftest/jvms/RemoteJVMFactoryTest.java
 
b/harness/src/test/java/org/apache/geode/perftest/jvms/RemoteJVMFactoryTest.java
index cb576f0..d8337e4 100644
--- 
a/harness/src/test/java/org/apache/geode/perftest/jvms/RemoteJVMFactoryTest.java
+++ 
b/harness/src/test/java/org/apache/geode/perftest/jvms/RemoteJVMFactoryTest.java
@@ -74,7 +74,7 @@ public void launchMethodCreatesControllerAndLaunchesNodes() 
throws Exception {
 
     when(controller.waitForWorkers(anyInt(), any())).thenReturn(true);
 
-    factory.launch(roles);
+    factory.launch(roles, Collections.emptyMap());
 
     InOrder inOrder = inOrder(controller, controllerFactory, jvmLauncher, 
classPathCopier, infra);
 
diff --git 
a/harness/src/test/java/org/apache/geode/perftest/runner/SharedContextTest.java 
b/harness/src/test/java/org/apache/geode/perftest/runner/SharedContextTest.java
index 7784105..33d7721 100644
--- 
a/harness/src/test/java/org/apache/geode/perftest/runner/SharedContextTest.java
+++ 
b/harness/src/test/java/org/apache/geode/perftest/runner/SharedContextTest.java
@@ -24,6 +24,7 @@
 import java.net.InetAddress;
 import java.net.UnknownHostException;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.Set;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
@@ -42,10 +43,10 @@ public void getHostsForRoleShouldReturnCorrectList() throws 
UnknownHostException
     InetAddress host2 = InetAddress.getByName("127.0.0.2");
     Infrastructure.Node node1 = mock(Infrastructure.Node.class);
     when(node1.getAddress()).thenReturn(host1);
-    JVMMapping mapping1 = new JVMMapping(node1, "role", 1);
+    JVMMapping mapping1 = new JVMMapping(node1, "role", 1, 
Collections.emptyList());
     Infrastructure.Node node2 = mock(Infrastructure.Node.class);
     when(node2.getAddress()).thenReturn(host2);
-    JVMMapping mapping2 = new JVMMapping(node2, "role", 2);
+    JVMMapping mapping2 = new JVMMapping(node2, "role", 2, 
Collections.emptyList());
 
     SharedContext context = new SharedContext(Arrays.asList(mapping1, 
mapping2));
 


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

Reply via email to