On 26/06/2009, Filip Hanik - Dev Lists <devli...@hanik.com> wrote:
> sebb wrote:
>
> > On 26/06/2009, Filip Hanik - Dev Lists <devli...@hanik.com> wrote:
> >
> >
> > > sebb wrote:
> > >
> > >
> > >
> > > > On 24/06/2009, Filip Hanik - Dev Lists <devli...@hanik.com> wrote:
> > > >
> > > >
> > > >
> > > >
> > > > > Cleaned up and fixed.
> > > > >
> > > > >  The release is located here:
> > > > >  http://people.apache.org/~fhanik/jdbc-pool/v1.0.5/
> > > > >
> > > > >
> > > > >
> > > > >
> > > > Exactly the same path names were used previously; I assume you are
> > > > referring to the following versions of the files:
> > > >
> > > >
> > > >
> > > >
> > >
> apache-tomcat-jdbc-1.0.5-bin.tar.gz.md5:808cf400c4f7f4de7294b844c68108fa
> > >
> apache-tomcat-jdbc-1.0.5-bin.zip.md5:3f20849d6b0dbe29bb9707cd519c456c
> > >
> apache-tomcat-jdbc-1.0.5-src.tar.gz.md5:6a63d1e77c47c5d6385cf680dac4514c
> > >
> apache-tomcat-jdbc-1.0.5-src.zip.md5:7b4870d50e498a18014031589b8a88eb
> > >
> > >
> > > > rather than the older ones:
> > > >
> > > >
> > > >
> > > >
> > >
> apache-tomcat-jdbc-1.0.5-bin.tar.gz.md5:b6081e6d34a8e9ecd70b505c90e73485
> > >
> apache-tomcat-jdbc-1.0.5-bin.zip.md5:76cb2efd7ce7093d71e4a989e71d2874
> > >
> apache-tomcat-jdbc-1.0.5-src.tar.gz.md5:d8d08870f3479080582d3261a4d1afe5
> > >
> apache-tomcat-jdbc-1.0.5-src.zip.md5:cc6992ff33524f15052f9b72588b628f
> > >
> > >
> > > > ==
> > > >
> > > > The source and test source archives contain META-INF/MANIFEST.MF files
> > > > which don't belong in a source archive.
> > > >
> > > >
> > > >
> > > >
> > >  these are fine.
> > >
> > >
> > >
> > > > The binary archives contain MD5 hashes of all but one of the jars;
> > > > again, these don't belong in the archives.
> > > >
> > > > The jars should contain NOTICE and LICENSE files.
> > > >
> > > >
> > > >
> > > >
> > >  no they should not. I think I've told you before, that NOTICE and
> LICENSE
> > > files are for a release, not for individual files within a release.
> > >
> > >
> > >
> > > > There's no easily accessible documentation on how to build and test
> the
> > > >
> > > >
> > > code.
> > >
> > >
> > > > If someone is familiar with Ant, they can work out what the targets
> > > > do, but the user should not have to do this.
> > > >
> > > > The ant script automatically downloads jars, some of which don't have
> > > > Apache Licenses. In particular, the MySQL licence appears to be GPL,
> > > > which is not compatible with the AL.
> > > >
> > > >
> > > >
> > > >
> > >  yes, I will remove this.
> > >
> > >
> > >
> > > > AFAICT, this is specifically forbidden:
> > > >
> > > >
> > > >
> > > >
> > >
> http://www.apache.org/legal/3party.html#options-build-may2
> > >
> > >
> > > > The "ant test" target generates a few warnings, e.g.
> > > >
> > > > WARNING: Database connection pool evicter thread interval is set to
> > > > lower than 1 second.
> > > >
> > > > Several of the tests fail.
> > > >
> > > >
> > > >
> > > >
> > >  I will remove all tests. It was a bad idea to include to begin with,
> since
> > > they are not part of the release either.
> > >
> > >
> > >
> > > > There's no documentation on what database needs to be set up in order
> > > > to run the database tests so I don't know if these are due to failure
> > > > to set up the database correctly or whether the test failures were
> > > > nothing to do with the database.
> > > >
> > > >
> > > >
> > > >
> > >  that will be solved when the tests go away
> > >
> > >
> >
> > In which case, how can reviewers test the code?
> >
> >
>  you misunderstand "test the code". When you test a car, do you run unit
> test on the car components? No you drive the car on the road.
>  Same thing goes on here.

They are both ways of testing the code.
If the unit tests work for you, but don't work for other reviewers,
then the problem needs to be investigated and fixed.

If a user test finds a problem in the code, then normally the first
thing that needs to be done is to write a unit test case to
demonstrate the error; the error is fixed, and then the users can
demonstrate that the problem has been fixed by running the test code
on as many platforms as possible.

It is unfair to ask every reviewer to generate their own test code if
they don't want to.
One should at least give them the opportunity of testing the code
using the unit tests; if they want to run further tests, so much the
better.

> > That is not the solution either.
> >
> > It should be fairly easy to remove the dependency on MySQL and c3p0 -
> > if not, then IMO the tests are too specific, as Tomcat DBCP should
> > work with any JDBC provider.
> >
> >
>  Actually, I can let the build script download MySQL, or change it to Derby.
> Its not forbidden to download something, as long as the user has to turn on
> a flag to do so.

Yes, the user has to give consent.

>
> > As an experiment, I tried using Derby instead of MySQL, and most of
> > the tests worked. [I'm not yet sure why some tests failed.
> > Unfortunately the output gives no clue to me.]

I have found out why the Ant output is not helpful - there needs to be
the following tag under <junit>:

   <formatter type="brief" usefile="no"/>

> >
> >
>  I think its better to use Derby. yes.
>

That's not what I meant - Derby is just an example I happen to have available.

The unit tests should work with any JDBC database.

I think it's reasonable to assume that the user of the library will
have a database and JDBC jar available. So the build file does not
need to download any database files; there just needs to be a way to
provide the JDBC jar to the test classpath.

There also needs to be a way to define any other database-specific or
installation-specific properties. So far I have found that the
following may vary between DBs:

URL
Classname
user
password
validation query ("SELECT 1" is not valid for Derby; use "VALUES 1" instead)

There then needs to be a short text document explaining how to
configure the database and the test properties.

> > I suggest changing the Ant test classpath to include whatever jars it
> > finds in the include directory, and change the test code to pick up
> > the database settings from build.properties Then all a tester has to
> > do is put their JDBC jar in the directory and set up the database as
> > required.
> >
> >
> >
> > >
> > > > Ideally, the user should be given the option of running the JDBC tests
> > > > against whatever database they prefer. If the tests require tables etc
> > > > to be set up, these should either be done as part of the test setup,
> > > > or there should be clear documentation on what data needs to be set
> > > > up.
> > > >
> > > >
> > > >
> > > >
> > > >
> > > > >  <ballot>
> > > > >  [ ] STABLE - I couldn't find any bugs
> > > > >  [ ] BETA   - I found some bugs but not critical
> > > > >  [ ] BROKEN - I found some show stoppers
> > > > >  </ballot>
> > > > >
> > > > >  Any comments ?
> > > > >
> > > > >  Thanks,
> > > > >  Filip
> > > > >
> > > > >
> > > > >
> > > > >
> > > >
> > >
> ---------------------------------------------------------------------
> > >
> > >
> > > >
> > > > >  To unsubscribe, e-mail:
> > > > >
> > > > >
> > > >
> > > dev-unsubscr...@tomcat.apache.org
> > >
> > >
> > > >
> > > > >  For additional commands, e-mail: dev-h...@tomcat.apache.org
> > > > >
> > > > >
> > > > >
> > > > >
> > > > >
> > > > >
> > > >
> > > >
> > >
> ---------------------------------------------------------------------
> > >
> > >
> > > > To unsubscribe, e-mail:
> dev-unsubscr...@tomcat.apache.org
> > > > For additional commands, e-mail: dev-h...@tomcat.apache.org
> > > >
> > > >
> > > >
> > > >
> > > >
> > > >
> > >
> ---------------------------------------------------------------------
> > >  To unsubscribe, e-mail:
> dev-unsubscr...@tomcat.apache.org
> > >  For additional commands, e-mail: dev-h...@tomcat.apache.org
> > >
> > >
> > >
> > >
> >
> >
> ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> > For additional commands, e-mail: dev-h...@tomcat.apache.org
> >
> >
> >
> >
>
>
> ---------------------------------------------------------------------
>  To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
>  For additional commands, e-mail: dev-h...@tomcat.apache.org
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Reply via email to