river-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Patricia Shanahan <p...@acm.org>
Subject Re: Test anomaly
Date Fri, 15 Apr 2011 22:46:08 GMT
Chris,

I changed the config file for the specific test to get some events. It
failed due to my exception. It turns out that LookupDiscoveryManager has
two notifyListener methods, one for notifying all listeners, the other
for notifying a specific listener, presumably one that has just been
added. I've modified the second method based on the patch, and I'll take
a look at the other notifiers to see if there are any similar situations.

I was not trying to test the second method, because I didn't even know
it existed. It was a result of my fumbling with the test. This confirms
my feeling that testing is worth doing, and pushes me to a new strategy:

1. Make sure all listener-related tests really are getting events,
unless the test comments indicate against it.

2. Find all the places where the tests add listeners, and throw in a
BadListener as well.

That should maximize the chances of finding any other notifyListener
type methods that have not been hardened.

Patricia


On 4/15/2011 11:45 AM, Patricia Shanahan wrote:
> Thanks for looking at this. I agree with you about the impenetrability
> of the test code - that is why I'm working by modifying existing tests
> rather than just writing the tests I need from scratch.
>
> If this becomes the limiting factor on a release, I'll check in the
> patch without a QA test.
>
> However, I am strongly in favor of QA tests for the sake of regression
> testing. I want Hudson to tell us if some future change brings back the
> vulnerability to listener exceptions.
>
> Patricia
>
>
> On 4/15/2011 11:21 AM, Christopher Dolan wrote:
>> Patricia,
>>
>> I looked at the existing AddNewDiscoveryListener and agree with your
>> assessment. But the test code is nigh impenetrable, I have to say. It
>> would be beneficial in the long run if some of the common code in
>> LookupDiscovery and LookupLocatorDiscovery were broken apart into
>> separately testable units... I tried writing my own test case, but it
>> doesn't seem possible to mock any of the providers, so to test it you
>> need an actual registrar, it appears.
>>
>> For what it's worth, today I wrote my own private test that runs in my
>> djinn and verified the patch.
>>
>> Chris
>>
>>
>> -----Original Message-----
>> From: Patricia Shanahan [mailto:pats@acm.org]
>> Sent: Thursday, April 14, 2011 7:41 PM
>> To: river-dev@incubator.apache.org
>> Subject: Test anomaly
>>
>> I'm attempting to construct QA tests for the RIVER-395 by copying
>> existing tests of event delivery and adding a special listener that
>> throws exceptions. If the patch is correct, the modified test should
>> pass with all expected events going to the existing listeners despite
>> the exceptions.
>>
>> I'm running my tests with 'com.sun.jini.qa.harness.level = FINEST'
>> logging, so that I can assure myself that the exceptions are being
>> thrown and events delivered after an exception.
>>
>> I tried to base a test on
>> com/sun/jini/test/spec/discoverymanager/AddNewDiscoveryListener.td, and
>> got no events.
>>
>> I've since run the complete discoverymanager category. From the logs, I
>> got no change events, and only 4 tests appeared to have non-zero numbers
>>
>> of initial events.
>>
>> Could someone else take a look at this and see if this is as expected?
>> It looked to me from the comments in AddNewDiscoveryListener.java as
>> though processing a non-zero number of events is important to the test.
>>
>> Patricia
>>
>


Mime
View raw message