This is the mail archive of the
mauve-patches@sourceware.org
mailing list for the Mauve project.
Re: RFC: JDWP filter tests
- From: Keith Seitz <keiths at redhat dot com>
- To: Kyle Galloway <kgallowa at redhat dot com>
- Cc: mauve-patches <mauve-patches at sources dot redhat dot com>
- Date: Fri, 09 Jun 2006 14:02:41 -0700
- Subject: Re: RFC: JDWP filter tests
- References: <44898BC3.9050704@redhat.com> <44898E21.2010600@redhat.com>
Kyle Galloway wrote:
Index: gnu/testlet/gnu/classpath/jdwp/events/filters/TestOfClassExcludeFilter.java
===================================================================
RCS file: gnu/testlet/gnu/classpath/jdwp/events/filters/TestOfClassExcludeFilter.java
diff -N gnu/testlet/gnu/classpath/jdwp/events/filters/TestOfClassExcludeFilter.java
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ gnu/testlet/gnu/classpath/jdwp/events/filters/TestOfClassExcludeFilter.java 1 Jan 1970 00:00:00 -0000
@@ -0,0 +1,97 @@
I only have one comment on this (and it applies to the others, too,
probably): Are there any inputs to ClassMatchFilter which are simply
illegal?
For example, the spec describes the pattern as "Disallowed class
pattern. Matches are limited to exact matches of the given class pattern
and matches of patterns that begin or end with '*'; for example, "*.Foo"
or "java.*"." One of the errors that is listed at the end of the
description of Event.Set is "INVALID_STRING".
Perhaps that is bit overboard, though. I don't really know.
You can't really do much with this for many filters, e.g.,
ClassOnlyFilter, because of they require valid IDs and the like, and
they are checked by PacketProcessor and *CommandSet before creating the
filter.
Of course, this begs the question: Should this check be done by the
filter or the command set processor that is creating the filter? Alas,
that's a different conversation for a different list. [The most prudent
answer to this is that we should always test for invalid input. If that
check is done in the code in the filter, then it should be tested in the
filter. If the check is done somewhere else, it should be tested there,
too. But regardless of where the check and test are located, they should
both be done.]
Index: gnu/testlet/gnu/classpath/jdwp/events/filters/TestOfClassOnlyFilter.java
===================================================================
RCS file: gnu/testlet/gnu/classpath/jdwp/events/filters/TestOfClassOnlyFilter.java
diff -N gnu/testlet/gnu/classpath/jdwp/events/filters/TestOfClassOnlyFilter.java
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ gnu/testlet/gnu/classpath/jdwp/events/filters/TestOfClassOnlyFilter.java 1 Jan 1970 00:00:00 -0000
@@ -0,0 +1,105 @@
[snip]
+ public void testConstructor(TestHarness harness)
+ {
+ harness.checkPoint("Constructor");
+ for (int i = 0; i < 3; i++)
+ {
+ ReferenceTypeId testId = new ClassReferenceTypeId();
+ SoftReference testSR = new SoftReference(Integer.class);
+ testId.setReference(testSR);
+
+ try
+ {
+ ClassOnlyFilter testCOF = new ClassOnlyFilter(testId);
+ }
+ catch (InvalidClassException ex)
+ {
+ harness.check(false, "Constructor failed with exception" + ex);
+ }
+ }
+ }
I think I mentioned this before, but you could simply use VMIdManager to
get Object and ReferenceType IDs. No big deal, though, if you would
rather avoid that.
What about testing the case where the constructor *should* fail, i.e.,
don't set the reference for the ID. [aside from the whole "should it be
checked here somewhere else" thing again]
Index: gnu/testlet/gnu/classpath/jdwp/events/filters/TestOfCountFilter.java
===================================================================
RCS file: gnu/testlet/gnu/classpath/jdwp/events/filters/TestOfCountFilter.java
diff -N gnu/testlet/gnu/classpath/jdwp/events/filters/TestOfCountFilter.java
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ gnu/testlet/gnu/classpath/jdwp/events/filters/TestOfCountFilter.java 1 Jan 1970 00:00:00 -0000
[snip]
+ try
+ {
+ CountFilter testCF = new CountFilter(COUNT);
+ for (int i = 0; i < COUNT; i++)
+ {
+ matched = testCF.matches(new ClassPrepareEvent(Thread.currentThread(),
+ Integer.class, 0));
+ if(i < COUNT-1)
+ harness.check(!matched, "Matches returns false for < count");
+ else
+ harness.check(matched, "Matches returns true after count # of"
+ + "events");
+ }
+ }
+ catch(InvalidCountException ex)
+ {
+ harness.check(false, "Constructor failed for valid count");
+ }
+ }
I suggest looping over something like "COUNT + 1" to check what happens
when the count has been exceeded. It should *only* match once. [Missing
a space in the string "Matches returns true after count # ofevents".]
You could recycle the event instead of creating a new one every time.
Probably doesn't matter, but it would be trivial enough in this case to
keep that overhead out of the testsuite.
Index: gnu/testlet/gnu/classpath/jdwp/events/filters/TestOfInstanceOnlyFilter.java
===================================================================
RCS file: gnu/testlet/gnu/classpath/jdwp/events/filters/TestOfInstanceOnlyFilter.java
diff -N gnu/testlet/gnu/classpath/jdwp/events/filters/TestOfInstanceOnlyFilter.java
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ gnu/testlet/gnu/classpath/jdwp/events/filters/TestOfInstanceOnlyFilter.java 1 Jan 1970 00:00:00 -0000
[snip]
+ try
+ {
+ InstanceOnlyFilter testIOF = new InstanceOnlyFilter(testOID);
+ harness.check(true, "Constructor successful");
+ }
+ catch (InvalidObjectException ex)
+ {
+ harness.check(false, "constructor failed for valid ObjectId");
+ }
+
+ }
Another good place to try passing something invalid to the constructor
and test whether it fails. Also, for this one, "null" should be tested
as a constructor argument, since it is valid.
+ try
+ {
+ InstanceOnlyFilter testIOF = new InstanceOnlyFilter(testOID);
+ BreakpointEvent testEvent = new BreakpointEvent(Thread.currentThread(),
+ null, testInt);
+ harness.check(testIOF.matches(testEvent), "testing Instance match");
+ }
+ catch (InvalidObjectException ex)
+ {
+ harness.check(false, "constructor failed for valid ObjectId");
+ }
+ }
Missing a test where this should fail.
Hmm. What about things to test object equality? Maybe add a test that
checks against a clone of the object? I think for InstanceOnly, we need
obj1 == obj2, not obj1.equals(obj2).
Keith