Junit Task warning about multiple versions of Ant

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

Junit Task warning about multiple versions of Ant

Nicolas Lalevée
The Junit task is printing a warning if it finds multiple versions of Ant in the classpath of the unit tests. It seems it doesn’t do correctly the job if the ant runtime is explicitly removed from the classpath.

Here the function which checks the classpath:
https://github.com/apache/ant/blob/master/src/main/org/apache/tools/ant/taskdefs/optional/junit/JUnitTask.java#L1362 <https://github.com/apache/ant/blob/master/src/main/org/apache/tools/ant/taskdefs/optional/junit/JUnitTask.java#L1362>

And here is the one which build the classloader during the actual forked run:
https://github.com/apache/ant/blob/master/src/main/org/apache/tools/ant/taskdefs/optional/junit/JUnitTask.java#L1952 <https://github.com/apache/ant/blob/master/src/main/org/apache/tools/ant/taskdefs/optional/junit/JUnitTask.java#L1952>

Shouldn’t the classloader be built the same way in both function?

Nicolas,
trying to release Ivy, finding bugs in Ant :p

Reply | Threaded
Open this post in threaded view
|

Re: Junit Task warning about multiple versions of Ant

Stefan Bodewig
On 2018-04-12, Nicolas Lalevée wrote:

> The Junit task is printing a warning if it finds multiple versions of
> Ant in the classpath of the unit tests. It seems it doesn’t do
> correctly the job if the ant runtime is explicitly removed from the
> classpath.

Quite possible.

> Here the function which checks the classpath:
> https://github.com/apache/ant/blob/master/src/main/org/apache/tools/ant/taskdefs/optional/junit/JUnitTask.java#L1362 <https://github.com/apache/ant/blob/master/src/main/org/apache/tools/ant/taskdefs/optional/junit/JUnitTask.java#L1362>

this is in the forked case.

> And here is the one which build the classloader during the actual forked run:
> https://github.com/apache/ant/blob/master/src/main/org/apache/tools/ant/taskdefs/optional/junit/JUnitTask.java#L1952 <https://github.com/apache/ant/blob/master/src/main/org/apache/tools/ant/taskdefs/optional/junit/JUnitTask.java#L1952>

AFAICT createClassloader is not invoked for forked VMs, only in the
non-forked case.

> Shouldn’t the classloader be built the same way in both function?

In the forked case, the classloader is not built by the task, the
CommandLineJava instance collects the classpath and sets it as
-classpath command line argument.

> trying to release Ivy, finding bugs in Ant :p

:-)

Stefan

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: Junit Task warning about multiple versions of Ant

Nicolas Lalevée


> Le 12 avr. 2018 à 16:57, Stefan Bodewig <[hidden email]> a écrit :
>
> On 2018-04-12, Nicolas Lalevée wrote:
>
>> The Junit task is printing a warning if it finds multiple versions of
>> Ant in the classpath of the unit tests. It seems it doesn’t do
>> correctly the job if the ant runtime is explicitly removed from the
>> classpath.
>
> Quite possible.
>
>> Here the function which checks the classpath:
>> https://github.com/apache/ant/blob/master/src/main/org/apache/tools/ant/taskdefs/optional/junit/JUnitTask.java#L1362 <https://github.com/apache/ant/blob/master/src/main/org/apache/tools/ant/taskdefs/optional/junit/JUnitTask.java#L1362>
>
> this is in the forked case.
>
>> And here is the one which build the classloader during the actual forked run:
>> https://github.com/apache/ant/blob/master/src/main/org/apache/tools/ant/taskdefs/optional/junit/JUnitTask.java#L1952 <https://github.com/apache/ant/blob/master/src/main/org/apache/tools/ant/taskdefs/optional/junit/JUnitTask.java#L1952>
>
> AFAICT createClassloader is not invoked for forked VMs, only in the
> non-forked case.
>
>> Shouldn’t the classloader be built the same way in both function?
>
> In the forked case, the classloader is not built by the task, the
> CommandLineJava instance collects the classpath and sets it as
> -classpath command line argument.

Ha yes, you’re right.

As far as I can see, the classpath used by checkForkedPath is the proper one. The function which manipulates the classpath to add the Ant runtime [1] is called before. So I should start looking into the AntClassLoader which is improperly finding the Ant classes. Maybe we should « isolate » it.

Or maybe that check for duplicate ant jar is only useful when includeantruntime is _not_ set to « no ». Since includeantruntime is true by default, it is nice that Ant is printing a warning when it also find Ant classes in the provided classpath, it is a common pitfall. But when includeantruntime is explicitely set to false, then I would say that the user know what he's doing, thus no need for special check.

Nicolas

[1] https://github.com/apache/ant/blob/master/src/main/org/apache/tools/ant/taskdefs/optional/junit/JUnitTask.java#L1320 <https://github.com/apache/ant/blob/master/src/main/org/apache/tools/ant/taskdefs/optional/junit/JUnitTask.java#L1320>
Reply | Threaded
Open this post in threaded view
|

Re: Junit Task warning about multiple versions of Ant

Stefan Bodewig
On 2018-04-12, Nicolas Lalevée wrote:

> As far as I can see, the classpath used by checkForkedPath is the
> proper one. The function which manipulates the classpath to add the
> Ant runtime [1] is called before. So I should start looking into the
> AntClassLoader which is improperly finding the Ant classes. Maybe we
> should « isolate » it.

Sounds reasonable, so we can ensure it really only contains what will be
on -classpath as we..

> Or maybe that check for duplicate ant jar is only useful when
> includeantruntime is _not_ set to « no ». Since includeantruntime is
> true by default, it is nice that Ant is printing a warning when it
> also find Ant classes in the provided classpath, it is a common
> pitfall. But when includeantruntime is explicitely set to false, then
> I would say that the user know what he's doing, thus no need for
> special check.

I'm not sure. To be honest the check and message are there so we get
fewer bug reports by helping people figure out their problem
themselves. At one point in time this has been a very common problem,
which likely predated the includeAntRuntime attribute. If we manage to
isolate the classloader (the infrastructure should be there) then we
shouldn't need to disable the check.

Stefan

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: Junit Task warning about multiple versions of Ant

Nicolas Lalevée
I have been able to make unit test which reproduces the false warning. And I have fixed the way the classloader was created. Thanks for the insights.

Nicolas

> Le 13 avr. 2018 à 07:38, Stefan Bodewig <[hidden email]> a écrit :
>
> On 2018-04-12, Nicolas Lalevée wrote:
>
>> As far as I can see, the classpath used by checkForkedPath is the
>> proper one. The function which manipulates the classpath to add the
>> Ant runtime [1] is called before. So I should start looking into the
>> AntClassLoader which is improperly finding the Ant classes. Maybe we
>> should « isolate » it.
>
> Sounds reasonable, so we can ensure it really only contains what will be
> on -classpath as we..
>
>> Or maybe that check for duplicate ant jar is only useful when
>> includeantruntime is _not_ set to « no ». Since includeantruntime is
>> true by default, it is nice that Ant is printing a warning when it
>> also find Ant classes in the provided classpath, it is a common
>> pitfall. But when includeantruntime is explicitely set to false, then
>> I would say that the user know what he's doing, thus no need for
>> special check.
>
> I'm not sure. To be honest the check and message are there so we get
> fewer bug reports by helping people figure out their problem
> themselves. At one point in time this has been a very common problem,
> which likely predated the includeAntRuntime attribute. If we manage to
> isolate the classloader (the infrastructure should be there) then we
> shouldn't need to disable the check.
>
> Stefan
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]