Bug 494650: Refactor BaseTestCase to allow multiple launches per test

Bug 494650 has an issues when multiple launches are terminated, at
present the test infrastructure makes it very difficult to launch
multiple launches within one test. This commit refactors the base test
case to enable launching additional tests with doLaunchInner.

Change-Id: I501edf4e485c304b0a00c18f1d5e3813011a0491
Signed-off-by: Jonah Graham <jonah@kichwacoders.com>

2 files changed, 251 insertions, 81 deletions

diff --git a/dsf-gdb/org.eclipse.cdt.tests.dsf.gdb/src/org/eclipse/cdt/tests/dsf/gdb/framework/BaseTestCase.java b/dsf-gdb/org.eclipse.cdt.tests.dsf.gdb/src/org/eclipse/cdt/tests/dsf/gdb/framework/BaseTestCase.java
index fb60f5b91d7..3c2c87c2d9d 100644
--- a/dsf-gdb/org.eclipse.cdt.tests.dsf.gdb/src/org/eclipse/cdt/tests/dsf/gdb/framework/BaseTestCase.java
+++ b/dsf-gdb/org.eclipse.cdt.tests.dsf.gdb/src/org/eclipse/cdt/tests/dsf/gdb/framework/BaseTestCase.java
@@ -11,6 +11,7 @@
  *******************************************************************************/
 package org.eclipse.cdt.tests.dsf.gdb.framework;
 
+import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
@@ -25,6 +26,7 @@ import java.util.HashSet;
 import java.util.Map;
 import java.util.NoSuchElementException;
 import java.util.Set;
+import java.util.concurrent.Callable;
 import java.util.concurrent.TimeUnit;
 import java.util.stream.Collectors;
 
@@ -101,6 +103,9 @@ public class BaseTestCase {
     // The set of attributes used for the launch of a single test.
 	private Map<String, Object> launchAttributes;
 
+	// The launch configuration generated from the launch attributes
+	private ILaunchConfiguration fLaunchConfiguration;
+
 	// A set of global launch attributes which are not
 	// reset when we load a new class of tests.
 	// This allows a SuiteGdb to set an attribute
@@ -113,12 +118,6 @@ public class BaseTestCase {
 	/** The MI event associated with the breakpoint at main() */
 	private MIStoppedEvent fInitialStoppedEvent;
 
-	/** Flag we set to true when the target has reached the breakpoint at main() */
-	private boolean fTargetSuspended;
-
-	/** Event semaphore we set when the target has reached the breakpoint at main() */
-	final private Object fTargetSuspendedSem = new Object(); // just used as a semaphore
-
 	private static boolean fgStatusHandlersEnabled = true;
 
 	/** global cache of gdb versions, to avoid running gdb every time just to check if it is present*/
@@ -129,8 +128,15 @@ public class BaseTestCase {
 
 	private HashMap<String, Integer> fTagLocations = new HashMap<>();
 
+	/**
+	 * Return the launch created when {@link #doLaunch()} was called.
+	 */
     public GdbLaunch getGDBLaunch() { return fLaunch; }
 
+	public ILaunchConfiguration getLaunchConfiguration() {
+		return fLaunchConfiguration;
+	}
+
     public void setLaunchAttribute(String key, Object value) {
     	launchAttributes.put(key, value);
     }
@@ -168,16 +174,33 @@ public class BaseTestCase {
 	 * get to the breakpoint state, as we have no further need to monitor events
 	 * beyond that point.
 	 */
-    protected class SessionEventListener {
-    	private DsfSession fSession;
+	protected static class SessionEventListener {
+		private DsfSession fSession;
 
-		SessionEventListener(DsfSession session) {
-    		fSession = session;
-    		Assert.assertNotNull(session);
-    	}
+		/** The MI event associated with the breakpoint at main() */
+		private MIStoppedEvent fInitialStoppedEvent;
+
+		/** Event semaphore we set when the target has reached the breakpoint at main() */
+		final private Object fTargetSuspendedSem = new Object(); // just used as a semaphore
+
+		/** Flag we set to true when the target has reached the breakpoint at main() */
+		private boolean fTargetSuspended;
+
+		private ILaunchConfiguration fLaunchConfiguration;
+
+		public SessionEventListener(ILaunchConfiguration launchConfiguration) {
+			fLaunchConfiguration = launchConfiguration;
+		}
+
+		public void setSession(DsfSession session) {
+			fSession = session;
+			Assert.assertNotNull(fSession);
+		}
 
 		@DsfServiceEventHandler
-    	public void eventDispatched(IDMEvent<?> event) {
+		public void eventDispatched(IDMEvent<?> event) {
+			Assert.assertNotNull(fSession);
+
 			// Wait for the program to have stopped on main.
 			//
 			// We have to jump through hoops to properly handle the remote
@@ -195,36 +218,57 @@ public class BaseTestCase {
 			// is to look for an ISuspendedDMEvent and then confirming that it indicates
 			// in its frame that it stopped at "main".  This will allow us to skip
 			// the first *stopped event for GDB >= 7.0
-    		if (event instanceof ISuspendedDMEvent) {
-    			if (event instanceof IMIDMEvent) {
-    				IMIDMEvent iMIEvent = (IMIDMEvent)event;
-
-    				Object miEvent = iMIEvent.getMIEvent();
-    				if (miEvent instanceof MIStoppedEvent) {
-    					// Store the corresponding MI *stopped event
-    					fInitialStoppedEvent = (MIStoppedEvent)miEvent;
-
-    					// Check the content of the frame for the method we should stop at
-    					String stopAt = (String)launchAttributes.get(ICDTLaunchConfigurationConstants.ATTR_DEBUGGER_STOP_AT_MAIN_SYMBOL);
-    					if (stopAt == null) stopAt = "main";
-
-    					MIFrame frame = fInitialStoppedEvent.getFrame();
-    					if (frame != null &&
-    							frame.getFunction() != null && frame.getFunction().indexOf(stopAt) != -1) {
-    						// Set the event semaphore that will allow the test to proceed
-    						synchronized (fTargetSuspendedSem) {
-    							fTargetSuspended = true;
-    							fTargetSuspendedSem.notify();
-    						}
-
-    						// We found our event, no further need for this listener
-    						fSession.removeServiceEventListener(this);
-    					}
-    				}
-    			}
-    		}
-    	}
-    }
+			if (event instanceof ISuspendedDMEvent) {
+				if (event instanceof IMIDMEvent) {
+					IMIDMEvent iMIEvent = (IMIDMEvent) event;
+
+					Object miEvent = iMIEvent.getMIEvent();
+					if (miEvent instanceof MIStoppedEvent) {
+						// Store the corresponding MI *stopped event
+						fInitialStoppedEvent = (MIStoppedEvent) miEvent;
+
+						// Check the content of the frame for the method we
+						// should stop at
+						String stopAt = null;
+						try {
+							stopAt = fLaunchConfiguration.getAttribute(
+									ICDTLaunchConfigurationConstants.ATTR_DEBUGGER_STOP_AT_MAIN_SYMBOL, "main");
+						} catch (CoreException e) {
+						}
+						if (stopAt == null)
+							stopAt = "main";
+
+						MIFrame frame = fInitialStoppedEvent.getFrame();
+						if (frame != null && frame.getFunction() != null && frame.getFunction().indexOf(stopAt) != -1) {
+							// Set the event semaphore that will allow the test
+							// to proceed
+							synchronized (fTargetSuspendedSem) {
+								fTargetSuspended = true;
+								fTargetSuspendedSem.notify();
+							}
+
+							// We found our event, no further need for this
+							// listener
+							fSession.removeServiceEventListener(this);
+						}
+					}
+				}
+			}
+		}
+
+		public void waitUntilTargetSuspended() throws InterruptedException {
+			if (!fTargetSuspended) {
+				synchronized (fTargetSuspendedSem) {
+					fTargetSuspendedSem.wait(TestsPlugin.massageTimeout(2000));
+					Assert.assertTrue(fTargetSuspended);
+				}
+			}
+		}
+
+		public MIStoppedEvent getInitialStoppedEvent() {
+			return fInitialStoppedEvent;
+		}
+	}
 
 	/**
 	 * Make sure we are starting with a clean/known state. That means no
@@ -360,9 +404,6 @@ public class BaseTestCase {
     		GdbDebugOptions.trace("===============================================================================================\n");
     	}
 
- 		boolean postMortemLaunch = launchAttributes.get(ICDTLaunchConfigurationConstants.ATTR_DEBUGGER_START_MODE)
-	                                               .equals(ICDTLaunchConfigurationConstants.DEBUGGER_MODE_CORE);
-
 		launchGdbServer();
 
  		ILaunchManager launchMgr = DebugPlugin.getDefault().getLaunchManager();
@@ -375,15 +416,44 @@ public class BaseTestCase {
  		assert lcWorkingCopy != null;
  		lcWorkingCopy.setAttributes(launchAttributes);
 
- 		final ILaunchConfiguration lc = lcWorkingCopy.doSave();
+ 		fLaunchConfiguration = lcWorkingCopy.doSave();
+ 		fLaunch = doLaunchInner();
+
+ 		// If we started a gdbserver add it to the launch to make sure it is killed at the end
+ 		if (gdbserverProc != null) {
+            DebugPlugin.newProcess(fLaunch, gdbserverProc, "gdbserver");
+ 		}
+
+ 		// Now initialize our SyncUtility, since we have the launcher
+ 		SyncUtil.initialize(fLaunch.getSession());
+
+	}
+
+ 	/**
+ 	 * Perform the actual launch. This is normally called by {@link #doLaunch()}, however
+ 	 * it can be called repeatedly after an initial doLaunch sets up the environment. Doing
+ 	 * so allows multiple launches on the same launch configuration to be created. When this
+ 	 * method is called directly, the returned launch is not tracked and it is up to the
+ 	 * individual test to cleanup the launch. If the launch is not cleaned up, subsequent
+ 	 * tests will fail due to checks in {@link #doBeforeTest()} that verify state is clean
+ 	 * and no launches are currently running.
+ 	 * 
+ 	 * This method is blocking until the breakpoint at main in the program is reached.
+ 	 * 
+ 	 * @return the new launch created
+ 	 */
+	protected GdbLaunch doLaunchInner() throws Exception {
+		assertNotNull("The launch configuration has not been created. Call doLaunch first.", fLaunchConfiguration);
+		
+ 		boolean postMortemLaunch = launchAttributes.get(ICDTLaunchConfigurationConstants.ATTR_DEBUGGER_START_MODE)
+                .equals(ICDTLaunchConfigurationConstants.DEBUGGER_MODE_CORE);
 
-		// Register ourselves as a listener for the new session so that we can
-		// register ourselves with that particular session before any events
-		// occur. We want to find out when the break on main() occurs.
+ 		SessionEventListener sessionEventListener = new SessionEventListener(fLaunchConfiguration);
  		SessionStartedListener sessionStartedListener = new SessionStartedListener() {
 			@Override
 			public void sessionStarted(DsfSession session) {
-				session.addServiceEventListener(new SessionEventListener(session), null);
+				sessionEventListener.setSession(session);
+				session.addServiceEventListener(sessionEventListener, null);
 			}
 		};
 
@@ -391,50 +461,64 @@ public class BaseTestCase {
 		// before the launch() call returns (unless, of course, there was a
 		// problem launching and no session is created).
  		DsfSession.addSessionStartedListener(sessionStartedListener);
- 		fLaunch = (GdbLaunch)lc.launch(ILaunchManager.DEBUG_MODE, new NullProgressMonitor());
+ 		GdbLaunch launch = (GdbLaunch)fLaunchConfiguration.launch(ILaunchManager.DEBUG_MODE, new NullProgressMonitor());
  		DsfSession.removeSessionStartedListener(sessionStartedListener);
 
- 		// If we haven't hit main() yet,
- 		// wait for the program to hit the breakpoint at main() before
-		// proceeding. All tests assume that stable initial state. Two
-		// seconds is plenty; we typically get to that state in a few
-		// hundred milliseconds with the tiny test programs we use.
- 		if (!postMortemLaunch && !fTargetSuspended) {
- 			synchronized (fTargetSuspendedSem) {
- 				fTargetSuspendedSem.wait(TestsPlugin.massageTimeout(2000));
- 				Assert.assertTrue(fTargetSuspended);
- 			}
- 		}
-
- 		// This should be a given if the above check passes
- 		if (!postMortemLaunch) {
- 			synchronized(this) {
- 				Assert.assertNotNull(fInitialStoppedEvent);
+ 		try {
+
+ 	 		// If we haven't hit main() yet,
+ 	 		// wait for the program to hit the breakpoint at main() before
+ 			// proceeding. All tests assume that stable initial state. Two
+ 			// seconds is plenty; we typically get to that state in a few
+ 			// hundred milliseconds with the tiny test programs we use.
+ 			if (!postMortemLaunch) {
+ 				sessionEventListener.waitUntilTargetSuspended();
+ 	 		}
+
+ 	 		// This should be a given if the above check passes
+ 	 		if (!postMortemLaunch) {
+ 	 			synchronized(this) {
+ 	 				MIStoppedEvent initialStoppedEvent = sessionEventListener.getInitialStoppedEvent();
+ 	 				Assert.assertNotNull(initialStoppedEvent);
+					if (fInitialStoppedEvent == null) {
+						// On the very first launch we do, save the initial stopped event
+						// XXX: If someone writes a test with an additional launch
+						// that needs this info, they should resolve this return value then
+						fInitialStoppedEvent = initialStoppedEvent;
+					}
+ 	 			}
+ 	 		}
+ 	 		
+ 		} catch (Exception e) {
+ 			try {
+ 				launch.terminate();
+ 				assertLaunchTerminates(launch);
+ 			} catch (Exception inner) {
+ 				e.addSuppressed(inner);
  			}
+ 			throw e;
  		}
-
- 		// If we started a gdbserver add it to the launch to make sure it is killed at the end
- 		if (gdbserverProc != null) {
-            DebugPlugin.newProcess(fLaunch, gdbserverProc, "gdbserver");
- 		}
-
- 		// Now initialize our SyncUtility, since we have the launcher
- 		SyncUtil.initialize(fLaunch.getSession());
-
-	}
+		
+		return launch;
+	} 	
 
 	/**
 	 * Assert that the launch terminates. Callers should have already
 	 * terminated the launch in some way.
 	 */
 	protected void assertLaunchTerminates() throws Exception {
-		if (fLaunch != null) {
+		GdbLaunch launch = fLaunch;
+		assertLaunchTerminates(launch);
+	}
+
+	protected void assertLaunchTerminates(GdbLaunch launch) throws InterruptedException {
+		if (launch != null) {
 			// Give a few seconds to allow the launch to terminate
 			int waitCount = 100;
-			while (!fLaunch.isTerminated() && --waitCount > 0) {
+			while (!launch.isTerminated() && --waitCount > 0) {
 				Thread.sleep(TestsPlugin.massageTimeout(100));
 			}
-			assertTrue("Launch failed to terminate before timeout", fLaunch.isTerminated());
+			assertTrue("Launch failed to terminate before timeout", launch.isTerminated());
 		}
 	}
 
@@ -594,4 +678,23 @@ public class BaseTestCase {
 		IEclipsePreferences node = InstanceScope.INSTANCE.getNode(DebugPlugin.getUniqueIdentifier());
 		node.putBoolean(IInternalDebugCoreConstants.PREF_ENABLE_STATUS_HANDLERS, fgStatusHandlersEnabled);
 	}
+	
+	/**
+	 * Wait until the given callable returns true, must be within timeout millis.
+	 */
+	protected void waitUntil(String message, Callable<Boolean> callable,  long millis) throws Exception {
+		long endTime = System.currentTimeMillis() + millis;
+		while (!callable.call() && System.currentTimeMillis() < endTime) {
+			Thread.sleep(100);
+		}
+		assertTrue(message, callable.call());
+	}
+
+	/**
+	 * Wait until the given callable returns true, must be within default timeout.
+	 */
+	protected void waitUntil(String message, Callable<Boolean> callable) throws Exception {
+		waitUntil(message, callable, TestsPlugin.massageTimeout(2000));
+	}
+	
 }
diff --git a/dsf-gdb/org.eclipse.cdt.tests.dsf.gdb/src/org/eclipse/cdt/tests/dsf/gdb/tests/GDBTestTest.java b/dsf-gdb/org.eclipse.cdt.tests.dsf.gdb/src/org/eclipse/cdt/tests/dsf/gdb/tests/GDBTestTest.java
new file mode 100644
index 00000000000..efd463647ec
--- /dev/null
+++ b/dsf-gdb/org.eclipse.cdt.tests.dsf.gdb/src/org/eclipse/cdt/tests/dsf/gdb/tests/GDBTestTest.java
@@ -0,0 +1,67 @@
+/*******************************************************************************
+ * Copyright (c) 2016 Kichwa Coders and others.
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ *
+ * Contributors:
+ *    Jonah Graham (Kichwa Coders) - initial API and implementation
+ *******************************************************************************/
+package org.eclipse.cdt.tests.dsf.gdb.tests;
+
+import static org.junit.Assert.assertFalse;
+
+import org.eclipse.cdt.debug.core.ICDTLaunchConfigurationConstants;
+import org.eclipse.cdt.dsf.gdb.launching.GdbLaunch;
+import org.eclipse.cdt.tests.dsf.gdb.framework.BaseParametrizedTestCase;
+import org.junit.Assume;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+/**
+ * These are basic tests to demonstrate that the test infrastructure works as
+ * expected. This class can also be used as a starting point for additional test
+ * cases as this class tries to stay simple.
+ */
+@RunWith(Parameterized.class)
+public class GDBTestTest extends BaseParametrizedTestCase {
+	private static final String EXEC_NAME = "MultiThread.exe";
+
+	@Override
+	protected void setLaunchAttributes() {
+		super.setLaunchAttributes();
+		setLaunchAttribute(ICDTLaunchConfigurationConstants.ATTR_PROGRAM_NAME, EXEC_PATH + EXEC_NAME);
+	}
+
+	/**
+	 * Test that we can launch, as the launch and terminate is all handled in
+	 * before/after test, this test looks pretty minimal.
+	 */
+	@Test
+	public void testLaunch() {
+		assertFalse("Launch should be running", getGDBLaunch().isTerminated());
+	}
+	
+	/**
+	 * Test that test infrastructure allows multiple launches on same launch config. 
+	 */
+	@Test
+	public void testMultipleLaunch() throws Exception {
+		Assume.assumeFalse("Test framework only supports multiple launches for non-remote", remote);
+		
+		// get the launch that was created automatically
+		GdbLaunch autoLaunched = getGDBLaunch();
+		
+		autoLaunched.terminate();
+		waitUntil("Launch did not terminate", () -> autoLaunched.isTerminated());
+		
+		// launch an additional launch
+		GdbLaunch secondLaunch = doLaunchInner();
+		assertFalse("Second launch should be running", secondLaunch.isTerminated());
+		
+		secondLaunch.terminate();
+		waitUntil("Second launch did not terminate", () -> secondLaunch.isTerminated());
+	}
+}