river-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Patricia Shanahan <p...@acm.org>
Subject Re: Request for testing help
Date Wed, 29 Sep 2010 19:33:16 GMT
Jonathan Costers wrote:
> The complete current QA test suite passed with this change included.
> Also, I ran the "txnmanager" category, all tests pass except GetStateTest.td
> (which hangs), but this one was already behaving that way without this fix.
> 
> I'd say commit it.

I agree. Could you commit it for me? I don't have access to the computer 
that I have set up for River work.

> 
> PS: if anyone would be able to debug GetStateTest and figure out what is
> wrong with it, we could add another 40-something tests (covering mahalo impl
> and the transactionmanager spec) to our QA run on Hudson (txnmanager
> category).

If it is not fixed by the time I get back to River work in a couple of 
weeks, I'll take a look at it.

I'm glad to see that there are transactionmanager spec tests that are 
not being run. The failure that the patch fixes is so easy to test that 
I was troubled by it not having been detected before. Maybe there is a 
test that would report it that is not being run.

> 
> 2010/9/27 Jonathan Costers <jonathan.costers@googlemail.com>
> 
>> 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 <tvhobbs@googlemail.com>
>>
>> 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 <pats@acm.org> 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<pats@acm.org>
>>>>>
>>>>>  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
>>>>>>
>>>>>>
>>
> 


Mime
View raw message