Nice to see someone else committing to FileUpload! :-)

A few comments on the recent checkins:

1) Perhaps I'm jumping the gun here, but FileUploadTest needs to come back as DiskFileUploadTest. Even though that class is now deprecated, we still need to be able to test it for the next release, prior to removing it in the release beyond that.

2) We need to keep the tests in the same package as the classes they are testing. So, for example, the new ServletFileUploadTest should be pushed down into org.apache.fileupload.servlet. (This is something I hadn't got around to sorting out yet myself.)

3) ServletFileUploadTest appears to be testing DiskFileUpload instead of, um, well, ServletFileUpload. ;-)

4) Rather than removing the constructor from ServletFileUploadTest, shouldn't there still be one that takes a string parameter? I thought JUnit was happier (or provided more helpful messages) with the constructor there (but I could be wrong).

5) Is there some package out there in the world that we can simply depend on for mock objects, rather than having to have our own? I wasn't happy with the one just removed either, but I was hoping to find something else we could just use.

6) FileUpload is now using the Sun coding conventions, and no longer uses the Turbine ones. While I haven't yet made a pass through the tests to make this change, just a heads up that new code should follow the Sun conventions.

7) Most important of all: Thanks!

--
Martin Cooper

---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]



Reply via email to