You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@geode.apache.org by GitBox <gi...@apache.org> on 2020/06/02 17:19:04 UTC

[GitHub] [geode] kirklund opened a new pull request #5196: GEODE-8197: Add launcher acceptance testing for custom logging config

kirklund opened a new pull request #5196:
URL: https://github.com/apache/geode/pull/5196


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [geode] kirklund commented on a change in pull request #5196: GEODE-8197: Add launcher acceptance testing for custom logging config

Posted by GitBox <gi...@apache.org>.
kirklund commented on a change in pull request #5196:
URL: https://github.com/apache/geode/pull/5196#discussion_r436186564



##########
File path: geode-assembly/src/acceptanceTest/java/org/apache/geode/launchers/LocatorLauncherWithPulseAndCustomLogConfigAcceptanceTest.java
##########
@@ -0,0 +1,268 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.launchers;
+
+import static java.nio.file.Files.copy;
+import static org.apache.geode.test.awaitility.GeodeAwaitility.await;
+import static org.apache.geode.test.util.ResourceUtils.createFileFromResource;
+import static org.apache.geode.test.util.ResourceUtils.getResource;
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.io.File;
+import java.nio.file.Path;
+import java.util.concurrent.TimeUnit;
+
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Ignore;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+
+import org.apache.geode.internal.AvailablePortHelper;
+import org.apache.geode.test.assertj.LogFileAssert;
+import org.apache.geode.test.junit.rules.RequiresGeodeHome;
+
+public class LocatorLauncherWithPulseAndCustomLogConfigAcceptanceTest {
+
+  private static final String CONFIG_WITH_GEODE_PLUGINS_FILE_NAME =
+      "LocatorLauncherWithPulseAndCustomLogConfigAcceptanceTestWithGeodePlugins.xml";
+  private static final String CONFIG_WITHOUT_GEODE_PLUGINS_FILE_NAME =
+      "LocatorLauncherWithPulseAndCustomLogConfigAcceptanceTestWithoutGeodePlugins.xml";
+  private static final String LOCATOR_NAME = "the-locator";
+
+  private int locatorPort;
+  private int httpServicePort;
+  private Path configWithGeodePluginsFile;
+  private Path configWithoutGeodePluginsFile;
+  private Process locator;
+  private Path geodeDependencies;
+  private Path stdoutFile;
+  private Path locatorLogFile;
+  private Path pulseLogFile;
+
+  @Rule
+  public RequiresGeodeHome requiresGeodeHome = new RequiresGeodeHome();
+  @Rule
+  public TemporaryFolder temporaryFolder = new TemporaryFolder();
+
+  @Before
+  public void setUpLogConfigFiles() {
+    configWithGeodePluginsFile = createFileFromResource(
+        getResource(CONFIG_WITH_GEODE_PLUGINS_FILE_NAME), temporaryFolder.getRoot(),
+        CONFIG_WITH_GEODE_PLUGINS_FILE_NAME)
+            .toPath();
+
+    configWithoutGeodePluginsFile = createFileFromResource(
+        getResource(CONFIG_WITHOUT_GEODE_PLUGINS_FILE_NAME), temporaryFolder.getRoot(),
+        CONFIG_WITHOUT_GEODE_PLUGINS_FILE_NAME)
+            .toPath();
+  }
+
+  @Before
+  public void setUpGeodeDependencies() {
+    Path geodeHome = requiresGeodeHome.getGeodeHome().toPath();
+    geodeDependencies = geodeHome.resolve("lib/geode-dependencies.jar");
+
+    assertThat(geodeDependencies).exists();
+  }
+
+  @Before
+  public void setUpOutputFiles() {
+    stdoutFile = temporaryFolder.getRoot().toPath().resolve("stdout.txt");
+    locatorLogFile = temporaryFolder.getRoot().toPath().resolve(LOCATOR_NAME + ".log");
+    pulseLogFile = temporaryFolder.getRoot().toPath().resolve("pulse.log");
+  }
+
+  @Before
+  public void setUpRandomPorts() {
+    int[] ports = AvailablePortHelper.getRandomAvailableTCPPorts(2);
+    locatorPort = ports[0];
+    httpServicePort = ports[1];
+  }
+
+  @After
+  public void stopLocator() throws Exception {
+    if (locator != null) {
+      locator.destroyForcibly().waitFor(4, TimeUnit.SECONDS);
+    }
+  }
+
+  @Test
+  public void locatorLauncherUsesSpecifiedConfigFileWithoutGeodePlugins() throws Exception {
+    ProcessBuilder processBuilder = new ProcessBuilder()
+        .redirectErrorStream(true)
+        .redirectOutput(stdoutFile.toFile())
+        .directory(temporaryFolder.getRoot())
+        .command("java",
+            "-Dgemfire.http-service-port=" + httpServicePort,
+            "-Dgemfire.jmx-manager-start=true",
+            "-Djava.awt.headless=true",
+            "-Dlog4j.configurationFile=" + configWithoutGeodePluginsFile.toAbsolutePath(),
+            "-cp", geodeDependencies.toFile().getAbsolutePath(),
+            "org.apache.geode.distributed.LocatorLauncher", "start", LOCATOR_NAME,
+            "--port", String.valueOf(locatorPort));
+
+    System.out.println("Launching command: " + processBuilder.command());
+
+    locator = processBuilder
+        .start();
+
+    assertThat(locator.isAlive()).isTrue();
+
+    await().untilAsserted(() -> {
+      assertThat(locatorLogFile)
+          .doesNotExist();
+
+      assertThat(pulseLogFile)
+          .doesNotExist();
+
+      LogFileAssert.assertThat(stdoutFile.toFile())
+          .exists()
+          .contains("Located war: geode-pulse")
+          .contains("Adding webapp /pulse")
+          .contains("Starting server location for Distribution Locator")
+          .doesNotContain("geode-pulse war file was not found")
+          .doesNotContain("java.lang.IllegalStateException: No factory method found for class");
+    });
+  }
+
+  @Test
+  public void locatorLauncherUsesConfigFileInClasspathWithoutGeodePlugins() throws Exception {
+    copy(configWithoutGeodePluginsFile, temporaryFolder.getRoot().toPath().resolve("log4j2.xml"));
+
+    String classpath = temporaryFolder.getRoot().getAbsolutePath() + File.pathSeparator +
+        geodeDependencies.toFile().getAbsolutePath();
+
+    ProcessBuilder processBuilder = new ProcessBuilder()
+        .redirectErrorStream(true)
+        .redirectOutput(stdoutFile.toFile())
+        .directory(temporaryFolder.getRoot())
+        .command("java",
+            "-Dgemfire.http-service-port=" + httpServicePort,
+            "-Dgemfire.jmx-manager-start=true",
+            "-Djava.awt.headless=true",
+            "-cp", classpath,
+            "org.apache.geode.distributed.LocatorLauncher", "start", LOCATOR_NAME,
+            "--port", String.valueOf(locatorPort));
+
+    System.out.println("Launching command: " + processBuilder.command());
+
+    locator = processBuilder
+        .start();
+
+    assertThat(locator.isAlive()).isTrue();
+
+    await().untilAsserted(() -> {
+      assertThat(locatorLogFile)
+          .doesNotExist();
+
+      assertThat(pulseLogFile)
+          .doesNotExist();
+
+      LogFileAssert.assertThat(stdoutFile.toFile())
+          .exists()
+          .contains("Located war: geode-pulse")
+          .contains("Adding webapp /pulse")
+          .contains("Starting server location for Distribution Locator")
+          .doesNotContain("geode-pulse war file was not found")
+          .doesNotContain("java.lang.IllegalStateException: No factory method found for class");
+    });
+  }
+
+  @Test
+  @Ignore("GEODE-8197")
+  public void locatorLauncherUsesSpecifiedConfigFileWithGeodePlugins() throws Exception {
+    ProcessBuilder processBuilder = new ProcessBuilder()
+        .redirectErrorStream(true)
+        .redirectOutput(stdoutFile.toFile())
+        .directory(temporaryFolder.getRoot())
+        .command("java",
+            "-Dgemfire.http-service-port=" + httpServicePort,
+            "-Dgemfire.jmx-manager-start=true",
+            "-Djava.awt.headless=true",
+            "-Dlog4j.configurationFile=" + configWithGeodePluginsFile.toAbsolutePath(),
+            "-cp", geodeDependencies.toFile().getAbsolutePath(),
+            "org.apache.geode.distributed.LocatorLauncher", "start", LOCATOR_NAME,
+            "--port", String.valueOf(locatorPort));
+
+    System.out.println("Launching command: " + processBuilder.command());
+
+    locator = processBuilder
+        .start();
+
+    assertThat(locator.isAlive()).isTrue();
+
+    await().untilAsserted(() -> {
+      assertThat(locatorLogFile)
+          .doesNotExist();
+
+      assertThat(pulseLogFile)
+          .doesNotExist();
+
+      LogFileAssert.assertThat(stdoutFile.toFile())
+          .exists()
+          .contains("Located war: geode-pulse")
+          .contains("Adding webapp /pulse")
+          .contains("Starting server location for Distribution Locator")
+          .doesNotContain("geode-pulse war file was not found")
+          .doesNotContain("java.lang.IllegalStateException: No factory method found for class");
+    });
+  }
+
+  @Test
+  @Ignore("GEODE-8197")

Review comment:
       Yes. This PR is only creating new tests. I'll follow up in the future with a fix.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [geode] kirklund commented on a change in pull request #5196: GEODE-8197: Add launcher acceptance testing for custom logging config

Posted by GitBox <gi...@apache.org>.
kirklund commented on a change in pull request #5196:
URL: https://github.com/apache/geode/pull/5196#discussion_r436863494



##########
File path: geode-assembly/src/acceptanceTest/java/org/apache/geode/launchers/LocatorLauncherWithCustomLogConfigAcceptanceTest.java
##########
@@ -0,0 +1,175 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.launchers;
+
+import static org.apache.geode.test.awaitility.GeodeAwaitility.await;
+import static org.apache.geode.test.util.ResourceUtils.createFileFromResource;
+import static org.apache.geode.test.util.ResourceUtils.getResource;
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.io.File;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.concurrent.TimeUnit;
+
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+
+import org.apache.geode.internal.AvailablePortHelper;
+import org.apache.geode.test.assertj.LogFileAssert;
+import org.apache.geode.test.junit.rules.RequiresGeodeHome;
+
+public class LocatorLauncherWithCustomLogConfigAcceptanceTest {
+
+  private static final String CONFIG_FILE_NAME =
+      "LocatorLauncherWithCustomLogConfigAcceptanceTest.xml";
+  private static final String LOCATOR_NAME = "the-locator";
+
+  private int locatorPort;
+  private Path configFile;
+  private Process locator;
+  private Path geodeDependencies;
+  private Path stdoutFile;
+  private Path locatorLogFile;
+  private Path pulseLogFile;
+
+  @Rule
+  public RequiresGeodeHome requiresGeodeHome = new RequiresGeodeHome();
+  @Rule
+  public TemporaryFolder temporaryFolder = new TemporaryFolder();
+
+  @Before
+  public void setUpLogConfigFile() {
+    configFile = createFileFromResource(getResource(CONFIG_FILE_NAME), temporaryFolder.getRoot(),
+        CONFIG_FILE_NAME)
+            .toPath();
+  }
+
+  @Before
+  public void setUpGeodeDependencies() {
+    Path geodeHome = requiresGeodeHome.getGeodeHome().toPath();
+    geodeDependencies = geodeHome.resolve("lib/geode-dependencies.jar");
+
+    assertThat(geodeDependencies).exists();
+  }
+
+  @Before
+  public void setUpOutputFiles() {
+    stdoutFile = temporaryFolder.getRoot().toPath().resolve("stdout.txt");
+    locatorLogFile = temporaryFolder.getRoot().toPath().resolve(LOCATOR_NAME + ".log");
+    pulseLogFile = temporaryFolder.getRoot().toPath().resolve("pulse.log");
+  }
+
+  @Before
+  public void setUpRandomPorts() {
+    locatorPort = AvailablePortHelper.getRandomAvailableTCPPort();
+  }
+
+  @After
+  public void stopLocator() throws Exception {
+    if (locator != null) {
+      locator.destroyForcibly().waitFor(4, TimeUnit.SECONDS);
+    }
+  }
+
+  @Test
+  public void locatorLauncherUsesLog4jConfigurationFile() throws Exception {
+    ProcessBuilder processBuilder = new ProcessBuilder()
+        .redirectErrorStream(true)
+        .redirectOutput(stdoutFile.toFile())
+        .directory(temporaryFolder.getRoot())
+        .command("java",

Review comment:
       Thanks Owen!




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [geode] kirklund commented on a change in pull request #5196: GEODE-8197: Add launcher acceptance testing for custom logging config

Posted by GitBox <gi...@apache.org>.
kirklund commented on a change in pull request #5196:
URL: https://github.com/apache/geode/pull/5196#discussion_r436186564



##########
File path: geode-assembly/src/acceptanceTest/java/org/apache/geode/launchers/LocatorLauncherWithPulseAndCustomLogConfigAcceptanceTest.java
##########
@@ -0,0 +1,268 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.launchers;
+
+import static java.nio.file.Files.copy;
+import static org.apache.geode.test.awaitility.GeodeAwaitility.await;
+import static org.apache.geode.test.util.ResourceUtils.createFileFromResource;
+import static org.apache.geode.test.util.ResourceUtils.getResource;
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.io.File;
+import java.nio.file.Path;
+import java.util.concurrent.TimeUnit;
+
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Ignore;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+
+import org.apache.geode.internal.AvailablePortHelper;
+import org.apache.geode.test.assertj.LogFileAssert;
+import org.apache.geode.test.junit.rules.RequiresGeodeHome;
+
+public class LocatorLauncherWithPulseAndCustomLogConfigAcceptanceTest {
+
+  private static final String CONFIG_WITH_GEODE_PLUGINS_FILE_NAME =
+      "LocatorLauncherWithPulseAndCustomLogConfigAcceptanceTestWithGeodePlugins.xml";
+  private static final String CONFIG_WITHOUT_GEODE_PLUGINS_FILE_NAME =
+      "LocatorLauncherWithPulseAndCustomLogConfigAcceptanceTestWithoutGeodePlugins.xml";
+  private static final String LOCATOR_NAME = "the-locator";
+
+  private int locatorPort;
+  private int httpServicePort;
+  private Path configWithGeodePluginsFile;
+  private Path configWithoutGeodePluginsFile;
+  private Process locator;
+  private Path geodeDependencies;
+  private Path stdoutFile;
+  private Path locatorLogFile;
+  private Path pulseLogFile;
+
+  @Rule
+  public RequiresGeodeHome requiresGeodeHome = new RequiresGeodeHome();
+  @Rule
+  public TemporaryFolder temporaryFolder = new TemporaryFolder();
+
+  @Before
+  public void setUpLogConfigFiles() {
+    configWithGeodePluginsFile = createFileFromResource(
+        getResource(CONFIG_WITH_GEODE_PLUGINS_FILE_NAME), temporaryFolder.getRoot(),
+        CONFIG_WITH_GEODE_PLUGINS_FILE_NAME)
+            .toPath();
+
+    configWithoutGeodePluginsFile = createFileFromResource(
+        getResource(CONFIG_WITHOUT_GEODE_PLUGINS_FILE_NAME), temporaryFolder.getRoot(),
+        CONFIG_WITHOUT_GEODE_PLUGINS_FILE_NAME)
+            .toPath();
+  }
+
+  @Before
+  public void setUpGeodeDependencies() {
+    Path geodeHome = requiresGeodeHome.getGeodeHome().toPath();
+    geodeDependencies = geodeHome.resolve("lib/geode-dependencies.jar");
+
+    assertThat(geodeDependencies).exists();
+  }
+
+  @Before
+  public void setUpOutputFiles() {
+    stdoutFile = temporaryFolder.getRoot().toPath().resolve("stdout.txt");
+    locatorLogFile = temporaryFolder.getRoot().toPath().resolve(LOCATOR_NAME + ".log");
+    pulseLogFile = temporaryFolder.getRoot().toPath().resolve("pulse.log");
+  }
+
+  @Before
+  public void setUpRandomPorts() {
+    int[] ports = AvailablePortHelper.getRandomAvailableTCPPorts(2);
+    locatorPort = ports[0];
+    httpServicePort = ports[1];
+  }
+
+  @After
+  public void stopLocator() throws Exception {
+    if (locator != null) {
+      locator.destroyForcibly().waitFor(4, TimeUnit.SECONDS);
+    }
+  }
+
+  @Test
+  public void locatorLauncherUsesSpecifiedConfigFileWithoutGeodePlugins() throws Exception {
+    ProcessBuilder processBuilder = new ProcessBuilder()
+        .redirectErrorStream(true)
+        .redirectOutput(stdoutFile.toFile())
+        .directory(temporaryFolder.getRoot())
+        .command("java",
+            "-Dgemfire.http-service-port=" + httpServicePort,
+            "-Dgemfire.jmx-manager-start=true",
+            "-Djava.awt.headless=true",
+            "-Dlog4j.configurationFile=" + configWithoutGeodePluginsFile.toAbsolutePath(),
+            "-cp", geodeDependencies.toFile().getAbsolutePath(),
+            "org.apache.geode.distributed.LocatorLauncher", "start", LOCATOR_NAME,
+            "--port", String.valueOf(locatorPort));
+
+    System.out.println("Launching command: " + processBuilder.command());
+
+    locator = processBuilder
+        .start();
+
+    assertThat(locator.isAlive()).isTrue();
+
+    await().untilAsserted(() -> {
+      assertThat(locatorLogFile)
+          .doesNotExist();
+
+      assertThat(pulseLogFile)
+          .doesNotExist();
+
+      LogFileAssert.assertThat(stdoutFile.toFile())
+          .exists()
+          .contains("Located war: geode-pulse")
+          .contains("Adding webapp /pulse")
+          .contains("Starting server location for Distribution Locator")
+          .doesNotContain("geode-pulse war file was not found")
+          .doesNotContain("java.lang.IllegalStateException: No factory method found for class");
+    });
+  }
+
+  @Test
+  public void locatorLauncherUsesConfigFileInClasspathWithoutGeodePlugins() throws Exception {
+    copy(configWithoutGeodePluginsFile, temporaryFolder.getRoot().toPath().resolve("log4j2.xml"));
+
+    String classpath = temporaryFolder.getRoot().getAbsolutePath() + File.pathSeparator +
+        geodeDependencies.toFile().getAbsolutePath();
+
+    ProcessBuilder processBuilder = new ProcessBuilder()
+        .redirectErrorStream(true)
+        .redirectOutput(stdoutFile.toFile())
+        .directory(temporaryFolder.getRoot())
+        .command("java",
+            "-Dgemfire.http-service-port=" + httpServicePort,
+            "-Dgemfire.jmx-manager-start=true",
+            "-Djava.awt.headless=true",
+            "-cp", classpath,
+            "org.apache.geode.distributed.LocatorLauncher", "start", LOCATOR_NAME,
+            "--port", String.valueOf(locatorPort));
+
+    System.out.println("Launching command: " + processBuilder.command());
+
+    locator = processBuilder
+        .start();
+
+    assertThat(locator.isAlive()).isTrue();
+
+    await().untilAsserted(() -> {
+      assertThat(locatorLogFile)
+          .doesNotExist();
+
+      assertThat(pulseLogFile)
+          .doesNotExist();
+
+      LogFileAssert.assertThat(stdoutFile.toFile())
+          .exists()
+          .contains("Located war: geode-pulse")
+          .contains("Adding webapp /pulse")
+          .contains("Starting server location for Distribution Locator")
+          .doesNotContain("geode-pulse war file was not found")
+          .doesNotContain("java.lang.IllegalStateException: No factory method found for class");
+    });
+  }
+
+  @Test
+  @Ignore("GEODE-8197")
+  public void locatorLauncherUsesSpecifiedConfigFileWithGeodePlugins() throws Exception {
+    ProcessBuilder processBuilder = new ProcessBuilder()
+        .redirectErrorStream(true)
+        .redirectOutput(stdoutFile.toFile())
+        .directory(temporaryFolder.getRoot())
+        .command("java",
+            "-Dgemfire.http-service-port=" + httpServicePort,
+            "-Dgemfire.jmx-manager-start=true",
+            "-Djava.awt.headless=true",
+            "-Dlog4j.configurationFile=" + configWithGeodePluginsFile.toAbsolutePath(),
+            "-cp", geodeDependencies.toFile().getAbsolutePath(),
+            "org.apache.geode.distributed.LocatorLauncher", "start", LOCATOR_NAME,
+            "--port", String.valueOf(locatorPort));
+
+    System.out.println("Launching command: " + processBuilder.command());
+
+    locator = processBuilder
+        .start();
+
+    assertThat(locator.isAlive()).isTrue();
+
+    await().untilAsserted(() -> {
+      assertThat(locatorLogFile)
+          .doesNotExist();
+
+      assertThat(pulseLogFile)
+          .doesNotExist();
+
+      LogFileAssert.assertThat(stdoutFile.toFile())
+          .exists()
+          .contains("Located war: geode-pulse")
+          .contains("Adding webapp /pulse")
+          .contains("Starting server location for Distribution Locator")
+          .doesNotContain("geode-pulse war file was not found")
+          .doesNotContain("java.lang.IllegalStateException: No factory method found for class");
+    });
+  }
+
+  @Test
+  @Ignore("GEODE-8197")

Review comment:
       Yes. This PR is only creating new tests. I'll follow up in the future with a fix and reenable the two new tests that are currently disabled with "GEODE-8197". Right now these two tests will fail.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [geode] onichols-pivotal commented on pull request #5196: GEODE-8197: Add launcher acceptance testing for custom logging config

Posted by GitBox <gi...@apache.org>.
onichols-pivotal commented on pull request #5196:
URL: https://github.com/apache/geode/pull/5196#issuecomment-641616445


   > @onichols-pivotal Yay, JAVA_HOME/bin/java worked! AcceptanceTests all passed.
   
   Does this also pass locally for you?  For developers that do not have JAVA_HOME set but do have `java` on their system path, is it reasonable to require them to set JAVA_HOME in order to run these tests locally?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [geode] kirklund commented on a change in pull request #5196: GEODE-8197: Add launcher acceptance testing for custom logging config

Posted by GitBox <gi...@apache.org>.
kirklund commented on a change in pull request #5196:
URL: https://github.com/apache/geode/pull/5196#discussion_r436051212



##########
File path: geode-assembly/src/acceptanceTest/java/org/apache/geode/launchers/LocatorLauncherWithPulseAcceptanceTest.java
##########
@@ -0,0 +1,113 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.launchers;
+
+import static org.apache.geode.test.awaitility.GeodeAwaitility.await;
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.nio.file.Path;
+import java.util.concurrent.TimeUnit;
+
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+
+import org.apache.geode.internal.AvailablePortHelper;
+import org.apache.geode.test.assertj.LogFileAssert;
+import org.apache.geode.test.junit.rules.RequiresGeodeHome;
+
+public class LocatorLauncherWithPulseAcceptanceTest {
+
+  private static final String LOCATOR_NAME = "the-locator";
+
+  private int locatorPort;
+  private int httpServicePort;
+  private Process locator;
+  private Path geodeDependencies;
+  private Path stdoutFile;
+  private Path locatorLogFile;
+  private Path pulseLogFile;
+
+  @Rule
+  public RequiresGeodeHome requiresGeodeHome = new RequiresGeodeHome();
+  @Rule
+  public TemporaryFolder temporaryFolder = new TemporaryFolder();
+
+  @Before
+  public void setUpGeodeDependencies() {
+    Path geodeHome = requiresGeodeHome.getGeodeHome().toPath();
+    geodeDependencies = geodeHome.resolve("lib/geode-dependencies.jar");
+
+    assertThat(geodeDependencies).exists();
+  }
+
+  @Before
+  public void setUpOutputFiles() {
+    stdoutFile = temporaryFolder.getRoot().toPath().resolve("stdout.txt");
+    locatorLogFile = temporaryFolder.getRoot().toPath().resolve(LOCATOR_NAME + ".log");
+    pulseLogFile = temporaryFolder.getRoot().toPath().resolve("pulse.log");
+  }
+
+  @Before
+  public void setUpRandomPorts() {
+    int[] ports = AvailablePortHelper.getRandomAvailableTCPPorts(2);
+    locatorPort = ports[0];
+    httpServicePort = ports[1];
+  }
+
+  @After
+  public void stopLocator() throws Exception {
+    if (locator != null) {
+      locator.destroyForcibly().waitFor(4, TimeUnit.SECONDS);
+    }
+  }
+
+  @Test
+  public void locatorLauncherStartsPulse() throws Exception {
+    ProcessBuilder processBuilder = new ProcessBuilder()
+        .redirectErrorStream(true)
+        .redirectOutput(stdoutFile.toFile())
+        .directory(temporaryFolder.getRoot())
+        .command("java",
+            "-Dgemfire.http-service-port=" + httpServicePort,
+            "-Dgemfire.jmx-manager-start=true",
+            "-Djava.awt.headless=true",
+            "-cp", geodeDependencies.toFile().getAbsolutePath(),
+            "org.apache.geode.distributed.LocatorLauncher", "start", LOCATOR_NAME,
+            "--port", String.valueOf(locatorPort));
+
+    System.out.println("Launching command: " + processBuilder.command());
+
+    locator = processBuilder
+        .start();
+
+    assertThat(locator.isAlive()).isTrue();
+
+    await().untilAsserted(() -> {
+      LogFileAssert.assertThat(locatorLogFile.toFile())
+          .exists()

Review comment:
       LocatorLauncherWithPulseAcceptanceTest is the only test in this PR that doesn't provide a custom log4j2 xml file. So that means, it'll create a locator log file.
   
   The others tests all use a custom log4j2 xml file which turns off logging to a file and turns on logging to stdout. These tests assert that the locator log file doesn't exist to help verify that the custom log4j2 xml file for that test was actually used.
   
   The tests that use a custom log file include "WithCustomLogConfig" somewhere in the name of the test class, whereas LocatorLauncherWithPulseAcceptanceTest does not.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [geode] onichols-pivotal edited a comment on pull request #5196: GEODE-8197: Add launcher acceptance testing for custom logging config

Posted by GitBox <gi...@apache.org>.
onichols-pivotal edited a comment on pull request #5196:
URL: https://github.com/apache/geode/pull/5196#issuecomment-641616445


   > @onichols-pivotal Yay, JAVA_HOME/bin/java worked! AcceptanceTests all passed.
   
   Some developers may not have JAVA_HOME set but do have `java` on their system path.  You could implement a fallback to just `java` but then you would lose a good assert.  So it seems reasonable enough to require them to set JAVA_HOME in order to run these tests locally.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [geode] jchen21 commented on a change in pull request #5196: GEODE-8197: Add launcher acceptance testing for custom logging config

Posted by GitBox <gi...@apache.org>.
jchen21 commented on a change in pull request #5196:
URL: https://github.com/apache/geode/pull/5196#discussion_r435499656



##########
File path: geode-assembly/src/acceptanceTest/java/org/apache/geode/launchers/LocatorLauncherWithPulseAcceptanceTest.java
##########
@@ -0,0 +1,113 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.launchers;
+
+import static org.apache.geode.test.awaitility.GeodeAwaitility.await;
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.nio.file.Path;
+import java.util.concurrent.TimeUnit;
+
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+
+import org.apache.geode.internal.AvailablePortHelper;
+import org.apache.geode.test.assertj.LogFileAssert;
+import org.apache.geode.test.junit.rules.RequiresGeodeHome;
+
+public class LocatorLauncherWithPulseAcceptanceTest {
+
+  private static final String LOCATOR_NAME = "the-locator";
+
+  private int locatorPort;
+  private int httpServicePort;
+  private Process locator;
+  private Path geodeDependencies;
+  private Path stdoutFile;
+  private Path locatorLogFile;
+  private Path pulseLogFile;
+
+  @Rule
+  public RequiresGeodeHome requiresGeodeHome = new RequiresGeodeHome();
+  @Rule
+  public TemporaryFolder temporaryFolder = new TemporaryFolder();
+
+  @Before
+  public void setUpGeodeDependencies() {
+    Path geodeHome = requiresGeodeHome.getGeodeHome().toPath();
+    geodeDependencies = geodeHome.resolve("lib/geode-dependencies.jar");
+
+    assertThat(geodeDependencies).exists();
+  }
+
+  @Before
+  public void setUpOutputFiles() {
+    stdoutFile = temporaryFolder.getRoot().toPath().resolve("stdout.txt");
+    locatorLogFile = temporaryFolder.getRoot().toPath().resolve(LOCATOR_NAME + ".log");
+    pulseLogFile = temporaryFolder.getRoot().toPath().resolve("pulse.log");
+  }
+
+  @Before
+  public void setUpRandomPorts() {
+    int[] ports = AvailablePortHelper.getRandomAvailableTCPPorts(2);
+    locatorPort = ports[0];
+    httpServicePort = ports[1];
+  }
+
+  @After
+  public void stopLocator() throws Exception {
+    if (locator != null) {
+      locator.destroyForcibly().waitFor(4, TimeUnit.SECONDS);
+    }
+  }
+
+  @Test
+  public void locatorLauncherStartsPulse() throws Exception {
+    ProcessBuilder processBuilder = new ProcessBuilder()
+        .redirectErrorStream(true)
+        .redirectOutput(stdoutFile.toFile())
+        .directory(temporaryFolder.getRoot())
+        .command("java",
+            "-Dgemfire.http-service-port=" + httpServicePort,
+            "-Dgemfire.jmx-manager-start=true",
+            "-Djava.awt.headless=true",
+            "-cp", geodeDependencies.toFile().getAbsolutePath(),
+            "org.apache.geode.distributed.LocatorLauncher", "start", LOCATOR_NAME,
+            "--port", String.valueOf(locatorPort));
+
+    System.out.println("Launching command: " + processBuilder.command());
+
+    locator = processBuilder
+        .start();
+
+    assertThat(locator.isAlive()).isTrue();
+
+    await().untilAsserted(() -> {
+      LogFileAssert.assertThat(locatorLogFile.toFile())
+          .exists()

Review comment:
       Why `locatorLogFile` exits for this test, while ``locatorLogFile` doesn't exist for the other tests in this pull request? Can you share what configuration makes this difference?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [geode] kirklund commented on a change in pull request #5196: GEODE-8197: Add launcher acceptance testing for custom logging config

Posted by GitBox <gi...@apache.org>.
kirklund commented on a change in pull request #5196:
URL: https://github.com/apache/geode/pull/5196#discussion_r436051212



##########
File path: geode-assembly/src/acceptanceTest/java/org/apache/geode/launchers/LocatorLauncherWithPulseAcceptanceTest.java
##########
@@ -0,0 +1,113 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.launchers;
+
+import static org.apache.geode.test.awaitility.GeodeAwaitility.await;
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.nio.file.Path;
+import java.util.concurrent.TimeUnit;
+
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+
+import org.apache.geode.internal.AvailablePortHelper;
+import org.apache.geode.test.assertj.LogFileAssert;
+import org.apache.geode.test.junit.rules.RequiresGeodeHome;
+
+public class LocatorLauncherWithPulseAcceptanceTest {
+
+  private static final String LOCATOR_NAME = "the-locator";
+
+  private int locatorPort;
+  private int httpServicePort;
+  private Process locator;
+  private Path geodeDependencies;
+  private Path stdoutFile;
+  private Path locatorLogFile;
+  private Path pulseLogFile;
+
+  @Rule
+  public RequiresGeodeHome requiresGeodeHome = new RequiresGeodeHome();
+  @Rule
+  public TemporaryFolder temporaryFolder = new TemporaryFolder();
+
+  @Before
+  public void setUpGeodeDependencies() {
+    Path geodeHome = requiresGeodeHome.getGeodeHome().toPath();
+    geodeDependencies = geodeHome.resolve("lib/geode-dependencies.jar");
+
+    assertThat(geodeDependencies).exists();
+  }
+
+  @Before
+  public void setUpOutputFiles() {
+    stdoutFile = temporaryFolder.getRoot().toPath().resolve("stdout.txt");
+    locatorLogFile = temporaryFolder.getRoot().toPath().resolve(LOCATOR_NAME + ".log");
+    pulseLogFile = temporaryFolder.getRoot().toPath().resolve("pulse.log");
+  }
+
+  @Before
+  public void setUpRandomPorts() {
+    int[] ports = AvailablePortHelper.getRandomAvailableTCPPorts(2);
+    locatorPort = ports[0];
+    httpServicePort = ports[1];
+  }
+
+  @After
+  public void stopLocator() throws Exception {
+    if (locator != null) {
+      locator.destroyForcibly().waitFor(4, TimeUnit.SECONDS);
+    }
+  }
+
+  @Test
+  public void locatorLauncherStartsPulse() throws Exception {
+    ProcessBuilder processBuilder = new ProcessBuilder()
+        .redirectErrorStream(true)
+        .redirectOutput(stdoutFile.toFile())
+        .directory(temporaryFolder.getRoot())
+        .command("java",
+            "-Dgemfire.http-service-port=" + httpServicePort,
+            "-Dgemfire.jmx-manager-start=true",
+            "-Djava.awt.headless=true",
+            "-cp", geodeDependencies.toFile().getAbsolutePath(),
+            "org.apache.geode.distributed.LocatorLauncher", "start", LOCATOR_NAME,
+            "--port", String.valueOf(locatorPort));
+
+    System.out.println("Launching command: " + processBuilder.command());
+
+    locator = processBuilder
+        .start();
+
+    assertThat(locator.isAlive()).isTrue();
+
+    await().untilAsserted(() -> {
+      LogFileAssert.assertThat(locatorLogFile.toFile())
+          .exists()

Review comment:
       LocatorLauncherWithPulseAcceptanceTest is the only test in this PR that doesn't provide a custom log4j2 xml file. So that means, it'll create a locator log file.
   
   The others tests all use a custom log4j2 xml file which turns off logging to a file and turns on logging to stdout. These tests assert that the locator log file doesn't exist to help verify that the custom log4j2 xml file for that test was actually used.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [geode] kirklund commented on a change in pull request #5196: GEODE-8197: Add launcher acceptance testing for custom logging config

Posted by GitBox <gi...@apache.org>.
kirklund commented on a change in pull request #5196:
URL: https://github.com/apache/geode/pull/5196#discussion_r436051212



##########
File path: geode-assembly/src/acceptanceTest/java/org/apache/geode/launchers/LocatorLauncherWithPulseAcceptanceTest.java
##########
@@ -0,0 +1,113 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.launchers;
+
+import static org.apache.geode.test.awaitility.GeodeAwaitility.await;
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.nio.file.Path;
+import java.util.concurrent.TimeUnit;
+
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+
+import org.apache.geode.internal.AvailablePortHelper;
+import org.apache.geode.test.assertj.LogFileAssert;
+import org.apache.geode.test.junit.rules.RequiresGeodeHome;
+
+public class LocatorLauncherWithPulseAcceptanceTest {
+
+  private static final String LOCATOR_NAME = "the-locator";
+
+  private int locatorPort;
+  private int httpServicePort;
+  private Process locator;
+  private Path geodeDependencies;
+  private Path stdoutFile;
+  private Path locatorLogFile;
+  private Path pulseLogFile;
+
+  @Rule
+  public RequiresGeodeHome requiresGeodeHome = new RequiresGeodeHome();
+  @Rule
+  public TemporaryFolder temporaryFolder = new TemporaryFolder();
+
+  @Before
+  public void setUpGeodeDependencies() {
+    Path geodeHome = requiresGeodeHome.getGeodeHome().toPath();
+    geodeDependencies = geodeHome.resolve("lib/geode-dependencies.jar");
+
+    assertThat(geodeDependencies).exists();
+  }
+
+  @Before
+  public void setUpOutputFiles() {
+    stdoutFile = temporaryFolder.getRoot().toPath().resolve("stdout.txt");
+    locatorLogFile = temporaryFolder.getRoot().toPath().resolve(LOCATOR_NAME + ".log");
+    pulseLogFile = temporaryFolder.getRoot().toPath().resolve("pulse.log");
+  }
+
+  @Before
+  public void setUpRandomPorts() {
+    int[] ports = AvailablePortHelper.getRandomAvailableTCPPorts(2);
+    locatorPort = ports[0];
+    httpServicePort = ports[1];
+  }
+
+  @After
+  public void stopLocator() throws Exception {
+    if (locator != null) {
+      locator.destroyForcibly().waitFor(4, TimeUnit.SECONDS);
+    }
+  }
+
+  @Test
+  public void locatorLauncherStartsPulse() throws Exception {
+    ProcessBuilder processBuilder = new ProcessBuilder()
+        .redirectErrorStream(true)
+        .redirectOutput(stdoutFile.toFile())
+        .directory(temporaryFolder.getRoot())
+        .command("java",
+            "-Dgemfire.http-service-port=" + httpServicePort,
+            "-Dgemfire.jmx-manager-start=true",
+            "-Djava.awt.headless=true",
+            "-cp", geodeDependencies.toFile().getAbsolutePath(),
+            "org.apache.geode.distributed.LocatorLauncher", "start", LOCATOR_NAME,
+            "--port", String.valueOf(locatorPort));
+
+    System.out.println("Launching command: " + processBuilder.command());
+
+    locator = processBuilder
+        .start();
+
+    assertThat(locator.isAlive()).isTrue();
+
+    await().untilAsserted(() -> {
+      LogFileAssert.assertThat(locatorLogFile.toFile())
+          .exists()

Review comment:
       LocatorLauncherWithPulseAcceptanceTest is the only test in this PR that doesn't provide a custom log4j2 xml file. So that means, it'll create a locator log file.
   
   The other tests all use a custom log4j2 xml file which turns off logging to a file and turns on logging to stdout. These tests assert that the locator log file doesn't exist to help verify that the custom log4j2 xml file for that test was actually used.
   
   The tests that use a custom log file include "WithCustomLogConfig" somewhere in the name of the test class, whereas LocatorLauncherWithPulseAcceptanceTest does not.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [geode] onichols-pivotal commented on a change in pull request #5196: GEODE-8197: Add launcher acceptance testing for custom logging config

Posted by GitBox <gi...@apache.org>.
onichols-pivotal commented on a change in pull request #5196:
URL: https://github.com/apache/geode/pull/5196#discussion_r436309760



##########
File path: geode-assembly/src/acceptanceTest/java/org/apache/geode/launchers/LocatorLauncherWithCustomLogConfigAcceptanceTest.java
##########
@@ -0,0 +1,175 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.launchers;
+
+import static org.apache.geode.test.awaitility.GeodeAwaitility.await;
+import static org.apache.geode.test.util.ResourceUtils.createFileFromResource;
+import static org.apache.geode.test.util.ResourceUtils.getResource;
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.io.File;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.concurrent.TimeUnit;
+
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+
+import org.apache.geode.internal.AvailablePortHelper;
+import org.apache.geode.test.assertj.LogFileAssert;
+import org.apache.geode.test.junit.rules.RequiresGeodeHome;
+
+public class LocatorLauncherWithCustomLogConfigAcceptanceTest {
+
+  private static final String CONFIG_FILE_NAME =
+      "LocatorLauncherWithCustomLogConfigAcceptanceTest.xml";
+  private static final String LOCATOR_NAME = "the-locator";
+
+  private int locatorPort;
+  private Path configFile;
+  private Process locator;
+  private Path geodeDependencies;
+  private Path stdoutFile;
+  private Path locatorLogFile;
+  private Path pulseLogFile;
+
+  @Rule
+  public RequiresGeodeHome requiresGeodeHome = new RequiresGeodeHome();
+  @Rule
+  public TemporaryFolder temporaryFolder = new TemporaryFolder();
+
+  @Before
+  public void setUpLogConfigFile() {
+    configFile = createFileFromResource(getResource(CONFIG_FILE_NAME), temporaryFolder.getRoot(),
+        CONFIG_FILE_NAME)
+            .toPath();
+  }
+
+  @Before
+  public void setUpGeodeDependencies() {
+    Path geodeHome = requiresGeodeHome.getGeodeHome().toPath();
+    geodeDependencies = geodeHome.resolve("lib/geode-dependencies.jar");
+
+    assertThat(geodeDependencies).exists();
+  }
+
+  @Before
+  public void setUpOutputFiles() {
+    stdoutFile = temporaryFolder.getRoot().toPath().resolve("stdout.txt");
+    locatorLogFile = temporaryFolder.getRoot().toPath().resolve(LOCATOR_NAME + ".log");
+    pulseLogFile = temporaryFolder.getRoot().toPath().resolve("pulse.log");
+  }
+
+  @Before
+  public void setUpRandomPorts() {
+    locatorPort = AvailablePortHelper.getRandomAvailableTCPPort();
+  }
+
+  @After
+  public void stopLocator() throws Exception {
+    if (locator != null) {
+      locator.destroyForcibly().waitFor(4, TimeUnit.SECONDS);
+    }
+  }
+
+  @Test
+  public void locatorLauncherUsesLog4jConfigurationFile() throws Exception {
+    ProcessBuilder processBuilder = new ProcessBuilder()
+        .redirectErrorStream(true)
+        .redirectOutput(stdoutFile.toFile())
+        .directory(temporaryFolder.getRoot())
+        .command("java",

Review comment:
       If system property java.home is defined, please use `System.getProperty("java.home")+"/bin/java"` instead.  Only use `java` if system property java.home is not defined




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [geode] kirklund commented on pull request #5196: GEODE-8197: Add launcher acceptance testing for custom logging config

Posted by GitBox <gi...@apache.org>.
kirklund commented on pull request #5196:
URL: https://github.com/apache/geode/pull/5196#issuecomment-637704818


   @demery-pivotal Yep, I minimized the number of System.outs but I left them in on purpose to print out the command being executed. Two tests are currently disabled due to bug GEODE-8197 and will fail until it's fixed.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [geode] kirklund merged pull request #5196: GEODE-8197: Add launcher acceptance testing for custom logging config

Posted by GitBox <gi...@apache.org>.
kirklund merged pull request #5196:
URL: https://github.com/apache/geode/pull/5196


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [geode] kirklund commented on a change in pull request #5196: GEODE-8197: Add launcher acceptance testing for custom logging config

Posted by GitBox <gi...@apache.org>.
kirklund commented on a change in pull request #5196:
URL: https://github.com/apache/geode/pull/5196#discussion_r436186564



##########
File path: geode-assembly/src/acceptanceTest/java/org/apache/geode/launchers/LocatorLauncherWithPulseAndCustomLogConfigAcceptanceTest.java
##########
@@ -0,0 +1,268 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.launchers;
+
+import static java.nio.file.Files.copy;
+import static org.apache.geode.test.awaitility.GeodeAwaitility.await;
+import static org.apache.geode.test.util.ResourceUtils.createFileFromResource;
+import static org.apache.geode.test.util.ResourceUtils.getResource;
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.io.File;
+import java.nio.file.Path;
+import java.util.concurrent.TimeUnit;
+
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Ignore;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+
+import org.apache.geode.internal.AvailablePortHelper;
+import org.apache.geode.test.assertj.LogFileAssert;
+import org.apache.geode.test.junit.rules.RequiresGeodeHome;
+
+public class LocatorLauncherWithPulseAndCustomLogConfigAcceptanceTest {
+
+  private static final String CONFIG_WITH_GEODE_PLUGINS_FILE_NAME =
+      "LocatorLauncherWithPulseAndCustomLogConfigAcceptanceTestWithGeodePlugins.xml";
+  private static final String CONFIG_WITHOUT_GEODE_PLUGINS_FILE_NAME =
+      "LocatorLauncherWithPulseAndCustomLogConfigAcceptanceTestWithoutGeodePlugins.xml";
+  private static final String LOCATOR_NAME = "the-locator";
+
+  private int locatorPort;
+  private int httpServicePort;
+  private Path configWithGeodePluginsFile;
+  private Path configWithoutGeodePluginsFile;
+  private Process locator;
+  private Path geodeDependencies;
+  private Path stdoutFile;
+  private Path locatorLogFile;
+  private Path pulseLogFile;
+
+  @Rule
+  public RequiresGeodeHome requiresGeodeHome = new RequiresGeodeHome();
+  @Rule
+  public TemporaryFolder temporaryFolder = new TemporaryFolder();
+
+  @Before
+  public void setUpLogConfigFiles() {
+    configWithGeodePluginsFile = createFileFromResource(
+        getResource(CONFIG_WITH_GEODE_PLUGINS_FILE_NAME), temporaryFolder.getRoot(),
+        CONFIG_WITH_GEODE_PLUGINS_FILE_NAME)
+            .toPath();
+
+    configWithoutGeodePluginsFile = createFileFromResource(
+        getResource(CONFIG_WITHOUT_GEODE_PLUGINS_FILE_NAME), temporaryFolder.getRoot(),
+        CONFIG_WITHOUT_GEODE_PLUGINS_FILE_NAME)
+            .toPath();
+  }
+
+  @Before
+  public void setUpGeodeDependencies() {
+    Path geodeHome = requiresGeodeHome.getGeodeHome().toPath();
+    geodeDependencies = geodeHome.resolve("lib/geode-dependencies.jar");
+
+    assertThat(geodeDependencies).exists();
+  }
+
+  @Before
+  public void setUpOutputFiles() {
+    stdoutFile = temporaryFolder.getRoot().toPath().resolve("stdout.txt");
+    locatorLogFile = temporaryFolder.getRoot().toPath().resolve(LOCATOR_NAME + ".log");
+    pulseLogFile = temporaryFolder.getRoot().toPath().resolve("pulse.log");
+  }
+
+  @Before
+  public void setUpRandomPorts() {
+    int[] ports = AvailablePortHelper.getRandomAvailableTCPPorts(2);
+    locatorPort = ports[0];
+    httpServicePort = ports[1];
+  }
+
+  @After
+  public void stopLocator() throws Exception {
+    if (locator != null) {
+      locator.destroyForcibly().waitFor(4, TimeUnit.SECONDS);
+    }
+  }
+
+  @Test
+  public void locatorLauncherUsesSpecifiedConfigFileWithoutGeodePlugins() throws Exception {
+    ProcessBuilder processBuilder = new ProcessBuilder()
+        .redirectErrorStream(true)
+        .redirectOutput(stdoutFile.toFile())
+        .directory(temporaryFolder.getRoot())
+        .command("java",
+            "-Dgemfire.http-service-port=" + httpServicePort,
+            "-Dgemfire.jmx-manager-start=true",
+            "-Djava.awt.headless=true",
+            "-Dlog4j.configurationFile=" + configWithoutGeodePluginsFile.toAbsolutePath(),
+            "-cp", geodeDependencies.toFile().getAbsolutePath(),
+            "org.apache.geode.distributed.LocatorLauncher", "start", LOCATOR_NAME,
+            "--port", String.valueOf(locatorPort));
+
+    System.out.println("Launching command: " + processBuilder.command());
+
+    locator = processBuilder
+        .start();
+
+    assertThat(locator.isAlive()).isTrue();
+
+    await().untilAsserted(() -> {
+      assertThat(locatorLogFile)
+          .doesNotExist();
+
+      assertThat(pulseLogFile)
+          .doesNotExist();
+
+      LogFileAssert.assertThat(stdoutFile.toFile())
+          .exists()
+          .contains("Located war: geode-pulse")
+          .contains("Adding webapp /pulse")
+          .contains("Starting server location for Distribution Locator")
+          .doesNotContain("geode-pulse war file was not found")
+          .doesNotContain("java.lang.IllegalStateException: No factory method found for class");
+    });
+  }
+
+  @Test
+  public void locatorLauncherUsesConfigFileInClasspathWithoutGeodePlugins() throws Exception {
+    copy(configWithoutGeodePluginsFile, temporaryFolder.getRoot().toPath().resolve("log4j2.xml"));
+
+    String classpath = temporaryFolder.getRoot().getAbsolutePath() + File.pathSeparator +
+        geodeDependencies.toFile().getAbsolutePath();
+
+    ProcessBuilder processBuilder = new ProcessBuilder()
+        .redirectErrorStream(true)
+        .redirectOutput(stdoutFile.toFile())
+        .directory(temporaryFolder.getRoot())
+        .command("java",
+            "-Dgemfire.http-service-port=" + httpServicePort,
+            "-Dgemfire.jmx-manager-start=true",
+            "-Djava.awt.headless=true",
+            "-cp", classpath,
+            "org.apache.geode.distributed.LocatorLauncher", "start", LOCATOR_NAME,
+            "--port", String.valueOf(locatorPort));
+
+    System.out.println("Launching command: " + processBuilder.command());
+
+    locator = processBuilder
+        .start();
+
+    assertThat(locator.isAlive()).isTrue();
+
+    await().untilAsserted(() -> {
+      assertThat(locatorLogFile)
+          .doesNotExist();
+
+      assertThat(pulseLogFile)
+          .doesNotExist();
+
+      LogFileAssert.assertThat(stdoutFile.toFile())
+          .exists()
+          .contains("Located war: geode-pulse")
+          .contains("Adding webapp /pulse")
+          .contains("Starting server location for Distribution Locator")
+          .doesNotContain("geode-pulse war file was not found")
+          .doesNotContain("java.lang.IllegalStateException: No factory method found for class");
+    });
+  }
+
+  @Test
+  @Ignore("GEODE-8197")
+  public void locatorLauncherUsesSpecifiedConfigFileWithGeodePlugins() throws Exception {
+    ProcessBuilder processBuilder = new ProcessBuilder()
+        .redirectErrorStream(true)
+        .redirectOutput(stdoutFile.toFile())
+        .directory(temporaryFolder.getRoot())
+        .command("java",
+            "-Dgemfire.http-service-port=" + httpServicePort,
+            "-Dgemfire.jmx-manager-start=true",
+            "-Djava.awt.headless=true",
+            "-Dlog4j.configurationFile=" + configWithGeodePluginsFile.toAbsolutePath(),
+            "-cp", geodeDependencies.toFile().getAbsolutePath(),
+            "org.apache.geode.distributed.LocatorLauncher", "start", LOCATOR_NAME,
+            "--port", String.valueOf(locatorPort));
+
+    System.out.println("Launching command: " + processBuilder.command());
+
+    locator = processBuilder
+        .start();
+
+    assertThat(locator.isAlive()).isTrue();
+
+    await().untilAsserted(() -> {
+      assertThat(locatorLogFile)
+          .doesNotExist();
+
+      assertThat(pulseLogFile)
+          .doesNotExist();
+
+      LogFileAssert.assertThat(stdoutFile.toFile())
+          .exists()
+          .contains("Located war: geode-pulse")
+          .contains("Adding webapp /pulse")
+          .contains("Starting server location for Distribution Locator")
+          .doesNotContain("geode-pulse war file was not found")
+          .doesNotContain("java.lang.IllegalStateException: No factory method found for class");
+    });
+  }
+
+  @Test
+  @Ignore("GEODE-8197")

Review comment:
       Yes. This PR is only creating new tests. I'll follow up in the future with a fix and reenable the two new tests that are currently disabled with "GEODE-8197".




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [geode] kirklund commented on pull request #5196: GEODE-8197: Add launcher acceptance testing for custom logging config

Posted by GitBox <gi...@apache.org>.
kirklund commented on pull request #5196:
URL: https://github.com/apache/geode/pull/5196#issuecomment-639632632


   Looks like Docker blew up in AcceptanceTest. Retriggering...


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [geode] kirklund commented on pull request #5196: GEODE-8197: Add launcher acceptance testing for custom logging config

Posted by GitBox <gi...@apache.org>.
kirklund commented on pull request #5196:
URL: https://github.com/apache/geode/pull/5196#issuecomment-641615584


   @onichols-pivotal Yay, JAVA_HOME/bin/java worked! AcceptanceTests all passed.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [geode] jchen21 commented on a change in pull request #5196: GEODE-8197: Add launcher acceptance testing for custom logging config

Posted by GitBox <gi...@apache.org>.
jchen21 commented on a change in pull request #5196:
URL: https://github.com/apache/geode/pull/5196#discussion_r436080693



##########
File path: geode-assembly/src/acceptanceTest/java/org/apache/geode/launchers/LocatorLauncherWithPulseAndCustomLogConfigAcceptanceTest.java
##########
@@ -0,0 +1,268 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.launchers;
+
+import static java.nio.file.Files.copy;
+import static org.apache.geode.test.awaitility.GeodeAwaitility.await;
+import static org.apache.geode.test.util.ResourceUtils.createFileFromResource;
+import static org.apache.geode.test.util.ResourceUtils.getResource;
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.io.File;
+import java.nio.file.Path;
+import java.util.concurrent.TimeUnit;
+
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Ignore;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+
+import org.apache.geode.internal.AvailablePortHelper;
+import org.apache.geode.test.assertj.LogFileAssert;
+import org.apache.geode.test.junit.rules.RequiresGeodeHome;
+
+public class LocatorLauncherWithPulseAndCustomLogConfigAcceptanceTest {
+
+  private static final String CONFIG_WITH_GEODE_PLUGINS_FILE_NAME =
+      "LocatorLauncherWithPulseAndCustomLogConfigAcceptanceTestWithGeodePlugins.xml";
+  private static final String CONFIG_WITHOUT_GEODE_PLUGINS_FILE_NAME =
+      "LocatorLauncherWithPulseAndCustomLogConfigAcceptanceTestWithoutGeodePlugins.xml";
+  private static final String LOCATOR_NAME = "the-locator";
+
+  private int locatorPort;
+  private int httpServicePort;
+  private Path configWithGeodePluginsFile;
+  private Path configWithoutGeodePluginsFile;
+  private Process locator;
+  private Path geodeDependencies;
+  private Path stdoutFile;
+  private Path locatorLogFile;
+  private Path pulseLogFile;
+
+  @Rule
+  public RequiresGeodeHome requiresGeodeHome = new RequiresGeodeHome();
+  @Rule
+  public TemporaryFolder temporaryFolder = new TemporaryFolder();
+
+  @Before
+  public void setUpLogConfigFiles() {
+    configWithGeodePluginsFile = createFileFromResource(
+        getResource(CONFIG_WITH_GEODE_PLUGINS_FILE_NAME), temporaryFolder.getRoot(),
+        CONFIG_WITH_GEODE_PLUGINS_FILE_NAME)
+            .toPath();
+
+    configWithoutGeodePluginsFile = createFileFromResource(
+        getResource(CONFIG_WITHOUT_GEODE_PLUGINS_FILE_NAME), temporaryFolder.getRoot(),
+        CONFIG_WITHOUT_GEODE_PLUGINS_FILE_NAME)
+            .toPath();
+  }
+
+  @Before
+  public void setUpGeodeDependencies() {
+    Path geodeHome = requiresGeodeHome.getGeodeHome().toPath();
+    geodeDependencies = geodeHome.resolve("lib/geode-dependencies.jar");
+
+    assertThat(geodeDependencies).exists();
+  }
+
+  @Before
+  public void setUpOutputFiles() {
+    stdoutFile = temporaryFolder.getRoot().toPath().resolve("stdout.txt");
+    locatorLogFile = temporaryFolder.getRoot().toPath().resolve(LOCATOR_NAME + ".log");
+    pulseLogFile = temporaryFolder.getRoot().toPath().resolve("pulse.log");
+  }
+
+  @Before
+  public void setUpRandomPorts() {
+    int[] ports = AvailablePortHelper.getRandomAvailableTCPPorts(2);
+    locatorPort = ports[0];
+    httpServicePort = ports[1];
+  }
+
+  @After
+  public void stopLocator() throws Exception {
+    if (locator != null) {
+      locator.destroyForcibly().waitFor(4, TimeUnit.SECONDS);
+    }
+  }
+
+  @Test
+  public void locatorLauncherUsesSpecifiedConfigFileWithoutGeodePlugins() throws Exception {
+    ProcessBuilder processBuilder = new ProcessBuilder()
+        .redirectErrorStream(true)
+        .redirectOutput(stdoutFile.toFile())
+        .directory(temporaryFolder.getRoot())
+        .command("java",
+            "-Dgemfire.http-service-port=" + httpServicePort,
+            "-Dgemfire.jmx-manager-start=true",
+            "-Djava.awt.headless=true",
+            "-Dlog4j.configurationFile=" + configWithoutGeodePluginsFile.toAbsolutePath(),
+            "-cp", geodeDependencies.toFile().getAbsolutePath(),
+            "org.apache.geode.distributed.LocatorLauncher", "start", LOCATOR_NAME,
+            "--port", String.valueOf(locatorPort));
+
+    System.out.println("Launching command: " + processBuilder.command());
+
+    locator = processBuilder
+        .start();
+
+    assertThat(locator.isAlive()).isTrue();
+
+    await().untilAsserted(() -> {
+      assertThat(locatorLogFile)
+          .doesNotExist();
+
+      assertThat(pulseLogFile)
+          .doesNotExist();
+
+      LogFileAssert.assertThat(stdoutFile.toFile())
+          .exists()
+          .contains("Located war: geode-pulse")
+          .contains("Adding webapp /pulse")
+          .contains("Starting server location for Distribution Locator")
+          .doesNotContain("geode-pulse war file was not found")
+          .doesNotContain("java.lang.IllegalStateException: No factory method found for class");
+    });
+  }
+
+  @Test
+  public void locatorLauncherUsesConfigFileInClasspathWithoutGeodePlugins() throws Exception {
+    copy(configWithoutGeodePluginsFile, temporaryFolder.getRoot().toPath().resolve("log4j2.xml"));
+
+    String classpath = temporaryFolder.getRoot().getAbsolutePath() + File.pathSeparator +
+        geodeDependencies.toFile().getAbsolutePath();
+
+    ProcessBuilder processBuilder = new ProcessBuilder()
+        .redirectErrorStream(true)
+        .redirectOutput(stdoutFile.toFile())
+        .directory(temporaryFolder.getRoot())
+        .command("java",
+            "-Dgemfire.http-service-port=" + httpServicePort,
+            "-Dgemfire.jmx-manager-start=true",
+            "-Djava.awt.headless=true",
+            "-cp", classpath,
+            "org.apache.geode.distributed.LocatorLauncher", "start", LOCATOR_NAME,
+            "--port", String.valueOf(locatorPort));
+
+    System.out.println("Launching command: " + processBuilder.command());
+
+    locator = processBuilder
+        .start();
+
+    assertThat(locator.isAlive()).isTrue();
+
+    await().untilAsserted(() -> {
+      assertThat(locatorLogFile)
+          .doesNotExist();
+
+      assertThat(pulseLogFile)
+          .doesNotExist();
+
+      LogFileAssert.assertThat(stdoutFile.toFile())
+          .exists()
+          .contains("Located war: geode-pulse")
+          .contains("Adding webapp /pulse")
+          .contains("Starting server location for Distribution Locator")
+          .doesNotContain("geode-pulse war file was not found")
+          .doesNotContain("java.lang.IllegalStateException: No factory method found for class");
+    });
+  }
+
+  @Test
+  @Ignore("GEODE-8197")

Review comment:
       My understanding is that once GEODE-8197 related bug is fixed, this `@Ignore` annotation will be removed.

##########
File path: geode-assembly/src/acceptanceTest/java/org/apache/geode/launchers/LocatorLauncherWithPulseAndCustomLogConfigAcceptanceTest.java
##########
@@ -0,0 +1,268 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.launchers;
+
+import static java.nio.file.Files.copy;
+import static org.apache.geode.test.awaitility.GeodeAwaitility.await;
+import static org.apache.geode.test.util.ResourceUtils.createFileFromResource;
+import static org.apache.geode.test.util.ResourceUtils.getResource;
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.io.File;
+import java.nio.file.Path;
+import java.util.concurrent.TimeUnit;
+
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Ignore;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+
+import org.apache.geode.internal.AvailablePortHelper;
+import org.apache.geode.test.assertj.LogFileAssert;
+import org.apache.geode.test.junit.rules.RequiresGeodeHome;
+
+public class LocatorLauncherWithPulseAndCustomLogConfigAcceptanceTest {
+
+  private static final String CONFIG_WITH_GEODE_PLUGINS_FILE_NAME =
+      "LocatorLauncherWithPulseAndCustomLogConfigAcceptanceTestWithGeodePlugins.xml";
+  private static final String CONFIG_WITHOUT_GEODE_PLUGINS_FILE_NAME =
+      "LocatorLauncherWithPulseAndCustomLogConfigAcceptanceTestWithoutGeodePlugins.xml";
+  private static final String LOCATOR_NAME = "the-locator";
+
+  private int locatorPort;
+  private int httpServicePort;
+  private Path configWithGeodePluginsFile;
+  private Path configWithoutGeodePluginsFile;
+  private Process locator;
+  private Path geodeDependencies;
+  private Path stdoutFile;
+  private Path locatorLogFile;
+  private Path pulseLogFile;
+
+  @Rule
+  public RequiresGeodeHome requiresGeodeHome = new RequiresGeodeHome();
+  @Rule
+  public TemporaryFolder temporaryFolder = new TemporaryFolder();
+
+  @Before
+  public void setUpLogConfigFiles() {
+    configWithGeodePluginsFile = createFileFromResource(
+        getResource(CONFIG_WITH_GEODE_PLUGINS_FILE_NAME), temporaryFolder.getRoot(),
+        CONFIG_WITH_GEODE_PLUGINS_FILE_NAME)
+            .toPath();
+
+    configWithoutGeodePluginsFile = createFileFromResource(
+        getResource(CONFIG_WITHOUT_GEODE_PLUGINS_FILE_NAME), temporaryFolder.getRoot(),
+        CONFIG_WITHOUT_GEODE_PLUGINS_FILE_NAME)
+            .toPath();
+  }
+
+  @Before
+  public void setUpGeodeDependencies() {
+    Path geodeHome = requiresGeodeHome.getGeodeHome().toPath();
+    geodeDependencies = geodeHome.resolve("lib/geode-dependencies.jar");
+
+    assertThat(geodeDependencies).exists();
+  }
+
+  @Before
+  public void setUpOutputFiles() {
+    stdoutFile = temporaryFolder.getRoot().toPath().resolve("stdout.txt");
+    locatorLogFile = temporaryFolder.getRoot().toPath().resolve(LOCATOR_NAME + ".log");
+    pulseLogFile = temporaryFolder.getRoot().toPath().resolve("pulse.log");
+  }
+
+  @Before
+  public void setUpRandomPorts() {
+    int[] ports = AvailablePortHelper.getRandomAvailableTCPPorts(2);
+    locatorPort = ports[0];
+    httpServicePort = ports[1];
+  }
+
+  @After
+  public void stopLocator() throws Exception {
+    if (locator != null) {
+      locator.destroyForcibly().waitFor(4, TimeUnit.SECONDS);
+    }
+  }
+
+  @Test
+  public void locatorLauncherUsesSpecifiedConfigFileWithoutGeodePlugins() throws Exception {
+    ProcessBuilder processBuilder = new ProcessBuilder()
+        .redirectErrorStream(true)
+        .redirectOutput(stdoutFile.toFile())
+        .directory(temporaryFolder.getRoot())
+        .command("java",
+            "-Dgemfire.http-service-port=" + httpServicePort,
+            "-Dgemfire.jmx-manager-start=true",
+            "-Djava.awt.headless=true",
+            "-Dlog4j.configurationFile=" + configWithoutGeodePluginsFile.toAbsolutePath(),
+            "-cp", geodeDependencies.toFile().getAbsolutePath(),
+            "org.apache.geode.distributed.LocatorLauncher", "start", LOCATOR_NAME,
+            "--port", String.valueOf(locatorPort));
+
+    System.out.println("Launching command: " + processBuilder.command());
+
+    locator = processBuilder
+        .start();
+
+    assertThat(locator.isAlive()).isTrue();
+
+    await().untilAsserted(() -> {
+      assertThat(locatorLogFile)
+          .doesNotExist();
+
+      assertThat(pulseLogFile)
+          .doesNotExist();
+
+      LogFileAssert.assertThat(stdoutFile.toFile())
+          .exists()
+          .contains("Located war: geode-pulse")
+          .contains("Adding webapp /pulse")
+          .contains("Starting server location for Distribution Locator")
+          .doesNotContain("geode-pulse war file was not found")
+          .doesNotContain("java.lang.IllegalStateException: No factory method found for class");
+    });
+  }
+
+  @Test
+  public void locatorLauncherUsesConfigFileInClasspathWithoutGeodePlugins() throws Exception {
+    copy(configWithoutGeodePluginsFile, temporaryFolder.getRoot().toPath().resolve("log4j2.xml"));
+
+    String classpath = temporaryFolder.getRoot().getAbsolutePath() + File.pathSeparator +
+        geodeDependencies.toFile().getAbsolutePath();
+
+    ProcessBuilder processBuilder = new ProcessBuilder()
+        .redirectErrorStream(true)
+        .redirectOutput(stdoutFile.toFile())
+        .directory(temporaryFolder.getRoot())
+        .command("java",
+            "-Dgemfire.http-service-port=" + httpServicePort,
+            "-Dgemfire.jmx-manager-start=true",
+            "-Djava.awt.headless=true",
+            "-cp", classpath,
+            "org.apache.geode.distributed.LocatorLauncher", "start", LOCATOR_NAME,
+            "--port", String.valueOf(locatorPort));
+
+    System.out.println("Launching command: " + processBuilder.command());
+
+    locator = processBuilder
+        .start();
+
+    assertThat(locator.isAlive()).isTrue();
+
+    await().untilAsserted(() -> {
+      assertThat(locatorLogFile)
+          .doesNotExist();
+
+      assertThat(pulseLogFile)
+          .doesNotExist();
+
+      LogFileAssert.assertThat(stdoutFile.toFile())
+          .exists()
+          .contains("Located war: geode-pulse")
+          .contains("Adding webapp /pulse")
+          .contains("Starting server location for Distribution Locator")
+          .doesNotContain("geode-pulse war file was not found")
+          .doesNotContain("java.lang.IllegalStateException: No factory method found for class");
+    });
+  }
+
+  @Test
+  @Ignore("GEODE-8197")
+  public void locatorLauncherUsesSpecifiedConfigFileWithGeodePlugins() throws Exception {
+    ProcessBuilder processBuilder = new ProcessBuilder()
+        .redirectErrorStream(true)
+        .redirectOutput(stdoutFile.toFile())
+        .directory(temporaryFolder.getRoot())
+        .command("java",
+            "-Dgemfire.http-service-port=" + httpServicePort,
+            "-Dgemfire.jmx-manager-start=true",
+            "-Djava.awt.headless=true",
+            "-Dlog4j.configurationFile=" + configWithGeodePluginsFile.toAbsolutePath(),
+            "-cp", geodeDependencies.toFile().getAbsolutePath(),
+            "org.apache.geode.distributed.LocatorLauncher", "start", LOCATOR_NAME,
+            "--port", String.valueOf(locatorPort));
+
+    System.out.println("Launching command: " + processBuilder.command());
+
+    locator = processBuilder
+        .start();
+
+    assertThat(locator.isAlive()).isTrue();
+
+    await().untilAsserted(() -> {
+      assertThat(locatorLogFile)
+          .doesNotExist();
+
+      assertThat(pulseLogFile)
+          .doesNotExist();
+
+      LogFileAssert.assertThat(stdoutFile.toFile())
+          .exists()
+          .contains("Located war: geode-pulse")
+          .contains("Adding webapp /pulse")
+          .contains("Starting server location for Distribution Locator")
+          .doesNotContain("geode-pulse war file was not found")
+          .doesNotContain("java.lang.IllegalStateException: No factory method found for class");
+    });
+  }
+
+  @Test
+  @Ignore("GEODE-8197")

Review comment:
       My understanding is that once GEODE-8197 related bug is fixed, this `@Ignore` annotation will be removed.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org