Kristian Waagan wrote:
So far most of the comments I have received have been regarding
implementation, which was not my primary goal. Do we all agree what we
need, but we want to do it in different ways? Or are there still someone
out there that have more fundamental issues to comment on?
I agree with Andreas: have a base generic class and a base class for JDBC
/**
* Get a connection to some database.
* If the database does not exist, it will be created.
* As long as the state of this class is not changed, this method will
* return a connection to the same database. Most suites will require
that
* this condition is true.
*/
My "as long as state of this class is not changed" was supposed to
indicate that a connection to the same database was returned. "some" was
chosen because a test should not generally care what the database it
uses is named - as long as it is the same database for a given time
(typically while running a single test method or a suite). This is a
matter of wording.
OK. It's just not clear to me what "some database" is, and it took some
work for me to track it down.
- You should document what the defaults are, such as "wombat" for the
database name and "APP" for username and password.
I did not do this, because most tests should not care.
Why do you say that? I'd like to know what's happening by default
before I use the default behavior of an API.
- why can't setup() or teardown() automaticlly call resetState()
rather than asking the user to do it? This seems dangerous and highly
error-prone.
When I wrote the class, I was looking at the JDBC4 test suite. It is
configured to always run under DerbyNetClient, but it "internally" runs
tests for embedded as well. BasicDerbyJUnitTest was written with the
following paradigm in mind: do setup (basically set framework, maybe
change default name and force use of datasource), then use
getConnection() for all tests in suite. The setup would be done in a
TestSetup decorator. With this approach, the test code can be exactly
the same for multiple frameworks and/or setups, where as the test
decorator would do different things for each suite. Calling resetState
in TestCase.setUp() would delete the state setup by
TestDecorator.setUp(). If this way to run tests are not needed, things
can be rewritten (in many ways...).
OK, thanks for clarifying. It would be good to provide this discussion
in the Javadoc so users know why they have to jump through this
particular hoop. It also explains better the intent for this class.
By the way, wouldn't separating configuration out from the base class
make this management of state go away? You can pass in a non-default
configuration object and a non-default JDBCClient if you want to use
non-default behavior.
- I don't understand why you set a property for the database name
before calling obtainConnection() rather than just passing in the
database name to obtainConnection(). It would be good to at least
explain the motivation behind this, as at first read it seems a bit
odd. Scanning at the code, it looks like you use db(P_DBNAME) in
createBaseDatabaseUrl() where you could just as easily pass the
database name in as a parameter
All properties can be hardcoded and represented by constants instead of
using a Properties object. I just wanted to signal that results are
based on the state of the class, and that P_* are not set by environment
properties, but that S_* are (passed on from the test harness). Andreas'
TestConfiguration would separate this information from the class, which
is good. I do think it is a bit too rigid as it is to give the required
flexibility (not possible to change anything), but this is of course
easily changed. It also give me the impression that every configurable
aspect is passed on from the test harness, which is not yet true.
Again, I think this whole thing goes away if you use the
TestConfiguratio model. Right?
- You have a lot of useful debug statements, but your javadoc doesn't
really explain how to turn debug on and off, nor is it clear from the
class (e.g. there is not "debug(boolean)" method).
Would mentioning this in the public class documentation do the trick?
Yes, that would help.
- It would be nice to have a helper method that allows subclasses to
quickly obtain other properties set by the harness (e.g. so they don't
have to do a privileged block every time they want to get a system
property)
Are you thinking of a getSystemProperty(key) with a privileged block, or
a method accessing a "cached" system properties object?
The former if system properties are not read-only, otherwise the latter
is fine with me. Since accessing system properties directly requires
jumping through security hoops, I thought it would be good to isolate it.
Also, it would be very good to abstract out configuration implementation
details so that if we change the way tests are configured all the tests
don't have to change. Another motivation for the TestConfiguration class...
- I like Andreas' typesafe JDBCClient class, it seems a better approach.
Yes. It should be extended to have information about DataSource classes
also, or else it would not be sufficent in a JSR 169 (like) environment.
Some complications here if we want to allow for more than one type of
DataSource for a given framework (XA, Pooled, Simple, "normal").
OK
begin:vcard
fn:David W Van Couvering
n:Van Couvering;David W
org:Sun Microsystems, Inc.;Database Technology Group
email;internet:[EMAIL PROTECTED]
title:Senior Staff Software Engineer
tel;work:510-550-6819
tel;cell:510-684-7281
x-mozilla-html:TRUE
version:2.1
end:vcard
