ant-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Petar Tahchiev" <paranoia...@gmail.com>
Subject Re: The Cactus/JUnit-Task problem
Date Fri, 15 Feb 2008 18:52:17 GMT
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 <peter.kitt.reilly@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 <bodewig@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

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message