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

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_r132305098
  
    --- Diff: 
geode-core/src/test/java/org/apache/geode/internal/process/AbstractProcessStreamReaderIntegrationTest.java
 ---
    @@ -0,0 +1,234 @@
    +/*
    + * 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.internal.process;
    +
    +import static java.util.concurrent.TimeUnit.MILLISECONDS;
    +import static org.apache.commons.lang.SystemUtils.LINE_SEPARATOR;
    +
    +import java.io.File;
    +import java.util.ArrayList;
    +import java.util.Arrays;
    +import java.util.List;
    +
    +import org.awaitility.Awaitility;
    +import org.awaitility.core.ConditionFactory;
    +import org.junit.After;
    +
    +import org.apache.geode.internal.util.StopWatch;
    +
    +/**
    + * Abstract base class for functional integration testing of {@link 
ProcessStreamReader}.
    + */
    +public abstract class AbstractProcessStreamReaderIntegrationTest {
    +
    +  /** Timeout to join to a running ProcessStreamReader thread */
    +  protected static final int READER_JOIN_TIMEOUT_MILLIS = 20 * 1000;
    +
    +  /** Sleep timeout for {@link ProcessSleeps} instead of sleeping 
Long.MAX_VALUE */
    +  private static final int PROCESS_FAILSAFE_TIMEOUT_MILLIS = 10 * 60 * 
1000;
    +
    +  /** Additional time for launched processes to live before terminating */
    +  private static final int PROCESS_TIME_TO_LIVE_MILLIS = 3 * 500;
    +
    +  /** Timeout to wait for a new {@link ProcessStreamReader} to be running 
*/
    +  private static final int WAIT_FOR_READER_IS_RUNNING_TIMEOUT_MILLIS = 20 
* 1000;
    +
    +  protected Process process;
    +  protected ProcessStreamReader stderr;
    +  protected ProcessStreamReader stdout;
    +
    +  @After
    +  public void afterProcessStreamReaderTestCase() throws Exception {
    +    if (stderr != null) {
    +      stderr.stop();
    +    }
    +    if (stdout != null) {
    +      stdout.stop();
    +    }
    +    if (process != null) {
    +      try {
    +        process.getErrorStream().close();
    +        process.getInputStream().close();
    +        process.getOutputStream().close();
    +      } finally {
    +        // this is async and can require more than 10 seconds on slower 
machines
    +        process.destroy();
    +      }
    +    }
    +  }
    +
    +  protected ConditionFactory await() {
    +    return 
Awaitility.await().atMost(WAIT_FOR_READER_IS_RUNNING_TIMEOUT_MILLIS, 
MILLISECONDS);
    +  }
    +
    +  protected static boolean isAlive(final Process process) {
    --- End diff --
    
    All callers to this method should now be changed to use the new API instead 
of this old hack:
    ```java
    process.isAlive()
    ```



> 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