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