From 6edd1c6a53bde9187722e338dab4c043aed801b8 Mon Sep 17 00:00:00 2001 From: Jonah Graham Date: Fri, 28 Apr 2017 21:11:27 +0100 Subject: [PATCH] Bug 499778: Stop losing stacktraces in tests Rather than try/catch/fail just let exception cascade so that the full stacktrace is available in the test results. The braces have been left in place for scoping and to minimize changes. Change-Id: I5407829ea964f828e3f996794489a7b884de25fb --- .../framework/BaseParametrizedTestCase.java | 7 +- .../tests/dsf/gdb/framework/BaseTestCase.java | 2 +- .../LaunchConfigurationAndRestartTest.java | 116 +++--------------- .../MIRunControlTargetAvailableTest.java | 92 ++------------ .../OperationsWhileTargetIsRunningTest.java | 23 +--- .../dsf/gdb/tests/PostMortemCoreTest.java | 5 +- .../GDBMultiNonStopRunControlTest.java | 11 +- 7 files changed, 34 insertions(+), 222 deletions(-) diff --git a/dsf-gdb/org.eclipse.cdt.tests.dsf.gdb/src/org/eclipse/cdt/tests/dsf/gdb/framework/BaseParametrizedTestCase.java b/dsf-gdb/org.eclipse.cdt.tests.dsf.gdb/src/org/eclipse/cdt/tests/dsf/gdb/framework/BaseParametrizedTestCase.java index 7c4da86abd4..242be8406e0 100644 --- a/dsf-gdb/org.eclipse.cdt.tests.dsf.gdb/src/org/eclipse/cdt/tests/dsf/gdb/framework/BaseParametrizedTestCase.java +++ b/dsf-gdb/org.eclipse.cdt.tests.dsf.gdb/src/org/eclipse/cdt/tests/dsf/gdb/framework/BaseParametrizedTestCase.java @@ -11,7 +11,6 @@ package org.eclipse.cdt.tests.dsf.gdb.framework; import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; import java.util.Arrays; import java.util.Collection; @@ -178,8 +177,8 @@ public abstract class BaseParametrizedTestCase extends BaseTestCase { } @Override - protected void validateGdbVersion(GdbLaunch launch) { - try { + protected void validateGdbVersion(GdbLaunch launch) throws CoreException { + { String expected = getGdbVersionParameter(); if (expected.equals(DEFAULT_VERSION_STRING)) { // If the user has requested the default GDB, we accept whatever version runs. @@ -204,8 +203,6 @@ public abstract class BaseParametrizedTestCase extends BaseTestCase { assertTrue("Unexpected GDB version. Expected " + expected + " actual " + actual, LaunchUtils.compareVersions(expected, comparableActualString) == 0); - } catch (CoreException e) { - fail(e.getMessage()); } } } 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 0bd551e0383..08bb8d5909e 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 @@ -185,7 +185,7 @@ public class BaseTestCase { * * @param launch The launch in which we can find the gdb version */ - protected void validateGdbVersion(GdbLaunch launch) {}; + protected void validateGdbVersion(GdbLaunch launch) throws Exception {}; /** * We listen for the target to stop at the main breakpoint. This listener is diff --git a/dsf-gdb/org.eclipse.cdt.tests.dsf.gdb/src/org/eclipse/cdt/tests/dsf/gdb/tests/LaunchConfigurationAndRestartTest.java b/dsf-gdb/org.eclipse.cdt.tests.dsf.gdb/src/org/eclipse/cdt/tests/dsf/gdb/tests/LaunchConfigurationAndRestartTest.java index fb2ae6348db..57f0e7f4159 100644 --- a/dsf-gdb/org.eclipse.cdt.tests.dsf.gdb/src/org/eclipse/cdt/tests/dsf/gdb/tests/LaunchConfigurationAndRestartTest.java +++ b/dsf-gdb/org.eclipse.cdt.tests.dsf.gdb/src/org/eclipse/cdt/tests/dsf/gdb/tests/LaunchConfigurationAndRestartTest.java @@ -22,9 +22,7 @@ import static org.junit.Assert.fail; import java.io.File; import java.util.HashMap; import java.util.Map; -import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; -import java.util.concurrent.TimeoutException; import org.eclipse.cdt.debug.core.CDIDebugModel; import org.eclipse.cdt.debug.core.ICDTLaunchConfigurationConstants; @@ -182,15 +180,9 @@ public class LaunchConfigurationAndRestartTest extends BaseParametrizedTestCase rm); } }; - try { + { fExpService.getExecutor().execute(query); query.get(TestsPlugin.massageTimeout(500), TimeUnit.MILLISECONDS); - } catch (InterruptedException e) { - fail(e.getMessage()); - } catch (ExecutionException e) { - fail(e.getCause().getMessage()); - } catch (TimeoutException e) { - fail(e.getMessage()); } } @@ -223,7 +215,7 @@ public class LaunchConfigurationAndRestartTest extends BaseParametrizedTestCase try { doLaunch(); } catch (CoreException e) { - fail("Launch has failed even though the gdbinit file has the default name of .gdbinit"); + throw new AssertionError("Launch has failed even though the gdbinit file has the default name of .gdbinit", e); } } @@ -260,19 +252,13 @@ public class LaunchConfigurationAndRestartTest extends BaseParametrizedTestCase fExpService.getFormattedValueContext(argcDmc, MIExpressions.DETAILS_FORMAT), rm); } }; - try { + { fExpService.getExecutor().execute(query); FormattedValueDMData value = query.get(TestsPlugin.massageTimeout(500), TimeUnit.MILLISECONDS); // Argc should be 7: the program name and the six arguments assertTrue("Expected 7 but got " + value.getFormattedValue(), value.getFormattedValue().trim().equals("7")); - } catch (InterruptedException e) { - fail(e.getMessage()); - } catch (ExecutionException e) { - fail(e.getCause().getMessage()); - } catch (TimeoutException e) { - fail(e.getMessage()); } // Check that argv is also correct. For simplicity we only check the last argument @@ -284,17 +270,11 @@ public class LaunchConfigurationAndRestartTest extends BaseParametrizedTestCase fExpService.getFormattedValueContext(argvDmc, MIExpressions.DETAILS_FORMAT), rm); } }; - try { + { fExpService.getExecutor().execute(query2); FormattedValueDMData value = query2.get(TestsPlugin.massageTimeout(500), TimeUnit.MILLISECONDS); assertTrue("Expected \"6\" but got " + value.getFormattedValue(), value.getFormattedValue().trim().endsWith("\"6\"")); - } catch (InterruptedException e) { - fail(e.getMessage()); - } catch (ExecutionException e) { - fail(e.getCause().getMessage()); - } catch (TimeoutException e) { - fail(e.getMessage()); } } @@ -330,17 +310,11 @@ public class LaunchConfigurationAndRestartTest extends BaseParametrizedTestCase fExpService.getFormattedValueContext(exprDmc, MIExpressions.DETAILS_FORMAT), rm); } }; - try { + { fExpService.getExecutor().execute(query); FormattedValueDMData value = query.get(TestsPlugin.massageTimeout(500), TimeUnit.MILLISECONDS); assertTrue("Expected 0x0 but got " + value.getFormattedValue(), value.getFormattedValue().equals("0x0")); - } catch (InterruptedException e) { - fail(e.getMessage()); - } catch (ExecutionException e) { - fail(e.getCause().getMessage()); - } catch (TimeoutException e) { - fail(e.getMessage()); } } @@ -379,17 +353,11 @@ public class LaunchConfigurationAndRestartTest extends BaseParametrizedTestCase fExpService.getFormattedValueContext(exprDmc, MIExpressions.DETAILS_FORMAT), rm); } }; - try { + { fExpService.getExecutor().execute(query); FormattedValueDMData value = query.get(TestsPlugin.massageTimeout(500), TimeUnit.MILLISECONDS); assertTrue("Expected a string ending with \"IS SET\" but got " + value.getFormattedValue(), value.getFormattedValue().trim().endsWith("\"IS SET\"")); - } catch (InterruptedException e) { - fail(e.getMessage()); - } catch (ExecutionException e) { - fail(e.getCause().getMessage()); - } catch (TimeoutException e) { - fail(e.getMessage()); } // Check that the normal environment is there by checking that $HOME (which is stored in 'home" exists. @@ -401,17 +369,11 @@ public class LaunchConfigurationAndRestartTest extends BaseParametrizedTestCase fExpService.getFormattedValueContext(exprDmc2, MIExpressions.DETAILS_FORMAT), rm); } }; - try { + { fExpService.getExecutor().execute(query2); FormattedValueDMData value = query2.get(TestsPlugin.massageTimeout(500), TimeUnit.MILLISECONDS); assertFalse("Expected something else than 0x0", value.getFormattedValue().equals("0x0")); - } catch (InterruptedException e) { - fail(e.getMessage()); - } catch (ExecutionException e) { - fail(e.getCause().getMessage()); - } catch (TimeoutException e) { - fail(e.getMessage()); } } @@ -453,17 +415,11 @@ public class LaunchConfigurationAndRestartTest extends BaseParametrizedTestCase fExpService.getFormattedValueContext(exprDmc, MIExpressions.DETAILS_FORMAT), rm); } }; - try { + { fExpService.getExecutor().execute(query); FormattedValueDMData value = query.get(TestsPlugin.massageTimeout(500), TimeUnit.MILLISECONDS); assertTrue("Expected a string ending with \"IS SET\" but got " + value.getFormattedValue(), value.getFormattedValue().trim().endsWith("\"IS SET\"")); - } catch (InterruptedException e) { - fail(e.getMessage()); - } catch (ExecutionException e) { - fail(e.getCause().getMessage()); - } catch (TimeoutException e) { - fail(e.getMessage()); } // The program has stored the content of $HOME into a variable called 'home'. @@ -476,17 +432,11 @@ public class LaunchConfigurationAndRestartTest extends BaseParametrizedTestCase fExpService.getFormattedValueContext(exprDmc2, MIExpressions.DETAILS_FORMAT), rm); } }; - try { + { fExpService.getExecutor().execute(query2); FormattedValueDMData value = query2.get(TestsPlugin.massageTimeout(500), TimeUnit.MILLISECONDS); assertTrue("Expected 0x0 but got " + value.getFormattedValue(), value.getFormattedValue().equals("0x0")); - } catch (InterruptedException e) { - fail(e.getMessage()); - } catch (ExecutionException e) { - fail(e.getCause().getMessage()); - } catch (TimeoutException e) { - fail(e.getMessage()); } } @@ -523,19 +473,13 @@ public class LaunchConfigurationAndRestartTest extends BaseParametrizedTestCase fExpService.getFormattedValueContext(argcDmc, MIExpressions.DETAILS_FORMAT), rm); } }; - try { + { fExpService.getExecutor().execute(query); FormattedValueDMData value = query.get(TestsPlugin.massageTimeout(500), TimeUnit.MILLISECONDS); // Argc should be 7: the program name and the six arguments assertTrue("Expected 7 but got " + value.getFormattedValue(), value.getFormattedValue().trim().equals("7")); - } catch (InterruptedException e) { - fail(e.getMessage()); - } catch (ExecutionException e) { - fail(e.getCause().getMessage()); - } catch (TimeoutException e) { - fail(e.getMessage()); } // Check that argv is also correct. For simplicity we only check the last argument @@ -547,17 +491,11 @@ public class LaunchConfigurationAndRestartTest extends BaseParametrizedTestCase fExpService.getFormattedValueContext(argvDmc, MIExpressions.DETAILS_FORMAT), rm); } }; - try { + { fExpService.getExecutor().execute(query2); FormattedValueDMData value = query2.get(TestsPlugin.massageTimeout(500), TimeUnit.MILLISECONDS); assertTrue("Expected \"6\" but got " + value.getFormattedValue(), value.getFormattedValue().trim().endsWith("\"6\"")); - } catch (InterruptedException e) { - fail(e.getMessage()); - } catch (ExecutionException e) { - fail(e.getCause().getMessage()); - } catch (TimeoutException e) { - fail(e.getMessage()); } } @@ -745,7 +683,7 @@ public class LaunchConfigurationAndRestartTest extends BaseParametrizedTestCase }); } }; - try { + { fExpService.getExecutor().execute(query); MIBreakListInfo value = query.get(TestsPlugin.massageTimeout(500), TimeUnit.MILLISECONDS); MIBreakpoint[] bps = value.getMIBreakpoints(); @@ -753,12 +691,6 @@ public class LaunchConfigurationAndRestartTest extends BaseParametrizedTestCase bps.length == 1); assertTrue("Expending a breakpoint but got one at " + bps[0].getAddress(), bps[0].getAddress().equals("")); - } catch (InterruptedException e) { - fail(e.getMessage()); - } catch (ExecutionException e) { - fail(e.getCause().getMessage()); - } catch (TimeoutException e) { - fail(e.getMessage()); } } @@ -813,15 +745,9 @@ public class LaunchConfigurationAndRestartTest extends BaseParametrizedTestCase rm); } }; - try { + { fGdbControl.getExecutor().execute(query); query.get(TestsPlugin.massageTimeout(500), TimeUnit.MILLISECONDS); - } catch (InterruptedException e) { - fail(e.getMessage()); - } catch (ExecutionException e) { - fail(e.getCause().getMessage()); - } catch (TimeoutException e) { - fail(e.getMessage()); } stoppedEvent = eventWaitor.waitForEvent(TestsPlugin.massageTimeout(1000)); @@ -889,15 +815,9 @@ public class LaunchConfigurationAndRestartTest extends BaseParametrizedTestCase rm); } }; - try { + { fGdbControl.getExecutor().execute(query2); query2.get(TestsPlugin.massageTimeout(500), TimeUnit.MILLISECONDS); - } catch (InterruptedException e) { - fail(e.getMessage()); - } catch (ExecutionException e) { - fail(e.getCause().getMessage()); - } catch (TimeoutException e) { - fail(e.getMessage()); } stoppedEvent = eventWaitor.waitForEvent(TestsPlugin.massageTimeout(1000)); @@ -972,15 +892,9 @@ public class LaunchConfigurationAndRestartTest extends BaseParametrizedTestCase rm); } }; - try { + { fGdbControl.getExecutor().execute(query2); query2.get(TestsPlugin.massageTimeout(500), TimeUnit.MILLISECONDS); - } catch (InterruptedException e) { - fail(e.getMessage()); - } catch (ExecutionException e) { - fail(e.getCause().getMessage()); - } catch (TimeoutException e) { - fail(e.getMessage()); } stoppedEvent = eventWaitor.waitForEvent(TestsPlugin.massageTimeout(1000)); diff --git a/dsf-gdb/org.eclipse.cdt.tests.dsf.gdb/src/org/eclipse/cdt/tests/dsf/gdb/tests/MIRunControlTargetAvailableTest.java b/dsf-gdb/org.eclipse.cdt.tests.dsf.gdb/src/org/eclipse/cdt/tests/dsf/gdb/tests/MIRunControlTargetAvailableTest.java index 31bd9124d46..c80bc648794 100644 --- a/dsf-gdb/org.eclipse.cdt.tests.dsf.gdb/src/org/eclipse/cdt/tests/dsf/gdb/tests/MIRunControlTargetAvailableTest.java +++ b/dsf-gdb/org.eclipse.cdt.tests.dsf.gdb/src/org/eclipse/cdt/tests/dsf/gdb/tests/MIRunControlTargetAvailableTest.java @@ -15,7 +15,6 @@ import static org.junit.Assert.fail; import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; -import java.util.concurrent.TimeoutException; import org.eclipse.cdt.debug.core.ICDTLaunchConfigurationConstants; import org.eclipse.cdt.dsf.concurrent.CountingRequestMonitor; @@ -47,9 +46,6 @@ import org.junit.runners.Parameterized; */ @RunWith(Parameterized.class) public class MIRunControlTargetAvailableTest extends BaseParametrizedTestCase { - - private static final String TIMEOUT_MESSAGE = "Timeout"; - private DsfServicesTracker fServicesTracker; private IGDBControl fGDBCtrl; @@ -127,15 +123,9 @@ public class MIRunControlTargetAvailableTest extends BaseParametrizedTestCase { fRunCtrl.executeWithTargetAvailable(fContainerDmc, steps, rm); } }; - try { + { fRunCtrl.getExecutor().execute(query); query.get(TestsPlugin.massageTimeout(500), TimeUnit.MILLISECONDS); - } catch (InterruptedException e) { - fail(e.getMessage()); - } catch (ExecutionException e) { - fail(e.getCause().getMessage()); - } catch (TimeoutException e) { - fail(TIMEOUT_MESSAGE); } // Now resume the target and check that we stop at the breakpoint. @@ -182,15 +172,9 @@ public class MIRunControlTargetAvailableTest extends BaseParametrizedTestCase { fRunCtrl.executeWithTargetAvailable(fContainerDmc, steps, rm); } }; - try { + { fRunCtrl.getExecutor().execute(query); query.get(TestsPlugin.massageTimeout(500), TimeUnit.MILLISECONDS); - } catch (InterruptedException e) { - fail(e.getMessage()); - } catch (ExecutionException e) { - fail(e.getCause().getMessage()); - } catch (TimeoutException e) { - fail(TIMEOUT_MESSAGE); } // Now check that the target is stopped at the breakpoint. @@ -244,15 +228,9 @@ public class MIRunControlTargetAvailableTest extends BaseParametrizedTestCase { fRunCtrl.executeWithTargetAvailable(fContainerDmc, steps, rm); } }; - try { + { fRunCtrl.getExecutor().execute(query); query.get(TestsPlugin.massageTimeout(500), TimeUnit.MILLISECONDS); - } catch (InterruptedException e) { - fail(e.getMessage()); - } catch (ExecutionException e) { - fail(e.getCause().getMessage()); - } catch (TimeoutException e) { - fail(TIMEOUT_MESSAGE); } // Now resume the target three times and check that we stop three times. @@ -318,13 +296,9 @@ public class MIRunControlTargetAvailableTest extends BaseParametrizedTestCase { try { fRunCtrl.getExecutor().execute(query); query.get(TestsPlugin.massageTimeout(500), TimeUnit.MILLISECONDS); - } catch (InterruptedException e) { - fail(e.getMessage()); } catch (ExecutionException e) { // We want to detect the error, so this is success return; - } catch (TimeoutException e) { - fail(TIMEOUT_MESSAGE); } fail("Did not detect the error of the step"); @@ -381,15 +355,9 @@ public class MIRunControlTargetAvailableTest extends BaseParametrizedTestCase { fRunCtrl.executeWithTargetAvailable(fContainerDmc, steps, rm); } }; - try { + { fRunCtrl.getExecutor().execute(query); query.get(TestsPlugin.massageTimeout(500), TimeUnit.MILLISECONDS); - } catch (InterruptedException e) { - fail(e.getMessage()); - } catch (ExecutionException e) { - fail(e.getCause().getMessage()); - } catch (TimeoutException e) { - fail(TIMEOUT_MESSAGE); } // Now resume the target three times and check that we stop three times. @@ -463,12 +431,8 @@ public class MIRunControlTargetAvailableTest extends BaseParametrizedTestCase { try { fRunCtrl.getExecutor().execute(query); query.get(TestsPlugin.massageTimeout(500), TimeUnit.MILLISECONDS); - } catch (InterruptedException e) { - fail(e.getMessage()); } catch (ExecutionException e) { caughtError = true; - } catch (TimeoutException e) { - fail(TIMEOUT_MESSAGE); } Assert.assertTrue("Did not catch the error of the step", caughtError); @@ -524,15 +488,9 @@ public class MIRunControlTargetAvailableTest extends BaseParametrizedTestCase { crm.setDoneCount(index); } }; - try { + { fRunCtrl.getExecutor().execute(query); query.get(TestsPlugin.massageTimeout(500), TimeUnit.MILLISECONDS); - } catch (InterruptedException e) { - fail(e.getMessage()); - } catch (ExecutionException e) { - fail(e.getCause().getMessage()); - } catch (TimeoutException e) { - fail(TIMEOUT_MESSAGE); } for (int i=0; i drm); }; - private V runAsyncCall(final AsyncRunnable runnable) { + private V runAsyncCall(final AsyncRunnable runnable) throws Exception { return runAsyncCall(runnable, false); } - private V runAsyncCall(final AsyncRunnable runnable, boolean expectExecutionException) { + private V runAsyncCall(final AsyncRunnable runnable, boolean expectExecutionException) throws Exception { Query query = new Query() { @Override protected void execute(DataRequestMonitor rm) { @@ -126,15 +125,11 @@ public class GDBMultiNonStopRunControlTest extends BaseParametrizedTestCase { try { fMultiRun.getExecutor().execute(query); result = query.get(TestsPlugin.massageTimeout(500), TimeUnit.MILLISECONDS); - } catch (InterruptedException e) { - fail(e.getMessage()); } catch (ExecutionException e) { if (expectExecutionException) { return null; } - fail(e.getCause().getMessage()); - } catch (TimeoutException e) { - fail(e.getMessage()); + throw new AssertionError(e); } if (expectExecutionException) {