Hi Rick,
High level comments concerning your patch for DERBY-516:
First, the requirements for the full set of compatibilty tests to run
are quite extensive. Perhaps a little more extensive than a sort-of-
casual derby user/developer might expect when trying to run a set of
tests. It is not sufficient to download the source tree and the
dependencies listed in building.txt. It requires that you have JDK
1.3, 1.4, 1.5 for your platform, and Derby jars from the 10.0 and
10.1 branches available in specific locations. I think additional
information should be added to the testing README.htm in java/testing
to include the full requirements for the compatibility test as well
as a link to the README.htm down in java/testing/.../compatibility.
At any rate, I think an effort should be made - for now - to make the
compilation and execution of these tests optional, or at the very
least, fail gracefully with informative error messages. I still
haven't been able to get just jdbcapi/CompatibilityTest to run, and
the current build/test setup doesn't give me much information to help
me figure out why.
That said:
1 - For junit.jar - the instructions in BUILDING.txt are not clear as
to what you are supposed to do with junit.jar. "Unzip it somewhere"
is not nearly specific enough. The instructions should say what file
should be put into what location. This location should be added to
extrapath.properties and that location should be referenced in any
build files that require it, as is currently the case with osgi.jar,
for example. That location should probably be tools/java since that's
where we've been putting other dependencies.
In general, compilepath.properties is reserved for JDK-related
classes, jars containing java.*, javax.*, or direct dependencies. The
new property for junit.jar should probably be called ${junit}, added
to extrapath.properties, and the build.xml in java/testing/.../
functionTests/compatibility/build.xml should include ${junit} in the
appropriate classpath entries for the applicable <javac> tasks. Same
for java/testing/.../functionTests/util/build.xml and java/
testing/.../jdbcapi/build.xml
But I really think that new build targets that are conditional on $
{junit} being <available> should be made in the above build files, in
case someone wanted to compile the non-Junit tests but didn't have
Junit - for now.
2 - empty diff in java/build/org/apache/derbyBuild/build.xml
3 - the reference to junit-3.8.1.jar in java/testing/Readme.htm
should be made more generic, and reference the convention $jardir
which is already used in that file. '/local/derby/tools/java/
junit-3.8.1.jar' not only presupposes that the user will have put
junit.jar in tools/java (which is ok, we should probably instruct the
user to put junit.jar into tools/java and reference it in
extrapath.properties from there), but into a particular path in their
filesystem -- and that particular path does not apply to all OSs.
4 - Junit noise(?) stripped in Sed.java. Is it noise? Wouldn't the
"OK..." output be useful to narrow down testcase failure?
5 - if you are going to add jdbcapi/CompatibilityTest to
derbynetmats.runall, then you should have a way for the current
harness to gracefully fail / skip the test if all the requirements of
CompatibilityTest are not met. As it is, it couldn't get jdbcapi/
CompatibilityTest to run, but it's likely that I missed something in
my setup. At any rate, I can help with sorting out any issues with
regards to getting the test to run as a part of the current test
harness.
6 - Weird diffs in build.xml (top-level) and java/client/org/apache/
derby/client/net/Reply.java
7 - The default values in testScript.xml for the locations of the
JVMs are not applicable to Mac OS X (an itchy platform for me). I'll
follow up on this later.
Since I haven't been able to get the tests to run yet, that's all I
have for now, but I'll keep at it. :-)
andrew
