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 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]
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
