Hi David,
A couple responses follow. Cheers-Rick
David W. Van Couvering wrote:
Hi, Rick, this looks great, thanks for this work!
A couple of questions:
- Is there any reason these weren't written as JUnit tests? I suspect
the answer is (a) they were originally written before the JUnit
framework was in place and (b) it was too much work to migrate as part
of this patch.
Most of these tests are, deliberately, canon-less assertion-based tests,
which, as you note, would be natural fits for JUnit. You're right about
why these weren't coded under JUnit: the framework wasn't in place yet.
It would be good to migrate these to JUnit, but that wasn't my itch
here. I just wanted to fix the framework problem so that people could
add new tests that follow our usual conventions.
- I noticed in a couple of places you directly call
System.getProperty("framework"). Shouldn't these be in privileged
blocks? Or is this only an issue with JUnit tests because they
require JUnit to be able to access properties unless you use a
privileged block?
You're right, this will be a problem when these tests are converted to
JUnit.
I'm just curious -- how did you get ij.startJBMS() to work? I got
"null" whenever I tried to do that in the JDBC4 tests, as did others...
Dunno. It just works for me. If you've got a moment, could you
test-drive these changes in your environment to verify I haven't broken
the jdbc4 tests for you? For the record, I'm running with jdk1.6 build 76.
Thanks,
David
