This fix actually made the test pass for me for the first time. I'm running the complete suite against it (all the categories that are executed on Hudson) now.
2010/9/27 Tom Hobbs <[email protected]> > Good find! > > > There is an issue of whether there is anything else that depends on > getting a RuntimeException rather than a > > checked exception, so it needs a full regression test. > > If other tests rely on the incorrect behaviour of TxnManagerImpl then I'd > say that the other tests are wrong by definition. If this change breaks > other tests then those other tests need appropriate fixes. I'd check in > the > fix now, if the build starts breaking in new and exciting places then the > cause of those breakages should be easy to diagnose. > > > On Mon, Sep 27, 2010 at 11:18 AM, Patricia Shanahan <[email protected]> wrote: > > > I've diagnosed com/sun/jini/test/impl/mahalo/RandomStressTest.td, but may > > not have time to regression test and check in the fix. > > > > The test failure is due to an error in > > com/sun/jini/mahalo/TxnManagerImpl.java. It throws a NullPointerException > > from abort if the transaction is not known to transaction management. > That > > case can arise, even for a formerly valid transaction, due to it having > > already been committed or aborted, so it is timing dependent and more > likely > > to happen under stress than when doing more controlled tests. > > > > The TransactionManager interface specifies UnknownTransactionException > for > > that case. The test catches UnknownTransactionException and treats it as > a > > valid, non-error result. NullPointerException is not caught until it gets > > back to the TaskManager's thread run method, which turns it into a logged > > WARNING. The uncaught exception prevents the task from reporting > completion > > back to the test, leading to the timeout and test failure. > > > > There is an issue of whether there is anything else that depends on > getting > > a RuntimeException rather than a checked exception, so it needs a full > > regression test. > > > > I also recommend reducing the timeout from 30 minutes. When the test > > succeeds, it takes less than a minute. > > > > Here is a patch to change the exception: > > > > Index: src/com/sun/jini/mahalo/TxnManagerImpl.java > > =================================================================== > > --- src/com/sun/jini/mahalo/TxnManagerImpl.java (revision 1000286) > > +++ src/com/sun/jini/mahalo/TxnManagerImpl.java (working copy) > > @@ -933,7 +933,7 @@ > > } > > > > } else { > > - throw new NullPointerException("No such transaction > ["+id+"]"); > > + throw new UnknownTransactionException("No such transaction > > ["+id+"]"); > > > > } > > > > > > Patricia > > > > > > On 9/25/2010 7:17 AM, Jonathan Costers wrote: > > > >> I actually did come across a test that may be interesting to run for > these > >> changes ... > >> > >> com/sun/jini/test/impl/mahalo/RandomStressTest.td > >> > >> I have not been able to get this test to pass on any of my machines or > VMs > >> ... > >> It seems to make heavy use of TaskManager and RetryTask. > >> It can take a while to finish as well, so beware. > >> > >> You think this may be a good one? > >> > >> you can run it if you change to the<river>/qa directory and execute: > >> ant -Drun.tests=com/sun/jini/test/impl/mahalo/RandomStressTest.td > >> run-tests > >> > >> or similarly in an IDE, set property > >> run.tests=com/sun/jini/test/impl/mahalo/RandomStressTest.td > >> and run the run-tests target > >> > >> make sure everything is built first, of course. > >> > >> 2010/9/21 Patricia Shanahan<[email protected]> > >> > >> I'm testing my new TaskManager the , but I have some anomalies. It > would > >>> help me to get some more testing of > >>> > >>> > https://svn.apache.org/repos/asf/incubator/river/jtsk/skunk/patsTaskManagerdoneinother > WindowsXP environments. > >>> > >>> Both the head revision and revision 998737 need to be tested. Revision > >>> 998737 is the one I plan to merge into the trunk. It changes the > >>> interface > >>> between TaskManager and its callers, with minimal changes to > TaskManager. > >>> > >>> It is important that it be tested widely, because it affects a lot of > >>> critical classes, and would be difficult to back out. > >>> > >>> The head revision drops in a revised TaskManager. It should be easy to > >>> back > >>> out if necessary. > >>> > >>> Patricia > >>> > >>> > >> > > >
