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 Sat, 30 Apr 2011 23:03:15 GMT
Thanks for the explanation. I see what you mean. I'll add comments to 
the methods to clarify the reason for the difference.


On 4/25/2011 2:45 PM, Christopher Dolan wrote:
> Hi Patricia,
> Thanks for investing so much time in this issue.
> In my patch, I intentionally did not modify the second case (single
> listener) because in that scenario, the failure would only hurt the
> caller's thread, where the first case (all listeners) a failure crashed
> the LookupDiscovery/LookupLocatorDiscovery instance's notify thread. I
> thought that the single listener case deserved fail-fast behavior and it
> should be the caller's responsibility to catch their own broken
> listener's exceptions.
> Chris
> P.S. sorry for the delay. I'm fresh back from vacation.
> -----Original Message-----
> From: Patricia Shanahan [mailto:pats@acm.org]
> Sent: Friday, April 15, 2011 5:46 PM
> To: dev@river.apache.org
> Cc: Christopher Dolan
> Subject: Re: Test anomaly
> 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

View raw message