[ 
https://issues.apache.org/jira/browse/GEODE-3413?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16120742#comment-16120742
 ] 

ASF GitHub Bot commented on GEODE-3413:
---------------------------------------

Github user kirklund commented on a diff in the pull request:

    https://github.com/apache/geode/pull/699#discussion_r132322055
  
    --- Diff: 
geode-core/src/test/java/org/apache/geode/distributed/AbstractLauncherTest.java 
---
    @@ -14,253 +14,370 @@
      */
     package org.apache.geode.distributed;
     
    +import static java.util.concurrent.TimeUnit.DAYS;
    +import static java.util.concurrent.TimeUnit.HOURS;
    +import static java.util.concurrent.TimeUnit.MILLISECONDS;
    +import static java.util.concurrent.TimeUnit.MINUTES;
    +import static java.util.concurrent.TimeUnit.SECONDS;
    +import static 
org.apache.geode.distributed.AbstractLauncher.ServiceState.toDaysHoursMinutesSeconds;
     import static org.apache.geode.distributed.ConfigurationProperties.NAME;
    -import static org.junit.Assert.assertEquals;
    -import static org.junit.Assert.assertFalse;
    -import static org.junit.Assert.assertNotNull;
    -import static org.junit.Assert.assertNull;
    -import static org.junit.Assert.assertTrue;
    +import static org.assertj.core.api.Assertions.assertThat;
    +import static org.assertj.core.api.Assertions.entry;
     import static org.mockito.Mockito.mock;
     import static org.mockito.Mockito.times;
     import static org.mockito.Mockito.verify;
     
    +import java.net.URL;
    +import java.util.Properties;
    +
     import org.apache.commons.lang.StringUtils;
    -import org.apache.geode.test.junit.categories.UnitTest;
     import org.junit.Test;
     import org.junit.experimental.categories.Category;
     
    -import java.net.MalformedURLException;
    -import java.net.URL;
    -import java.util.Properties;
    -import java.util.concurrent.TimeUnit;
    +import org.apache.geode.test.junit.categories.UnitTest;
     
     /**
    - * The AbstractLauncherTest class is a test suite of unit tests testing 
the contract and
    - * functionality of the AbstractLauncher class.
    - * <p/>
    - * 
    - * @see org.apache.geode.distributed.AbstractLauncher
    - * @see org.junit.Assert
    - * @see org.junit.Test
    + * Unit tests for {@link AbstractLauncher}.
    + *
      * @since GemFire 7.0
      */
     @Category(UnitTest.class)
     public class AbstractLauncherTest {
     
    -  private AbstractLauncher<?> createAbstractLauncher(final String 
memberName,
    -      final String memberId) {
    -    return new FakeServiceLauncher(memberName, memberId);
    -  }
    -
       @Test
    -  public void shouldBeMockable() throws Exception {
    +  public void canBeMocked() throws Exception {
         AbstractLauncher mockAbstractLauncher = mock(AbstractLauncher.class);
         mockAbstractLauncher.setDebug(true);
         verify(mockAbstractLauncher, times(1)).setDebug(true);
       }
     
       @Test
    -  public void testIsSet() {
    -    final Properties properties = new Properties();
    +  public void isSetReturnsFalseIfPropertyDoesNotExist() throws Exception {
    +    assertThat(AbstractLauncher.isSet(new Properties(), NAME)).isFalse();
    +  }
     
    -    assertFalse(properties.containsKey(NAME));
    -    assertFalse(AbstractLauncher.isSet(properties, NAME));
    +  @Test
    +  public void isSetReturnsFalseIfPropertyHasEmptyValue() throws Exception {
    +    Properties properties = new Properties();
     
         properties.setProperty(NAME, "");
     
    -    assertTrue(properties.containsKey(NAME));
    -    assertFalse(AbstractLauncher.isSet(properties, NAME));
    +    assertThat(AbstractLauncher.isSet(properties, NAME)).isFalse();
    +  }
    +
    +  @Test
    +  public void isSetReturnsFalseIfPropertyHasBlankValue() throws Exception {
    +    Properties properties = new Properties();
     
         properties.setProperty(NAME, "  ");
     
    -    assertTrue(properties.containsKey(NAME));
    -    assertFalse(AbstractLauncher.isSet(properties, NAME));
    +    assertThat(AbstractLauncher.isSet(properties, NAME)).isFalse();
    +  }
    +
    +  @Test
    +  public void isSetReturnsFalseIfPropertyHasRealValue() throws Exception {
    +    Properties properties = new Properties();
     
         properties.setProperty(NAME, "memberOne");
     
    -    assertTrue(AbstractLauncher.isSet(properties, NAME));
    -    assertFalse(AbstractLauncher.isSet(properties, "NaMe"));
    +    assertThat(AbstractLauncher.isSet(properties, NAME)).isTrue();
       }
     
       @Test
    -  public void testLoadGemFirePropertiesWithNullURL() {
    -    final Properties properties = 
AbstractLauncher.loadGemFireProperties(null);
    -    assertNotNull(properties);
    -    assertTrue(properties.isEmpty());
    +  public void isSetKeyIsCaseSensitive() throws Exception {
    +    Properties properties = new Properties();
    +
    +    properties.setProperty(NAME, "memberOne");
    +
    +    assertThat(AbstractLauncher.isSet(properties, "NaMe")).isFalse();
       }
     
       @Test
    -  public void testLoadGemFirePropertiesWithNonExistingURL() throws 
MalformedURLException {
    -    final Properties properties = AbstractLauncher
    -        .loadGemFireProperties(new 
URL("file:///path/to/non_existing/gemfire.properties"));
    -    assertNotNull(properties);
    -    assertTrue(properties.isEmpty());
    +  public void loadGemFirePropertiesWithNullURLReturnsEmptyProperties() 
throws Exception {
    +    URL nullUrl = null;
    +
    +    Properties properties = 
AbstractLauncher.loadGemFireProperties(nullUrl);
    +
    +    assertThat(properties).isNotNull().isEmpty();
       }
     
       @Test
    -  public void testGetDistributedSystemProperties() {
    -    AbstractLauncher<?> launcher = createAbstractLauncher("memberOne", 
"1");
    +  public void 
loadGemFirePropertiesWithNonExistingURLReturnsEmptyProperties() throws 
Exception {
    +    URL nonExistingUrl = new 
URL("file:///path/to/non_existing/gemfire.properties");
    +
    +    Properties properties = 
AbstractLauncher.loadGemFireProperties(nonExistingUrl);
     
    -    assertNotNull(launcher);
    -    assertEquals("1", launcher.getMemberId());
    -    assertEquals("memberOne", launcher.getMemberName());
    +    assertThat(properties).isNotNull().isEmpty();
    +  }
     
    -    Properties distributedSystemProperties = 
launcher.getDistributedSystemProperties();
    +  @Test
    +  public void abstractLauncherHasMemberIdAndMemberName() throws Exception {
    --- End diff --
    
    Deleted.


> Overhaul launcher tests and process tests
> -----------------------------------------
>
>                 Key: GEODE-3413
>                 URL: https://issues.apache.org/jira/browse/GEODE-3413
>             Project: Geode
>          Issue Type: Improvement
>          Components: gfsh
>            Reporter: Kirk Lund
>            Assignee: Kirk Lund
>              Labels: LauncherTest, ProcessTest
>
> The launcher and process tests are closely related and in need of overhauling 
> to improve debugging and remove flakiness.
> In addition, the org.apache.geode.internal.process package is need of 
> improving the test code coverage.
> Launcher tests:
> * 
> geode-assembly/src/test/java/org/apache/geode/distributed/LocatorLauncherAssemblyIntegrationTest.java
> * 
> geode-core/src/test/java/org/apache/geode/distributed/AbstractLauncherIntegrationTest.java
> * 
> geode-core/src/test/java/org/apache/geode/distributed/AbstractLauncherServiceStatusTest.java
> * 
> geode-core/src/test/java/org/apache/geode/distributed/AbstractLauncherTest.java
> * 
> geode-core/src/test/java/org/apache/geode/distributed/LauncherMemberMXBeanIntegrationTest.java
> * 
> geode-core/src/test/java/org/apache/geode/distributed/LocatorLauncherIntegrationTest.java
> * 
> geode-core/src/test/java/org/apache/geode/distributed/LocatorLauncherLocalFileIntegrationTest.java
> * 
> geode-core/src/test/java/org/apache/geode/distributed/LocatorLauncherLocalIntegrationTest.java
> * 
> geode-core/src/test/java/org/apache/geode/distributed/LocatorLauncherRemoteFileIntegrationTest.java
> * 
> geode-core/src/test/java/org/apache/geode/distributed/LocatorLauncherRemoteIntegrationTest.java
> * 
> geode-core/src/test/java/org/apache/geode/distributed/LocatorLauncherRemoteWithCustomLoggingIntegrationTest.java
> * 
> geode-core/src/test/java/org/apache/geode/distributed/LocatorLauncherTest.java
> * geode-core/src/test/java/org/apache/geode/distributed/LocatorStateTest.java
> * 
> geode-core/src/test/java/org/apache/geode/distributed/ServerLauncherIntegrationTest.java
> * 
> geode-core/src/test/java/org/apache/geode/distributed/ServerLauncherLocalFileIntegrationTest.java
> * 
> geode-core/src/test/java/org/apache/geode/distributed/ServerLauncherLocalIntegrationTest.java
> * 
> geode-core/src/test/java/org/apache/geode/distributed/ServerLauncherRemoteFileIntegrationTest.java
> * 
> geode-core/src/test/java/org/apache/geode/distributed/ServerLauncherRemoteIntegrationTest.java
> * 
> geode-core/src/test/java/org/apache/geode/distributed/ServerLauncherRemoteWithCustomLoggingIntegrationTest.java
> * 
> geode-core/src/test/java/org/apache/geode/distributed/ServerLauncherRemoteWithCustomLoggingIntegrationTest.java
> * 
> geode-core/src/test/java/org/apache/geode/distributed/ServerLauncherTest.java
> * 
> geode-core/src/test/java/org/apache/geode/distributed/ServerLauncherWithProviderIntegrationTest.java
> Process tests:
> * 
> geode-core/src/test/java/org/apache/geode/internal/process/BlockingProcessStreamReaderJUnitTest.java
> * 
> geode-core/src/test/java/org/apache/geode/internal/process/FileProcessControllerIntegrationJUnitTest.java
> * 
> geode-core/src/test/java/org/apache/geode/internal/process/LocalProcessControllerJUnitTest.java
> * 
> geode-core/src/test/java/org/apache/geode/internal/process/LocalProcessLauncherDUnitTest.java
> * 
> geode-core/src/test/java/org/apache/geode/internal/process/LocalProcessLauncherJUnitTest.java
> * 
> geode-core/src/test/java/org/apache/geode/internal/process/NonBlockingProcessStreamReaderJUnitTest.java
> * 
> geode-core/src/test/java/org/apache/geode/internal/process/PidFileJUnitTest.java
> * 
> geode-core/src/test/java/org/apache/geode/internal/process/ProcessControllerFactoryJUnitTest.java



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

Reply via email to