Re: The Cactus/JUnit-Task problem
On Tue, 19 Feb 2008, Petar Tahchiev [EMAIL PROTECTED] 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: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: The Cactus/JUnit-Task problem
On Fri, 15 Feb 2008, Peter Reilly [EMAIL PROTECTED] 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: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: The Cactus/JUnit-Task problem
On Fri, 15 Feb 2008, Petar Tahchiev [EMAIL PROTECTED] 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: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
Re: The Cactus/JUnit-Task problem
On Sat, 16 Feb 2008, Petar Tahchiev [EMAIL PROTECTED] 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: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: The Cactus/JUnit-Task problem
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 [EMAIL PROTECTED]:
On Sat, 16 Feb 2008, Petar Tahchiev [EMAIL PROTECTED] 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
On Mon, 18 Feb 2008, Petar Tahchiev [EMAIL PROTECTED] 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: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
Re: The Cactus/JUnit-Task problem
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 [EMAIL PROTECTED]:
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 [EMAIL PROTECTED]:
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 [EMAIL PROTECTED]
wrote:
Hi all,
[Petar, it would be good if you subscribed to [EMAIL PROTECTED], 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
The Cactus/JUnit-Task problem
Hi all,
[Petar, it would be good if you subscribed to [EMAIL PROTECTED], 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: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
Re: The Cactus/JUnit-Task problem
Stefan Bodewig wrote: Hi all, [Petar, it would be good if you subscribed to [EMAIL PROTECTED], 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: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: The Cactus/JUnit-Task problem
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 [EMAIL PROTECTED]:
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 [EMAIL PROTECTED]
wrote:
Hi all,
[Petar, it would be good if you subscribed to [EMAIL PROTECTED], 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
