pubsub: fix testStartStopReset

The test verifies that calling stop() actually stops the emulator.
The previous implementation stops the emulator by
- sending HTTP POST to /shutdown
- terminating the emulator process
The second step follows directly after the first.
It then queries the server it just shut down to make sure it does not
respond.

This behavior is incorrect for two reasons.
- The server might continue serving request after responding to
  /shutdown.
- The "emulator process" might not be the emulator at all.
  It might be a script that in-turn runs the emulator.
  On UNIX systems, killing a parent process does not kill any of its
  children; the actual emulator might run indefinitely after
  we kill the process we spawned.

This commit changes this behavior.
After POSTing to /shutdown, we simply wait for the spawned process
to terminate on its own.

Technically, there is still a race:
Another emulator could have been started on the same port.
When we send a request to verify that the old emulator has terminated,
we might get a response from the new one.
Since we execute tests sequentially though,
I believe this should not be a problem in practice.

Updates #1429.

Author: pongad

google-cloud-core/src/main/java/com/google/cloud/testing/BaseEmulatorHelper.java

@@ -104,15 +104,16 @@ protected final void startProcess(String blockUntilOutput)
   }
 
   /**
-   * Stops the local service's subprocess and any possible thread listening for its output.
+   * Waits for the local service's subprocess to terminate,
+   * and stop any possible thread listening for its output.
    */
-  protected final void stopProcess() throws IOException, InterruptedException {
+  protected final void waitForProcess() throws IOException, InterruptedException {
     if (blockingProcessReader != null) {
       blockingProcessReader.terminate();
       blockingProcessReader = null;
     }
     if (activeRunner != null) {
-      activeRunner.stop();
+      activeRunner.waitFor();
       activeRunner = null;
     }
   }
@@ -195,9 +196,9 @@ protected interface EmulatorRunner {
     void start() throws IOException;
 
     /**
-     * Stops the emulator associated to this runner.
+     * Wait for the emulator associated to this runner to terminate.
      */
-    void stop() throws InterruptedException;
+    void waitFor() throws InterruptedException;
 
     /**
      * Returns the process associated to the emulator, if any.
@@ -239,9 +240,8 @@ public void start() throws IOException {
     }
 
     @Override
-    public void stop() throws InterruptedException {
+    public void waitFor() throws InterruptedException {
       if (process != null) {
-        process.destroy();
         process.waitFor();
       }
     }
@@ -337,9 +337,8 @@ public void start() throws IOException {
     }
 
     @Override
-    public void stop() throws InterruptedException {
+    public void waitFor() throws InterruptedException {
       if (process != null) {
-        process.destroy();
         process.waitFor();
       }
     }

google-cloud-core/src/test/java/com/google/cloud/testing/BaseEmulatorHelperTest.java

@@ -67,7 +67,7 @@ public void start() throws IOException, InterruptedException {
 
     @Override
     public void stop() throws IOException, InterruptedException {
-      stopProcess();
+      waitForProcess();
     }
 
     @Override
@@ -86,7 +86,7 @@ public void testEmulatorHelper() throws IOException, InterruptedException {
     emulatorRunner.start();
     EasyMock.expectLastCall();
     EasyMock.expect(emulatorRunner.getProcess()).andReturn(process);
-    emulatorRunner.stop();
+    emulatorRunner.waitFor();
     EasyMock.expectLastCall();
     EasyMock.replay(process, emulatorRunner);
     TestEmulatorHelper helper =
@@ -108,7 +108,7 @@ public void testEmulatorHelperMultipleRunners() throws IOException, InterruptedE
     secondRunner.start();
     EasyMock.expectLastCall();
     EasyMock.expect(secondRunner.getProcess()).andReturn(process);
-    secondRunner.stop();
+    secondRunner.waitFor();
     EasyMock.expectLastCall();
     EasyMock.replay(process, secondRunner);
     TestEmulatorHelper helper =

google-cloud-datastore/src/main/java/com/google/cloud/datastore/testing/LocalDatastoreHelper.java

@@ -227,7 +227,7 @@ public void reset() throws IOException {
    */
   public void stop() throws IOException, InterruptedException {
     sendPostRequest("/shutdown");
-    stopProcess();
+    waitForProcess();
     deleteRecursively(gcdPath);
   }
 

google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/testing/LocalPubSubHelper.java

@@ -146,6 +146,6 @@ public void reset() throws IOException {
   @Override
   public void stop() throws IOException, InterruptedException {
     sendPostRequest("/shutdown");
-    stopProcess();
+    waitForProcess();
   }
 }