You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by bu...@apache.org on 2009/04/30 09:06:42 UTC
DO NOT REPLY [Bug 47124] New: Run the unit tests as part of the
build!!!
https://issues.apache.org/bugzilla/show_bug.cgi?id=47124
Summary: Run the unit tests as part of the build!!!
Product: Tomcat 6
Version: unspecified
Platform: PC
OS/Version: Linux
Status: NEW
Severity: normal
Priority: P2
Component: Catalina
AssignedTo: dev@tomcat.apache.org
ReportedBy: mguillemot@yahoo.fr
Created an attachment (id=23565)
--> (https://issues.apache.org/bugzilla/attachment.cgi?id=23565)
Patch running the unit tests as part of the build
The lack of unit tests is really a shame for a project like Tomcat! A few unit
tests are available in the source code but first they are not run as part of
the build process and second many of them fail.
I've attached a patch to this issue that:
- runs the existing unit tests as integral part of the build
- renames some of the unit tests to have a consistent naming pattern for the
unit tests (ie XXXXXTest)
- makes a minimal cleanup
The consequence is that the build fails with this patch as 11 of the 51 unit
tests fail. My current knowledge of Tomcat doesn't allow me to decide if the
failing tests should be simply removed or adapted. With feedback on this, I can
provide a modified version of the patch.
--
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
DO NOT REPLY [Bug 47124] Run the unit tests as part of the build!!!
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=47124
--- Comment #2 from Marc Guillemot <mg...@yahoo.fr> 2009-05-04 05:07:33 PST ---
I'm glad to see that you've committed some changes, nevertheless for me it
doesn't really change the situation that unit tests are not considered
seriously in Tomcat project.
Tests *should* be integral part of the build. It's normal that a build takes
(far) longer than 5 minutes if it runs tests (nevertheless these tests should
have some sense to be worth this time, what is perhaps not yet the case).
I'm sorry if I didn't followed the convention for external libs. I thought that
it was the case. What about using Ivy rather than this self made "strategy"?
This is is far less important that running the tests as part of the build, but
I think that the tests should be renamed for consistency and as it just allows
to use name patterns to select the tests (currently there is no convention
here). In the current state of the test suite, I would prefer to select the
tests by name pattern rather than having a TestAll as it doesn't have any
advantage and this file needs to be modified each time a test is added (with
the risk to have tests that aren't run).
I don't understand either why you don't use Ant's junit task.
At the end I have the feeling that this issue has been closed as WONTFIX rather
than as FIXED :-(
--
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
DO NOT REPLY [Bug 47124] Run the unit tests as part of the build!!!
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=47124
Mark Thomas <ma...@apache.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|RESOLVED |REOPENED
Resolution|FIXED |
Severity|normal |enhancement
--- Comment #5 from Mark Thomas <ma...@apache.org> 2009-05-05 05:24:24 PST ---
(In reply to comment #4)
> Isn't it even more efficient if one committer (you?) just rename the files and
> commit the changes?
Depends how you measure efficiency. It would be less work for me just to review
a set of proposed changes.
> this was in the proposed patch. Should I upload an other one?
There was a lot to the patch. Separating out the junit task (and checking that
it still applies to the latest trunk) would be useful.
Re-opening so this doesn't get forgotten.
--
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
DO NOT REPLY [Bug 47124] Run the unit tests as part of the build!!!
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=47124
Mark Thomas <ma...@apache.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |RESOLVED
Resolution| |FIXED
--- Comment #1 from Mark Thomas <ma...@apache.org> 2009-05-02 16:20:16 PST ---
I am -1 for this patch as is on the following grounds:
- The tests take far too long to run by default with every build. This patch
increases the rebuild time from 1s to over 5min.
- The patch doesn't follow the Tomcat convention for defining external libs
I'm also very close to -1 on the renaming. I don't think it adds that much.
Given this, what I have done is modified the current test/build.xml in trunk to
run all the tests. There are currently see a number of errors/failures - mostly
in Tribes. How many of those are genuine and how many are faulty tests is TBD.
Given the failures and the time taken, the Tribes tests are currently commented
out.
The remaining failures are caused by the use of ServerFactory that doesn't play
nicely with o.a.c.startup.Tomcat when multiple Tomcat instance are created.
As noted in the code ServerFactory needs to be removed.
Several resolutions are applicable here, I went for FIXED.
--
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
DO NOT REPLY [Bug 47124] Run the unit tests as part of the build!!!
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=47124
Mark Thomas <ma...@apache.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|REOPENED |RESOLVED
Resolution| |FIXED
--- Comment #7 from Mark Thomas <ma...@apache.org> 2009-06-15 12:42:42 PST ---
I found some time to work on this.
Test cases have been renamed and a test task added to build. It will remain
optional for now but could be made mandatory in the future if that is deemed
useful.
It hasn't been proposed for backport to 6.0.x, mainly as the one class that
will hopefully be really helpful when building tests cases
(o.a.c.startup.Tomcat) isn't available and would need a number amount of other
patches to be applied before it could be used.
We can always revisit the back-porting decision at a later date.
--
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
DO NOT REPLY [Bug 47124] Run the unit tests as part of the build!!!
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=47124
--- Comment #6 from Marc Guillemot <mg...@yahoo.fr> 2009-05-05 06:12:03 PST ---
(In reply to comment #5)
> (In reply to comment #4)
> > Isn't it even more efficient if one committer (you?) just rename the files and
> > commit the changes?
>
> Depends how you measure efficiency. It would be less work for me just to review
> a set of proposed changes.
if you know an easy way to generate the set of SVN commands, I'm interested.
Otherwise. due to the very small number of files impacted, it's better if you
do it directly. I've already provided a patch for that but you haven't applied
it.
> > this was in the proposed patch. Should I upload an other one?
>
> There was a lot to the patch. Separating out the junit task (and checking that
> it still applies to the latest trunk) would be useful.
>
> Re-opening so this doesn't get forgotten.
ok, I'll submit an other one just with this features
--
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
DO NOT REPLY [Bug 47124] Run the unit tests as part of the build!!!
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=47124
--- Comment #3 from Mark Thomas <ma...@apache.org> 2009-05-04 07:58:53 PST ---
(In reply to comment #2)
> Tests *should* be integral part of the build. It's normal that a build takes
> (far) longer than 5 minutes if it runs tests (nevertheless these tests should
> have some sense to be worth this time, what is perhaps not yet the case).
Running the tests as part of the Gump build would make sense. Changing the
build we use all the time in development from 1s to >5min does not.
> I'm sorry if I didn't followed the convention for external libs. I thought that
> it was the case. What about using Ivy rather than this self made "strategy"?
Changing this is a separate question for the dev list. As with any change,
there would need to sufficient benefit to justify it.
> This is is far less important that running the tests as part of the build, but
> I think that the tests should be renamed for consistency and as it just allows
> to use name patterns to select the tests (currently there is no convention
> here). In the current state of the test suite, I would prefer to select the
> tests by name pattern rather than having a TestAll as it doesn't have any
> advantage and this file needs to be modified each time a test is added (with
> the risk to have tests that aren't run).
That is a reasonable argument. Patches tend to get very large when renames are
concerned. A set of svn commands to run would be easier to review and to apply.
> I don't understand either why you don't use Ant's junit task.
No idea. Merging the test.xml with the main build.xml and using Ant's junit
task would make sense.
--
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
DO NOT REPLY [Bug 47124] Run the unit tests as part of the build!!!
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=47124
--- Comment #4 from Marc Guillemot <mg...@yahoo.fr> 2009-05-05 03:37:21 PST ---
(In reply to comment #3)
>...
> > This is is far less important that running the tests as part of the build, but
> > I think that the tests should be renamed for consistency and as it just allows
> > to use name patterns to select the tests (currently there is no convention
> > here). In the current state of the test suite, I would prefer to select the
> > tests by name pattern rather than having a TestAll as it doesn't have any
> > advantage and this file needs to be modified each time a test is added (with
> > the risk to have tests that aren't run).
> That is a reasonable argument. Patches tend to get very large when renames are
> concerned. A set of svn commands to run would be easier to review and to apply.
Isn't it even more efficient if one committer (you?) just rename the files and
commit the changes?
>
> > I don't understand either why you don't use Ant's junit task.
> No idea. Merging the test.xml with the main build.xml and using Ant's junit
> task would make sense.
this was in the proposed patch. Should I upload an other one?
--
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org