2009/11/10 Martin Ritchie <[email protected]> > 2009/11/10 Robert Godfrey <[email protected]>: > > Hi Aidan, > > > > Why did you remove the setting of the linked exception here: > > > > --- > > > a/qpid/java/client/src/main/java/org/apache/qpid/client/BasicMessageProducer_0_10.java > > +++ > > > b/qpid/java/client/src/main/java/org/apache/qpid/client/BasicMessageProducer_0_10.java > > @@ -180,8 +180,6 @@ public class BasicMessageProducer_0_10 extends > > BasicMessageProducer > > catch (RuntimeException rte) > > { > > JMSException ex = new JMSException("Exception when sending > > message"); > > - rte.printStackTrace(); > > - ex.setLinkedException(rte); > > throw ex; > > } > > } > > > > Also, we probably want to communicate the stack trace somehow in > > > > --- > > > a/qpid/java/client/src/test/java/org/apache/qpid/client/protocol/AMQProtocolHandlerTest.java > > +++ > > > b/qpid/java/client/src/test/java/org/apache/qpid/client/protocol/AMQProtocolHandlerTest.java > > @@ -178,7 +178,6 @@ public class AMQProtocolHandlerTest extends TestCase > > } > > catch (Exception e) > > { > > - e.printStackTrace(); > > fail(e.getMessage()); > > } > > } > > > > although I would think that just not catching the exception here would > > probably do it... > > > > Don't immediately see any problems with the patch otherwise since no > > information should be lost... (i.e. the stack should still print to a log > > somewhere if desired I guess) > > Removing the stackTrace alone here(AMQProtocolHandlerTest.java) is not > the solution. This is a test and as Rob points out the exception needs > to be reported back. I believe I wrote the test I clearly wasn't paing > attention as calling fail() on a different thread will not fail the > test. > > I'd say either leave the stacktrace in on this test .. with a comment > asking anyone seeing this to please raise a JIRA; or ensure the > exception is correctly communicated to the main test thread and test > failure occurs. > > Just my thoughts. > > Martin >
OK - I'm guilty of only looking at the diff and not the context... I just assumed if you could call fail then throwing the exception would be the answer... I do think that causing the test to fail in some way would be the answer... -- Rob
