You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ant.apache.org by Stefan Bodewig <bo...@apache.org> on 2008/02/15 11:48:07 UTC
The Cactus/JUnit-Task problem
Hi all,
[Petar, it would be good if you subscribed to dev@ant, it is not that
high traffic anyway]
last night I chatted with Petar about the backwards incompatible
change to the JUnit task we introduced in Ant 1.7.0 that broke Cactus.
Cactus' Ant task extends the JUnit task (and if memory serves me right
is one of the reasons that a bunch of methods in JUnitTask have
protected access in the first place). It used to override execute()
completely and invoke the execute variants that acceps tests or lists
of tests (I don't recall which).
This doesn't work any longer since execute() performs setup work on
the delegate that decouples Ant from the junit library and the execute
variants rely on this setup.
On my way home I thought that maybe the easiest solution would be to
have the execute variants check whether the setup has been performed
and if not - simply do it themselves. The appended patch does just
that and I'd like to get some feedback.
The patch would make deleteClassloader protected so that subclasses
can cleanup themselves, this may not strictly be necessary.
With this patch in place, Cactus should be able to use Ant without any
modifications, but could benefit from more control over resource
cleanup if it wants to.
BTW, I'd love to merge whatever solution we agree on to the 1.7 branch
and have it go into 1.7.1. Right now Cactus users are forced to stick
to 1.6.5 and we should change that.
Of course Petar should make sure that Gump can build Cactus so that he
can hit us if we break it again. 8-)
Stefan
Index: src/main/org/apache/tools/ant/taskdefs/optional/junit/JUnitTask.java
===================================================================
--- src/main/org/apache/tools/ant/taskdefs/optional/junit/JUnitTask.java (revision 627950)
+++ src/main/org/apache/tools/ant/taskdefs/optional/junit/JUnitTask.java (working copy)
@@ -162,6 +162,7 @@
private boolean splitJunit = false;
private JUnitTaskMirror delegate;
+ private ClassLoader mirrorLoader;
/** A boolean on whether to get the forked path for ant classes */
private boolean forkedPathChecked = false;
@@ -746,14 +747,10 @@
}
/**
- * Runs the testcase.
- *
- * @throws BuildException in case of test failures or errors
- * @since Ant 1.2
+ * Sets up the delegate that will actually run the tests.
*/
- public void execute() throws BuildException {
+ protected void setupJUnitDelegate() {
ClassLoader myLoader = JUnitTask.class.getClassLoader();
- ClassLoader mirrorLoader;
if (splitJunit) {
Path path = new Path(getProject());
path.add(antRuntimeClasses);
@@ -766,7 +763,17 @@
mirrorLoader = myLoader;
}
delegate = createMirror(this, mirrorLoader);
+ }
+ /**
+ * Runs the testcase.
+ *
+ * @throws BuildException in case of test failures or errors
+ * @since Ant 1.2
+ */
+ public void execute() throws BuildException {
+ setupJUnitDelegate();
+
List testLists = new ArrayList();
boolean forkPerTest = forkMode.getValue().equals(ForkMode.PER_TEST);
@@ -794,10 +801,6 @@
}
} finally {
deleteClassLoader();
- if (mirrorLoader instanceof SplitLoader) {
- ((SplitLoader) mirrorLoader).cleanup();
- }
- delegate = null;
}
}
@@ -1262,6 +1265,10 @@
* @return the results
*/
private TestResultHolder executeInVM(JUnitTest arg) throws BuildException {
+ if (delegate == null) {
+ setupJUnitDelegate();
+ }
+
JUnitTest test = (JUnitTest) arg.clone();
test.setProperties(getProject().getProperties());
if (dir != null) {
@@ -1514,6 +1521,10 @@
*/
private void logVmExit(FormatterElement[] feArray, JUnitTest test,
String message, String testCase) {
+ if (delegate == null) {
+ setupJUnitDelegate();
+ }
+
try {
log("Using System properties " + System.getProperties(),
Project.MSG_VERBOSE);
@@ -1595,11 +1606,16 @@
* Removes a classloader if needed.
* @since Ant 1.7
*/
- private void deleteClassLoader() {
+ protected void deleteClassLoader() {
if (classLoader != null) {
classLoader.cleanup();
classLoader = null;
}
+ if (mirrorLoader instanceof SplitLoader) {
+ ((SplitLoader) mirrorLoader).cleanup();
+ }
+ mirrorLoader = null;
+ delegate = null;
}
/**
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org
Re: The Cactus/JUnit-Task problem
Posted by Stefan Bodewig <bo...@apache.org>.
On Fri, 15 Feb 2008, Petar Tahchiev <pa...@gmail.com> wrote:
> in the execute(JUnitTest arg) method, too. Well, as Stefan
> suggested, we can put the
> if (delegate == null) {
> setupJUnitDelegate();
> }
>
> in executeInVM() method, that we call when we don't fork the JUnit
> execution. Well, as you see a little bit
> down in the code there is a piece that states:
> if (splitJunit) {
> classLoader = (AntClassLoader) delegate.getClass
> ().getClassLoader();
> } else {
> createClassLoader();
> }
>
> since I have no control over splitJUnit it is false, and then we
> invoke the createClassLoader(); method, which, invokes
> deleteClassLoader() and it, again, nullify-s the delegate object.
Ouch, my fault.
Actually I added the "delegate = null" statement to
deleteClassLoader after running my tests and didn't rerun the
tests after the change. I know I shouldn't do that, even if a
change looks simple.
Appended is a revised patch that doesn't exhibit the same
problem.
Stefan
Index: src/main/org/apache/tools/ant/taskdefs/optional/junit/JUnitTask.java
===================================================================
--- src/main/org/apache/tools/ant/taskdefs/optional/junit/JUnitTask.java (revision 628687)
+++ src/main/org/apache/tools/ant/taskdefs/optional/junit/JUnitTask.java (working copy)
@@ -162,6 +162,7 @@
private boolean splitJunit = false;
private JUnitTaskMirror delegate;
+ private ClassLoader mirrorLoader;
/** A boolean on whether to get the forked path for ant classes */
private boolean forkedPathChecked = false;
@@ -746,14 +747,10 @@
}
/**
- * Runs the testcase.
- *
- * @throws BuildException in case of test failures or errors
- * @since Ant 1.2
+ * Sets up the delegate that will actually run the tests.
*/
- public void execute() throws BuildException {
+ protected void setupJUnitDelegate() {
ClassLoader myLoader = JUnitTask.class.getClassLoader();
- ClassLoader mirrorLoader;
if (splitJunit) {
Path path = new Path(getProject());
path.add(antRuntimeClasses);
@@ -766,7 +763,17 @@
mirrorLoader = myLoader;
}
delegate = createMirror(this, mirrorLoader);
+ }
+ /**
+ * Runs the testcase.
+ *
+ * @throws BuildException in case of test failures or errors
+ * @since Ant 1.2
+ */
+ public void execute() throws BuildException {
+ setupJUnitDelegate();
+
List testLists = new ArrayList();
boolean forkPerTest = forkMode.getValue().equals(ForkMode.PER_TEST);
@@ -793,11 +800,7 @@
}
}
} finally {
- deleteClassLoader();
- if (mirrorLoader instanceof SplitLoader) {
- ((SplitLoader) mirrorLoader).cleanup();
- }
- delegate = null;
+ cleanup();
}
}
@@ -1262,6 +1265,10 @@
* @return the results
*/
private TestResultHolder executeInVM(JUnitTest arg) throws BuildException {
+ if (delegate == null) {
+ setupJUnitDelegate();
+ }
+
JUnitTest test = (JUnitTest) arg.clone();
test.setProperties(getProject().getProperties());
if (dir != null) {
@@ -1514,6 +1521,10 @@
*/
private void logVmExit(FormatterElement[] feArray, JUnitTest test,
String message, String testCase) {
+ if (delegate == null) {
+ setupJUnitDelegate();
+ }
+
try {
log("Using System properties " + System.getProperties(),
Project.MSG_VERBOSE);
@@ -1592,6 +1603,14 @@
}
/**
+ * Removes resources.
+ */
+ protected void cleanup() {
+ deleteClassLoader();
+ delegate = null;
+ }
+
+ /**
* Removes a classloader if needed.
* @since Ant 1.7
*/
@@ -1600,6 +1619,10 @@
classLoader.cleanup();
classLoader = null;
}
+ if (mirrorLoader instanceof SplitLoader) {
+ ((SplitLoader) mirrorLoader).cleanup();
+ }
+ mirrorLoader = null;
}
/**
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org
Re: The Cactus/JUnit-Task problem
Posted by Stefan Bodewig <bo...@apache.org>.
On Tue, 19 Feb 2008, Petar Tahchiev <pa...@gmail.com> wrote:
> Once again sorry, and you can feel free to apply the patch to
> Ant 1.7.2 and Ant 1.8.
It already is in trunk, so it should go into 1.8. I'll see whether it
can go into 1.7.1.
Stefan
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org
Re: The Cactus/JUnit-Task problem
Posted by Petar Tahchiev <pa...@gmail.com>.
Hi everybody,
I am really, really sorry for my ignorance :-(.
I spent the whole day, trying to figure out what is
the reason for the vmCrashString to be null. And I found it,
actually :-). The reason was me :-). I unintentially was
adding the
<dependency org="ant" name="ant-junit" rev="1.6.5"/>
to the classpath. After removing it, everything works just perfect.
Once again sorry, and you can feel free to apply the patch to
Ant 1.7.2 and Ant 1.8.
Cheers, Petar.
2008/2/19, Stefan Bodewig <bo...@apache.org>:
>
> On Mon, 18 Feb 2008, Petar Tahchiev <pa...@gmail.com> wrote:
>
> > Stefan, my patch was almost the same as your improved
> > version.
>
> OK, I've committed my changes as a first part.
>
> > However, my patch had these additional lines:
> >
> > @@ -1035,7 +1024,7 @@
> > if (watchdog != null && watchdog.killedProcess()) {
> > result.timedOut = true;
> > logTimeout(feArray, test, vmCrashString);
> > - } else if
> > (!Constants.TERMINATED_SUCCESSFULLY.equals(vmCrashString)) {
> > + } else if
> > (!Constants.TERMINATED_SUCCESSFULLY.equals(vmCrashString) &&
> > vmCrashString != null) { result.crashed = true; logVmCrash(feArray,
> > test, vmCrashString);
> > }
>
> No, that change would be wrong.
>
> Ant's TestRunner writes a file that will contain
> Constants.TERMINATED_SUCCESSFULLY if the TestRunner has run to
> completion. This file will be empty or contains anything else if the
> forked VM has been terminated before writing the file.
>
> With your change we'd no longer detect crashed forked VMs.
>
> > As you can see I have added the && vmCrashString != null statement,
> > because cactus's tests seem to return null when the test is executed
> > successfully.
>
> Then we should try to find out why it is null when you run the tests
> from Cactus.
>
> How do you start the forked VM?
>
> Stefan
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
> For additional commands, e-mail: dev-help@ant.apache.org
>
>
--
Regards, Petar!
Karlovo, Bulgaria.
EOOXML Objections
http://www.grokdoc.net/index.php/EOOXML_objections
Public PGP Key at:
https://keyserver1.pgp.com/vkd/DownloadKey.event?keyid=0x19658550C3110611
Key Fingerprint: A369 A7EE 61BC 93A3 CDFF 55A5 1965 8550 C311 0611
Re: The Cactus/JUnit-Task problem
Posted by Stefan Bodewig <bo...@apache.org>.
On Mon, 18 Feb 2008, Petar Tahchiev <pa...@gmail.com> wrote:
> Stefan, my patch was almost the same as your improved
> version.
OK, I've committed my changes as a first part.
> However, my patch had these additional lines:
>
> @@ -1035,7 +1024,7 @@
> if (watchdog != null && watchdog.killedProcess()) {
> result.timedOut = true;
> logTimeout(feArray, test, vmCrashString);
> - } else if
> (!Constants.TERMINATED_SUCCESSFULLY.equals(vmCrashString)) {
> + } else if
> (!Constants.TERMINATED_SUCCESSFULLY.equals(vmCrashString) &&
> vmCrashString != null) { result.crashed = true; logVmCrash(feArray,
> test, vmCrashString);
> }
No, that change would be wrong.
Ant's TestRunner writes a file that will contain
Constants.TERMINATED_SUCCESSFULLY if the TestRunner has run to
completion. This file will be empty or contains anything else if the
forked VM has been terminated before writing the file.
With your change we'd no longer detect crashed forked VMs.
> As you can see I have added the && vmCrashString != null statement,
> because cactus's tests seem to return null when the test is executed
> successfully.
Then we should try to find out why it is null when you run the tests
from Cactus.
How do you start the forked VM?
Stefan
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org
Re: The Cactus/JUnit-Task problem
Posted by Petar Tahchiev <pa...@gmail.com>.
Hi all,
Stefan, my patch was almost the same as your improved version. However, my
patch had these
additional lines:
@@ -1035,7 +1024,7 @@
if (watchdog != null && watchdog.killedProcess()) {
result.timedOut = true;
logTimeout(feArray, test, vmCrashString);
- } else if
(!Constants.TERMINATED_SUCCESSFULLY.equals(vmCrashString)) {
+ } else if
(!Constants.TERMINATED_SUCCESSFULLY.equals(vmCrashString) && vmCrashString
!= null) {
result.crashed = true;
logVmCrash(feArray, test, vmCrashString);
}
As you can see I have added the
&& vmCrashString != null
statement, because cactus's tests seem to return null when the test is
executed successfully.
With this one statement and your improved patch all works just perfect.
Kind regards, Petar.
2008/2/18, Stefan Bodewig <bo...@apache.org>:
>
> On Sat, 16 Feb 2008, Petar Tahchiev <pa...@gmail.com> wrote:
>
> > I have modified Stefan's patch just a little bit, so that now
> > everything works just perfect.
>
> Your patch didn't make it to the list. Could you please take a
> look at my revised patch to verify it works for you?
>
> Thanks
>
> Stefan
>
--
Regards, Petar!
Karlovo, Bulgaria.
EOOXML Objections
http://www.grokdoc.net/index.php/EOOXML_objections
Public PGP Key at:
https://keyserver1.pgp.com/vkd/DownloadKey.event?keyid=0x19658550C3110611
Key Fingerprint: A369 A7EE 61BC 93A3 CDFF 55A5 1965 8550 C311 0611
Re: The Cactus/JUnit-Task problem
Posted by Stefan Bodewig <bo...@apache.org>.
On Sat, 16 Feb 2008, Petar Tahchiev <pa...@gmail.com> wrote:
> I have modified Stefan's patch just a little bit, so that now
> everything works just perfect.
Your patch didn't make it to the list. Could you please take a
look at my revised patch to verify it works for you?
Thanks
Stefan
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org
Re: The Cactus/JUnit-Task problem
Posted by Petar Tahchiev <pa...@gmail.com>.
Hi guys,
I have modified Stefan's patch just a little bit, so that now everything
works just perfect.
Please review it and approve it, if you think everything is OK. Otherwise,
please tell me
what is wrong with the patch, so that I can improve it.
Kind regards, Petar.
2008/2/15, Petar Tahchiev <pa...@gmail.com>:
>
> Hi everybody,
>
> I like the solution of separating the configuration of the delegate object
> in a new method most,also.
> But the point is that this solution will be really hard for several
> reasons.
> 1) First of all, as Peter said, Cactus extends JUnit task and so we
> override the
> execute method. So we have to add the snippet
>
> setupJUnitDelegate()
>
> in the execute(JUnitTest arg) method, too. Well, as Stefan suggested, we
> can put the
> if (delegate == null) {
> setupJUnitDelegate();
> }
>
> in executeInVM() method, that we call when we don't fork the JUnit
> execution. Well, as you see a little bit
> down in the code there is a piece that states:
> if (splitJunit) {
> classLoader = (AntClassLoader) delegate.getClass
> ().getClassLoader();
> } else {
> createClassLoader();
> }
>
> since I have no control over splitJUnit it is false, and then we invoke
> the createClassLoader(); method,
> which, invokes deleteClassLoader() and it, again, nullify-s the delegate
> object. So another NLP is thrown
> on this line <1>:
>
> runner = delegate.newJUnitTestRunner(test, test.getHaltonerror
> (),
> test.getFiltertrace(),
> test.getHaltonfailure(), false,
> true, classLoader);
>
>
> Well, we can hack it really ugly by adding this line:
> if (delegate == null) {
> setupJUnitDelegate();
> }
>
> right before the line where the line with the NLP (<1>). Then everything
> works smooth.
>
> What happens if we do fork the test execution. Then in case of a failure
> the logVmExit
> method is called and in it we see the same situation:
> 1) we initialize the delegate object with the setup method
> 2) due to the splitJunit is being false we enter the createClassLoader()
> method where we nullify the delegate object.
>
> Maybe I am too unaware of the Ant API, and maybe I am mistaken at some
> point, so please correct if
> anything from the upper is wrong.
>
> I hope to find some resolution.
>
> Cheers, Petar.
>
>
> 2008/2/15, Peter Reilly <pe...@gmail.com>:
> >
> > This sounds excellent.
> > However, since Cactus replaces the execute method, would it not
> > need to add code to call setupJUnitDelegate()
> >
> >
> > Peter
> >
> >
> > On Fri, Feb 15, 2008 at 10:48 AM, Stefan Bodewig <bo...@apache.org>
> > wrote:
> > > Hi all,
> > >
> > > [Petar, it would be good if you subscribed to dev@ant, it is not that
> > > high traffic anyway]
> > >
> > > last night I chatted with Petar about the backwards incompatible
> > > change to the JUnit task we introduced in Ant 1.7.0 that broke
> > Cactus.
> > >
> > > Cactus' Ant task extends the JUnit task (and if memory serves me
> > right
> > > is one of the reasons that a bunch of methods in JUnitTask have
> > > protected access in the first place). It used to override execute()
> > > completely and invoke the execute variants that acceps tests or lists
> > > of tests (I don't recall which).
> > >
> > > This doesn't work any longer since execute() performs setup work on
> > > the delegate that decouples Ant from the junit library and the
> > execute
> > > variants rely on this setup.
> > >
> > > On my way home I thought that maybe the easiest solution would be to
> > > have the execute variants check whether the setup has been performed
> > > and if not - simply do it themselves. The appended patch does just
> > > that and I'd like to get some feedback.
> > >
> > > The patch would make deleteClassloader protected so that subclasses
> > > can cleanup themselves, this may not strictly be necessary.
> > >
> > > With this patch in place, Cactus should be able to use Ant without
> > any
> > > modifications, but could benefit from more control over resource
> > > cleanup if it wants to.
> > >
> > > BTW, I'd love to merge whatever solution we agree on to the 1.7branch
> > > and have it go into 1.7.1. Right now Cactus users are forced to
> > stick
> > > to 1.6.5 and we should change that.
> > >
> > > Of course Petar should make sure that Gump can build Cactus so that
> > he
> > > can hit us if we break it again. 8-)
> > >
> > > Stefan
> > >
> > > Index:
> > src/main/org/apache/tools/ant/taskdefs/optional/junit/JUnitTask.java
> > > ===================================================================
> > > ---
> > src/main/org/apache/tools/ant/taskdefs/optional/junit/JUnitTask.java (revision
> > 627950)
> > > +++
> > src/main/org/apache/tools/ant/taskdefs/optional/junit/JUnitTask.java (working
> > copy)
> > > @@ -162,6 +162,7 @@
> > >
> > > private boolean splitJunit = false;
> > > private JUnitTaskMirror delegate;
> > > + private ClassLoader mirrorLoader;
> > >
> > > /** A boolean on whether to get the forked path for ant classes
> > */
> > > private boolean forkedPathChecked = false;
> > > @@ -746,14 +747,10 @@
> > > }
> > >
> > > /**
> > > - * Runs the testcase.
> > > - *
> > > - * @throws BuildException in case of test failures or errors
> > > - * @since Ant 1.2
> > > + * Sets up the delegate that will actually run the tests.
> > > */
> > > - public void execute() throws BuildException {
> > > + protected void setupJUnitDelegate() {
> > > ClassLoader myLoader = JUnitTask.class.getClassLoader();
> > > - ClassLoader mirrorLoader;
> > > if (splitJunit) {
> > > Path path = new Path(getProject());
> > > path.add(antRuntimeClasses);
> > > @@ -766,7 +763,17 @@
> > > mirrorLoader = myLoader;
> > > }
> > > delegate = createMirror(this, mirrorLoader);
> > > + }
> > >
> > > + /**
> > > + * Runs the testcase.
> > > + *
> > > + * @throws BuildException in case of test failures or errors
> > > + * @since Ant 1.2
> > > + */
> > > + public void execute() throws BuildException {
> > > + setupJUnitDelegate();
> > > +
> > > List testLists = new ArrayList();
> > >
> > > boolean forkPerTest = forkMode.getValue().equals(
> > ForkMode.PER_TEST);
> > > @@ -794,10 +801,6 @@
> > > }
> > > } finally {
> > > deleteClassLoader();
> > > - if (mirrorLoader instanceof SplitLoader) {
> > > - ((SplitLoader) mirrorLoader).cleanup();
> > > - }
> > > - delegate = null;
> > > }
> > > }
> > >
> > > @@ -1262,6 +1265,10 @@
> > > * @return the results
> > > */
> > > private TestResultHolder executeInVM(JUnitTest arg) throws
> > BuildException {
> > > + if (delegate == null) {
> > > + setupJUnitDelegate();
> > > + }
> > > +
> > > JUnitTest test = (JUnitTest) arg.clone();
> > > test.setProperties(getProject().getProperties());
> > > if (dir != null) {
> > > @@ -1514,6 +1521,10 @@
> > > */
> > > private void logVmExit(FormatterElement[] feArray, JUnitTest
> > test,
> > > String message, String testCase) {
> > > + if (delegate == null) {
> > > + setupJUnitDelegate();
> > > + }
> > > +
> > > try {
> > > log("Using System properties " + System.getProperties(),
> > > Project.MSG_VERBOSE);
> > > @@ -1595,11 +1606,16 @@
> > > * Removes a classloader if needed.
> > > * @since Ant 1.7
> > > */
> > > - private void deleteClassLoader() {
> > > + protected void deleteClassLoader() {
> > > if (classLoader != null) {
> > > classLoader.cleanup();
> > > classLoader = null;
> > > }
> > > + if (mirrorLoader instanceof SplitLoader) {
> > > + ((SplitLoader) mirrorLoader).cleanup();
> > > + }
> > > + mirrorLoader = null;
> > > + delegate = null;
> > > }
> > >
> > > /**
> > >
> >
> > > ---------------------------------------------------------------------
> > > To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
> > > For additional commands, e-mail: dev-help@ant.apache.org
> > >
> > >
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
> > For additional commands, e-mail: dev-help@ant.apache.org
> >
> >
>
>
> --
> Regards, Petar!
> Karlovo, Bulgaria.
>
> EOOXML Objections
> http://www.grokdoc.net/index.php/EOOXML_objections
>
> Public PGP Key at:
> https://keyserver1.pgp.com/vkd/DownloadKey.event?keyid=0x19658550C3110611
> Key Fingerprint: A369 A7EE 61BC 93A3 CDFF 55A5 1965 8550 C311 0611
--
Regards, Petar!
Karlovo, Bulgaria.
EOOXML Objections
http://www.grokdoc.net/index.php/EOOXML_objections
Public PGP Key at:
https://keyserver1.pgp.com/vkd/DownloadKey.event?keyid=0x19658550C3110611
Key Fingerprint: A369 A7EE 61BC 93A3 CDFF 55A5 1965 8550 C311 0611
Re: The Cactus/JUnit-Task problem
Posted by Petar Tahchiev <pa...@gmail.com>.
Hi everybody,
I like the solution of separating the configuration of the delegate object
in a new method most,also.
But the point is that this solution will be really hard for several reasons.
1) First of all, as Peter said, Cactus extends JUnit task and so we override
the
execute method. So we have to add the snippet
setupJUnitDelegate()
in the execute(JUnitTest arg) method, too. Well, as Stefan suggested, we can
put the
if (delegate == null) {
setupJUnitDelegate();
}
in executeInVM() method, that we call when we don't fork the JUnit
execution. Well, as you see a little bit
down in the code there is a piece that states:
if (splitJunit) {
classLoader = (AntClassLoader) delegate.getClass
().getClassLoader();
} else {
createClassLoader();
}
since I have no control over splitJUnit it is false, and then we invoke the
createClassLoader(); method,
which, invokes deleteClassLoader() and it, again, nullify-s the delegate
object. So another NLP is thrown
on this line <1>:
runner = delegate.newJUnitTestRunner(test, test.getHaltonerror
(),
test.getFiltertrace(),
test.getHaltonfailure(), false,
true, classLoader);
Well, we can hack it really ugly by adding this line:
if (delegate == null) {
setupJUnitDelegate();
}
right before the line where the line with the NLP (<1>). Then everything
works smooth.
What happens if we do fork the test execution. Then in case of a failure the
logVmExit
method is called and in it we see the same situation:
1) we initialize the delegate object with the setup method
2) due to the splitJunit is being false we enter the createClassLoader()
method where we nullify the delegate object.
Maybe I am too unaware of the Ant API, and maybe I am mistaken at some
point, so please correct if
anything from the upper is wrong.
I hope to find some resolution.
Cheers, Petar.
2008/2/15, Peter Reilly <pe...@gmail.com>:
>
> This sounds excellent.
> However, since Cactus replaces the execute method, would it not
> need to add code to call setupJUnitDelegate()
>
>
> Peter
>
>
> On Fri, Feb 15, 2008 at 10:48 AM, Stefan Bodewig <bo...@apache.org>
> wrote:
> > Hi all,
> >
> > [Petar, it would be good if you subscribed to dev@ant, it is not that
> > high traffic anyway]
> >
> > last night I chatted with Petar about the backwards incompatible
> > change to the JUnit task we introduced in Ant 1.7.0 that broke Cactus.
> >
> > Cactus' Ant task extends the JUnit task (and if memory serves me right
> > is one of the reasons that a bunch of methods in JUnitTask have
> > protected access in the first place). It used to override execute()
> > completely and invoke the execute variants that acceps tests or lists
> > of tests (I don't recall which).
> >
> > This doesn't work any longer since execute() performs setup work on
> > the delegate that decouples Ant from the junit library and the execute
> > variants rely on this setup.
> >
> > On my way home I thought that maybe the easiest solution would be to
> > have the execute variants check whether the setup has been performed
> > and if not - simply do it themselves. The appended patch does just
> > that and I'd like to get some feedback.
> >
> > The patch would make deleteClassloader protected so that subclasses
> > can cleanup themselves, this may not strictly be necessary.
> >
> > With this patch in place, Cactus should be able to use Ant without any
> > modifications, but could benefit from more control over resource
> > cleanup if it wants to.
> >
> > BTW, I'd love to merge whatever solution we agree on to the 1.7 branch
> > and have it go into 1.7.1. Right now Cactus users are forced to stick
> > to 1.6.5 and we should change that.
> >
> > Of course Petar should make sure that Gump can build Cactus so that he
> > can hit us if we break it again. 8-)
> >
> > Stefan
> >
> > Index:
> src/main/org/apache/tools/ant/taskdefs/optional/junit/JUnitTask.java
> > ===================================================================
> > ---
> src/main/org/apache/tools/ant/taskdefs/optional/junit/JUnitTask.java (revision
> 627950)
> > +++
> src/main/org/apache/tools/ant/taskdefs/optional/junit/JUnitTask.java (working
> copy)
> > @@ -162,6 +162,7 @@
> >
> > private boolean splitJunit = false;
> > private JUnitTaskMirror delegate;
> > + private ClassLoader mirrorLoader;
> >
> > /** A boolean on whether to get the forked path for ant classes */
> > private boolean forkedPathChecked = false;
> > @@ -746,14 +747,10 @@
> > }
> >
> > /**
> > - * Runs the testcase.
> > - *
> > - * @throws BuildException in case of test failures or errors
> > - * @since Ant 1.2
> > + * Sets up the delegate that will actually run the tests.
> > */
> > - public void execute() throws BuildException {
> > + protected void setupJUnitDelegate() {
> > ClassLoader myLoader = JUnitTask.class.getClassLoader();
> > - ClassLoader mirrorLoader;
> > if (splitJunit) {
> > Path path = new Path(getProject());
> > path.add(antRuntimeClasses);
> > @@ -766,7 +763,17 @@
> > mirrorLoader = myLoader;
> > }
> > delegate = createMirror(this, mirrorLoader);
> > + }
> >
> > + /**
> > + * Runs the testcase.
> > + *
> > + * @throws BuildException in case of test failures or errors
> > + * @since Ant 1.2
> > + */
> > + public void execute() throws BuildException {
> > + setupJUnitDelegate();
> > +
> > List testLists = new ArrayList();
> >
> > boolean forkPerTest = forkMode.getValue().equals(
> ForkMode.PER_TEST);
> > @@ -794,10 +801,6 @@
> > }
> > } finally {
> > deleteClassLoader();
> > - if (mirrorLoader instanceof SplitLoader) {
> > - ((SplitLoader) mirrorLoader).cleanup();
> > - }
> > - delegate = null;
> > }
> > }
> >
> > @@ -1262,6 +1265,10 @@
> > * @return the results
> > */
> > private TestResultHolder executeInVM(JUnitTest arg) throws
> BuildException {
> > + if (delegate == null) {
> > + setupJUnitDelegate();
> > + }
> > +
> > JUnitTest test = (JUnitTest) arg.clone();
> > test.setProperties(getProject().getProperties());
> > if (dir != null) {
> > @@ -1514,6 +1521,10 @@
> > */
> > private void logVmExit(FormatterElement[] feArray, JUnitTest test,
> > String message, String testCase) {
> > + if (delegate == null) {
> > + setupJUnitDelegate();
> > + }
> > +
> > try {
> > log("Using System properties " + System.getProperties(),
> > Project.MSG_VERBOSE);
> > @@ -1595,11 +1606,16 @@
> > * Removes a classloader if needed.
> > * @since Ant 1.7
> > */
> > - private void deleteClassLoader() {
> > + protected void deleteClassLoader() {
> > if (classLoader != null) {
> > classLoader.cleanup();
> > classLoader = null;
> > }
> > + if (mirrorLoader instanceof SplitLoader) {
> > + ((SplitLoader) mirrorLoader).cleanup();
> > + }
> > + mirrorLoader = null;
> > + delegate = null;
> > }
> >
> > /**
> >
>
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
> > For additional commands, e-mail: dev-help@ant.apache.org
> >
> >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
> For additional commands, e-mail: dev-help@ant.apache.org
>
>
--
Regards, Petar!
Karlovo, Bulgaria.
EOOXML Objections
http://www.grokdoc.net/index.php/EOOXML_objections
Public PGP Key at:
https://keyserver1.pgp.com/vkd/DownloadKey.event?keyid=0x19658550C3110611
Key Fingerprint: A369 A7EE 61BC 93A3 CDFF 55A5 1965 8550 C311 0611
Re: The Cactus/JUnit-Task problem
Posted by Stefan Bodewig <bo...@apache.org>.
On Fri, 15 Feb 2008, Peter Reilly <pe...@gmail.com> wrote:
> However, since Cactus replaces the execute method, would it not
> need to add code to call setupJUnitDelegate()
No, it is invoked implicitly by all methods that need a delegate if
none has been defined.
Stefan
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org
Re: The Cactus/JUnit-Task problem
Posted by Peter Reilly <pe...@gmail.com>.
This sounds excellent.
However, since Cactus replaces the execute method, would it not
need to add code to call setupJUnitDelegate()
Peter
On Fri, Feb 15, 2008 at 10:48 AM, Stefan Bodewig <bo...@apache.org> wrote:
> Hi all,
>
> [Petar, it would be good if you subscribed to dev@ant, it is not that
> high traffic anyway]
>
> last night I chatted with Petar about the backwards incompatible
> change to the JUnit task we introduced in Ant 1.7.0 that broke Cactus.
>
> Cactus' Ant task extends the JUnit task (and if memory serves me right
> is one of the reasons that a bunch of methods in JUnitTask have
> protected access in the first place). It used to override execute()
> completely and invoke the execute variants that acceps tests or lists
> of tests (I don't recall which).
>
> This doesn't work any longer since execute() performs setup work on
> the delegate that decouples Ant from the junit library and the execute
> variants rely on this setup.
>
> On my way home I thought that maybe the easiest solution would be to
> have the execute variants check whether the setup has been performed
> and if not - simply do it themselves. The appended patch does just
> that and I'd like to get some feedback.
>
> The patch would make deleteClassloader protected so that subclasses
> can cleanup themselves, this may not strictly be necessary.
>
> With this patch in place, Cactus should be able to use Ant without any
> modifications, but could benefit from more control over resource
> cleanup if it wants to.
>
> BTW, I'd love to merge whatever solution we agree on to the 1.7 branch
> and have it go into 1.7.1. Right now Cactus users are forced to stick
> to 1.6.5 and we should change that.
>
> Of course Petar should make sure that Gump can build Cactus so that he
> can hit us if we break it again. 8-)
>
> Stefan
>
> Index: src/main/org/apache/tools/ant/taskdefs/optional/junit/JUnitTask.java
> ===================================================================
> --- src/main/org/apache/tools/ant/taskdefs/optional/junit/JUnitTask.java (revision 627950)
> +++ src/main/org/apache/tools/ant/taskdefs/optional/junit/JUnitTask.java (working copy)
> @@ -162,6 +162,7 @@
>
> private boolean splitJunit = false;
> private JUnitTaskMirror delegate;
> + private ClassLoader mirrorLoader;
>
> /** A boolean on whether to get the forked path for ant classes */
> private boolean forkedPathChecked = false;
> @@ -746,14 +747,10 @@
> }
>
> /**
> - * Runs the testcase.
> - *
> - * @throws BuildException in case of test failures or errors
> - * @since Ant 1.2
> + * Sets up the delegate that will actually run the tests.
> */
> - public void execute() throws BuildException {
> + protected void setupJUnitDelegate() {
> ClassLoader myLoader = JUnitTask.class.getClassLoader();
> - ClassLoader mirrorLoader;
> if (splitJunit) {
> Path path = new Path(getProject());
> path.add(antRuntimeClasses);
> @@ -766,7 +763,17 @@
> mirrorLoader = myLoader;
> }
> delegate = createMirror(this, mirrorLoader);
> + }
>
> + /**
> + * Runs the testcase.
> + *
> + * @throws BuildException in case of test failures or errors
> + * @since Ant 1.2
> + */
> + public void execute() throws BuildException {
> + setupJUnitDelegate();
> +
> List testLists = new ArrayList();
>
> boolean forkPerTest = forkMode.getValue().equals(ForkMode.PER_TEST);
> @@ -794,10 +801,6 @@
> }
> } finally {
> deleteClassLoader();
> - if (mirrorLoader instanceof SplitLoader) {
> - ((SplitLoader) mirrorLoader).cleanup();
> - }
> - delegate = null;
> }
> }
>
> @@ -1262,6 +1265,10 @@
> * @return the results
> */
> private TestResultHolder executeInVM(JUnitTest arg) throws BuildException {
> + if (delegate == null) {
> + setupJUnitDelegate();
> + }
> +
> JUnitTest test = (JUnitTest) arg.clone();
> test.setProperties(getProject().getProperties());
> if (dir != null) {
> @@ -1514,6 +1521,10 @@
> */
> private void logVmExit(FormatterElement[] feArray, JUnitTest test,
> String message, String testCase) {
> + if (delegate == null) {
> + setupJUnitDelegate();
> + }
> +
> try {
> log("Using System properties " + System.getProperties(),
> Project.MSG_VERBOSE);
> @@ -1595,11 +1606,16 @@
> * Removes a classloader if needed.
> * @since Ant 1.7
> */
> - private void deleteClassLoader() {
> + protected void deleteClassLoader() {
> if (classLoader != null) {
> classLoader.cleanup();
> classLoader = null;
> }
> + if (mirrorLoader instanceof SplitLoader) {
> + ((SplitLoader) mirrorLoader).cleanup();
> + }
> + mirrorLoader = null;
> + delegate = null;
> }
>
> /**
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
> For additional commands, e-mail: dev-help@ant.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org
Re: The Cactus/JUnit-Task problem
Posted by Steve Loughran <st...@apache.org>.
Stefan Bodewig wrote:
> Hi all,
>
> [Petar, it would be good if you subscribed to dev@ant, it is not that
> high traffic anyway]
>
> last night I chatted with Petar about the backwards incompatible
> change to the JUnit task we introduced in Ant 1.7.0 that broke Cactus.
>
> Cactus' Ant task extends the JUnit task (and if memory serves me right
> is one of the reasons that a bunch of methods in JUnitTask have
> protected access in the first place). It used to override execute()
> completely and invoke the execute variants that acceps tests or lists
> of tests (I don't recall which).
>
> This doesn't work any longer since execute() performs setup work on
> the delegate that decouples Ant from the junit library and the execute
> variants rely on this setup.
>
> On my way home I thought that maybe the easiest solution would be to
> have the execute variants check whether the setup has been performed
> and if not - simply do it themselves. The appended patch does just
> that and I'd like to get some feedback.
>
> The patch would make deleteClassloader protected so that subclasses
> can cleanup themselves, this may not strictly be necessary.
>
> With this patch in place, Cactus should be able to use Ant without any
> modifications, but could benefit from more control over resource
> cleanup if it wants to.
>
> BTW, I'd love to merge whatever solution we agree on to the 1.7 branch
> and have it go into 1.7.1. Right now Cactus users are forced to stick
> to 1.6.5 and we should change that.
+1
>
> Of course Petar should make sure that Gump can build Cactus so that he
> can hit us if we break it again. 8-)
+2
--
Steve Loughran http://www.1060.org/blogxter/publish/5
Author: Ant in Action http://antbook.org/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org