Mass changes to various projects under Ant umbrella - should we be doing it?

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

Re: Mass changes to various projects under Ant umbrella - should we be doing it?

Gintautas Grigelionis
2018-04-13 5:33 GMT+00:00 Stefan Bodewig <[hidden email]>:

> On 2018-04-12, Gintautas Grigelionis wrote:
>
> > 2018-04-12 15:18 GMT+00:00 Stefan Bodewig <[hidden email]>:
>
> >> On 2018-04-12, Gintautas Grigelionis wrote:
>
> >>> 2018-04-12 14:39 GMT+00:00 Stefan Bodewig <[hidden email]>:
>
> >>>> On 2018-04-11, Gintautas Grigelionis wrote:
>
> >>>>> XMLCatalogTest is an example of unit tests complicated by root
> >> property.
>
> >>>> Actually, the test even passes if you remove the line using the
> property
> >>>> as long as your current working directory is Ant's basedir :-)
>
> >>> What is the reason of having and/or keeping the property, anyway?
>
> >> Running the test when your current working directory is not the root of
> >> the Ant source tree. I don't know who does so, but it has been something
> >> somebody needed some time ago (and took the time to make it
> >> possible). Right now I don't see any reason to stop supporting that.
>
> > I'd say it's the other way around: unless I'm mistaken, I have to
> > remember to set the property in the IDEs that I use if I want to use a
> > test case for debugging. So IMHO it's an unnecessary complication for
> > no good reason.
>
> Ant test cases are not designed to be run from an IDE, this has never
> been a goal. I'm surprised this system property is the only thing you
> need to remember :-)
>

 Well, I decided to give it a spin :-) I find it useful to run debugger on
test cases from IDE.
Reply | Threaded
Open this post in threaded view
|

Re: Mass changes to various projects under Ant umbrella - should we be doing it?

Stefan Bodewig
On 2018-04-13, Gintautas Grigelionis wrote:

> 2018-04-13 5:33 GMT+00:00 Stefan Bodewig <[hidden email]>:

>> Ant test cases are not designed to be run from an IDE, this has never
>> been a goal. I'm surprised this system property is the only thing you
>> need to remember :-)

>  Well, I decided to give it a spin :-) I find it useful to run
> debugger on test cases from IDE.

I'm glad it works for you.

Please don't break different approaches of doing things just to make the
IDE use case easier, though. In particular please don't remove the root
system property and please run all tests via the build file before
pushing changes.

Thanks

        Stefan

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

Reply | Threaded
Open this post in threaded view
|

Re: Mass changes to various projects under Ant umbrella - should we be doing it?

Gintautas Grigelionis
2018-04-14 16:23 GMT+00:00 Stefan Bodewig <[hidden email]>:

> On 2018-04-13, Gintautas Grigelionis wrote:
>
> > 2018-04-13 5:33 GMT+00:00 Stefan Bodewig <[hidden email]>:
>
> >> Ant test cases are not designed to be run from an IDE, this has never
> >> been a goal. I'm surprised this system property is the only thing you
> >> need to remember :-)
>
> >  Well, I decided to give it a spin :-) I find it useful to run
> > debugger on test cases from IDE.
>
> I'm glad it works for you.
>
> Please don't break different approaches of doing things just to make the
> IDE use case easier, though. In particular please don't remove the root
> system property and please run all tests via the build file before
> pushing changes.
>

I do run tests, but my environment get messy sometimes, too.
I keep an eye on Jenkins, but I am not always in a position to fix things
immediately,
and I'm sorry about that.

Could you please explain what alternative approach does the "root" property
support
_and_ add assumptions checks, since it causes nasty NPEs when not set?
I'd rather be adding assumptions checks for "ant.home" property.
By the looks of it, JUnit 5 runner tests need an assumption check, too.

Thanks,
Gintas
Reply | Threaded
Open this post in threaded view
|

Re: Mass changes to various projects under Ant umbrella - should we be doing it?

Stefan Bodewig
On 2018-04-15, Gintautas Grigelionis wrote:

> Could you please explain what alternative approach does the "root"
> property support

It has been added with
https://github.com/apache/ant/commit/71333195c9d57d80d1a44cd8362a641c62d5e214
and the commit message states the main use case "running tests from
an arbitrary directory". I don't recall the details, you may want to
look at the mailing list archive of December 2004.

My main point is this is something that has been working for more than
thriteen years and I don't want to break it for people who rely on it
just because it makes running the tests from an IDE slightly
inconvenient (by having to remember something).

> _and_ add assumptions checks, since it causes nasty NPEs when not set?

IIRC the fallback would be to use the current working directory instead
of the "root" property if the former is not set. We can easily deal with
this by using

System.getProperty("root", System.getProperty("user.dir"))

instead, right?

> By the looks of it, JUnit 5 runner tests need an assumption check, too.

Not sure why, but I may again be missing something.

Stefan

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

Reply | Threaded
Open this post in threaded view
|

Re: Mass changes to various projects under Ant umbrella - should we be doing it?

Gintautas Grigelionis
2018-04-15 9:59 GMT+00:00 Stefan Bodewig <[hidden email]>:

> On 2018-04-15, Gintautas Grigelionis wrote:
>
> > Could you please explain what alternative approach does the "root"
> > property support
>
> It has been added with
> https://github.com/apache/ant/commit/71333195c9d57d80d1a44cd8362a64
> 1c62d5e214
> and the commit message states the main use case "running tests from
> an arbitrary directory". I don't recall the details, you may want to
> look at the mailing list archive of December 2004.
>
> My main point is this is something that has been working for more than
> thirteen years and I don't want to break it for people who rely on it
> just because it makes running the tests from an IDE slightly
> inconvenient (by having to remember something).
>

Thanks for the pointer. The idea with assumptions is that they serve as a
reminder
and the way they should be used is by splitting the tests so that they are
executed or not dependent
on assumptions -- which makes test cases simple. See RmicTest, for example.

Moreover, the refactoring of tests is not about extracting repetitive code
into "smart" methods;
it's about parameterizing -- which is another nice thing about JUnit 4 --
and we'll be getting to that.
Yet another thing is splitting the tests in order to extract the fixtures
which belong to @Before.

> _and_ add assumptions checks, since it causes nasty NPEs when not set?
>
> IIRC the fallback would be to use the current working directory instead
> of the "root" property if the former is not set. We can easily deal with
> this by using
>
> System.getProperty("root", System.getProperty("user.dir"))
>
> instead, right?
>

Rather,

System.getProperty("root",
buildRule.getProject().getBaseDir().getAbsolutePath())

but, yeah, good point. Have to check if it works everywhere, though.

> By the looks of it, JUnit 5 runner tests need an assumption check, too.
>
> Not sure why, but I may again be missing something.
>

Please have a look at TeamCity tests at JetBrains. They start with a clean
directory, then run
ant -Ddest=optional -f fetch.xml, then ./build.sh test, and fail with

java.lang.Exception: No runnable methods


Gintas
Reply | Threaded
Open this post in threaded view
|

Re: Mass changes to various projects under Ant umbrella - should we be doing it?

Stefan Bodewig
On 2018-04-15, Gintautas Grigelionis wrote:

> Moreover, the refactoring of tests is not about extracting repetitive
> code into "smart" methods; it's about parameterizing -- which is
> another nice thing about JUnit 4 -- and we'll be getting to that.

If we could get there with diff smaller than several 1000 lines, I'd
appreciate that. :-)

> 2018-04-15 9:59 GMT+00:00 Stefan Bodewig <[hidden email]>:

>> IIRC the fallback would be to use the current working directory instead
>> of the "root" property if the former is not set. We can easily deal with
>> this by using

>> System.getProperty("root", System.getProperty("user.dir"))

>> instead, right?

> Rather,

> System.getProperty("root",
> buildRule.getProject().getBaseDir().getAbsolutePath())

The one case where you removed the root property you switched to using
the one-arg File constructor, so I assumed you needed the current
working directory. Some of the tests using "root" don't use
BuildFileRule right now. But I don't really care how the code gets
adapted as long as it keeps the root property alive and allows you to
run the tests from an IDE.

>> By the looks of it, JUnit 5 runner tests need an assumption check, too.

>> Not sure why, but I may again be missing something.

> Please have a look at TeamCity tests at JetBrains. They start with a
> clean directory, then run ant -Ddest=optional -f fetch.xml, then
> ./build.sh test, and fail with

> java.lang.Exception: No runnable methods

This more looks as if we need an extra dependency for the tests and
should exclude the test class if no engine is present, but I'm not sure.

Stefan

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

Reply | Threaded
Open this post in threaded view
|

JUnit 5 tests incorrectly picked up during Ant junit tests

Jaikiran Pai
It indeed looks like a build exclusion that I might have missed. I will
take a look at this and fix it.

-Jaikiran


On 15/04/18 9:35 PM, Stefan Bodewig wrote:

> On 2018-04-15, Gintautas Grigelionis wrote:
>
>
>>> By the looks of it, JUnit 5 runner tests need an assumption check, too.
>>> Not sure why, but I may again be missing something.
>> Please have a look at TeamCity tests at JetBrains. They start with a
>> clean directory, then run ant -Ddest=optional -f fetch.xml, then
>> ./build.sh test, and fail with
>> java.lang.Exception: No runnable methods
> This more looks as if we need an extra dependency for the tests and
> should exclude the test class if no engine is present, but I'm not sure.
>
> Stefan
>


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

12