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:[email protected]]
Sent: Thursday, April 14, 2011 7:41 PM
To: [email protected]
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
