[Geoserver-devel] Junit 4 and @BeforeClass... no good?

Hi,
I've spent some time trying to make our read only test cases
initialize the data directory just once but I think I'm stumbling
into a design limitation of JUnit4.

On trunk the GeoServerAbstractTestClass has a configurable setup
method that calls some extra template methods the subclasses
have to provide:
* getLoggingConfiguration(): to change the logs config while
   building/debugging a test suite, but silence them otherwise
   (by default a very silent log config is used)
* TestData getTestData(), that builds the TestData object
   that sets up the data directory. We have subclasses for
   "mock" testing based on property fiels and geotiffs,
   one for tests that do use a pre-cooked data dir, and
   one that uses pre-cooked data dir + external dbms based
   datastore
* getSpringContextLocations() to have a test run with a specific
   set of applicationContext.xml files instead of the usual one.

The problem is, whatever method is marked as one time setup
using @BeforeClass has to be static, so the above framework
cannot work.

The way Gabriel was referring to for running one time
setup with junit3 uses static fields and methods too...

I'm a little short on ideas, anyone with some smarts on how
to deal with this limitation?

The one solution I found so far is... to avoid using JUnit
and use TestNG instead. TestNG has a different model
for running tests and that allows the usage of non
static methods for @BeforeClass initializations:
http://www.ibm.com/developerworks/java/library/j-cq08296/

More information here:
http://www.scribd.com/doc/331929/Beyong-JUnit-TestNG
http://testng.org/doc/

Glub... any better idea?
Cheers
Andrea

Andrea Aime a écrit :

The problem is, whatever method is marked as one time setup
using @BeforeClass has to be static, so the above framework
cannot work.

I have the feeling that it is on intend in order to make sure that the @BeforeClass method writes only in static fields. Given that JUnit creates a new instance of the test class for every test methods (last time I checked - maybe TestNG does not), then if we want to have non-static fields initialized we would need to run the initialization method before every tests, which is exactly what we wish to avoid with @BeforeClass...

Glub... any better idea?

I have not looked to the test so I may be telling a inaplicable proposal... But is there anything preventing to have a @BeforeClass method initializing some static fields, and a @Before method copying the values from the static fields to non-static fields?

  Martin

Andrea Aime ha scritto:

Hi,

...

Glub... any better idea?

Hum, I managed to get it sort of working using only junit3. I realized
I could keep only the applicationContext and TestData fields
as static, and setup/teardown would stay non static, but access
the two static fields (which no subclass has legitimate needs
to override).
Basically, setup creates the static fields, and teardowns
decides whether to kill them or not based on a method
that provides a data/config modification flag.

However, there is a big downside for this approach... since
the static field is the same (same base class), under surefire the same
data dir will be used for all tests (even of different classes).
Any idea of how to fix this???

I've tried this on two different configurations. One is a single
test of WFS, GetFeatureTest. Testing in eclipse using a main with
a text based test runner, the time went down from 11s to 9s.
Eek, less than 20% gain!
Looking with a profiler the problem became quite easy to spot:
what's killing us in tests is classloading and JAI initialization,
which happens one time anyways.

Yet, doing the same for the whole wcs1_1 package (which only
does read only tests so it was easy to change)
I got a nice speedup, the whole "mvn test"
went down from 54 seconds to 28s, which is sort of the gain
I was hoping for (in this case the same VM is used for all the
tests, so classloading is payed just once for _all_ the tests).

So ok, it seems there is some goodness in one time initialization,
the problem is getting it right, so that the static fields
are wiped out when the suite for the current class is done.
Hmmm.... ideas? :slight_smile:

Cheers
Andrea

Andrea Aime ha scritto:

Andrea Aime ha scritto:

Hi,

...

Glub... any better idea?

Hum, I managed to get it sort of working using only junit3. I realized
I could keep only the applicationContext and TestData fields
as static, and setup/teardown would stay non static, but access
the two static fields (which no subclass has legitimate needs
to override).
Basically, setup creates the static fields, and teardowns
decides whether to kill them or not based on a method
that provides a data/config modification flag.

However, there is a big downside for this approach... since
the static field is the same (same base class), under surefire the same
data dir will be used for all tests (even of different classes).
Any idea of how to fix this???

Sorry for this many mails, they are helping me think :slight_smile:
Ok, the fields could be wiped out by using a junit4 @BeforeClass
method that nullifies them. This should work.

It just feels quite a bit convoluted. The one time setup methods
are not actually doing the one time setup, but only cleanup,
whilst setup() internally does its checks and makes sure to
reinitialize the data dir only when needed.

Sure a TestNG solution would be cleaner... thought there
seem to be some issues with it and surefire, in that one
has to use the proper surefire/testng releases combo
in order to have stuff work:
http://www.kai-mai.com/node/96

Cheers
Andrea

Martin Desruisseaux ha scritto:
...

Glub... any better idea?

I have not looked to the test so I may be telling a inaplicable proposal... But is there anything preventing to have a @BeforeClass method initializing some static fields, and a @Before method copying the values from the static fields to non-static fields?

The problem is that the @beforeClass method has to access non static methods that will be overridden/defined in subclasses (the main geoserver test class is abstract). Basically every subclass can define
its own logging level, fake data directory creation mode, and spring context composition... but I cannot do that if the @BeforeClass method is static.

I have a solution (see my last mail) but brr.... feels quite like a hack. TestNG would provide an elegant way to do what I'm trying to,
but it's not as established as JUnit...

Cheers
Andrea

Andrea Aime a écrit :

I have a solution (see my last mail) but brr.... feels quite like a hack. TestNG would provide an elegant way to do what I'm trying to,
but it's not as established as JUnit...

I have nothing again a switch from JUnit to TestNG (given that IBM article seems to recommand TestNG) if it work with NetBeans/Maven, and if we can manage to do a progressive transition... I believe that TestNG use annotations of the same name than JUnit 4 (at least @Test, maybe @Before and @After as well). So maybe we could:

1) Make sure that all the code base migrated from JUnit 3 to JUnit 4.
2) Perform a search and replace from JUnit imports to TestNG imports.

So in the short term the action would just to ask maintainers to migrate their module to JUnit 4 when they have a chance...

  Martin

Martin Desruisseaux ha scritto:

Andrea Aime a écrit :

I have a solution (see my last mail) but brr.... feels quite like a hack. TestNG would provide an elegant way to do what I'm trying to,
but it's not as established as JUnit...

I have nothing again a switch from JUnit to TestNG (given that IBM article seems to recommand TestNG) if it work with NetBeans/Maven, and if we can manage to do a progressive transition... I believe that TestNG use annotations of the same name than JUnit 4 (at least @Test, maybe @Before and @After as well). So maybe we could:

1) Make sure that all the code base migrated from JUnit 3 to JUnit 4.
2) Perform a search and replace from JUnit imports to TestNG imports.

So in the short term the action would just to ask maintainers to migrate their module to JUnit 4 when they have a chance...

Afaik there is a tool to migrate all junit3 tests to testng in one shot:
http://testng.org/doc/migrating.html

Cheers
Andrea

Hi Andrea,

good stuff.

I can only think of a custom "framework" to handle that, which would be
something like:

public abstract class GeoServerAbstractTestClass extends TestCase{

*private* static boolean oneTimeSetupDone = false;

public *final* void setUp() throws Exception{
        if(!oneTimeSetupDone){
         oneTimeSetup();
       }
       oneTimeSetupDone = true;
       setUp_Internal();
}

  /** oneTimeSetup not being static allows to use geos resources when already
available */
  protected *abstract* void oneTimeSetup();

  protected *abstract* void setUp_Internal()throws Exception;

}

This way, we call oneTimeSetup in a non static context, but let to the
concrete subclasses the responsability of writing their own oneTimeTearDown()
*static* method.
Let me know if that makes sense or what am I missing.

Cheers,

Gabriel

On Wednesday 09 April 2008 04:08:45 pm Andrea Aime wrote:

Andrea Aime ha scritto:
> Andrea Aime ha scritto:
>> Hi,
>
> ...
>
>> Glub... any better idea?
>
> Hum, I managed to get it sort of working using only junit3. I realized
> I could keep only the applicationContext and TestData fields
> as static, and setup/teardown would stay non static, but access
> the two static fields (which no subclass has legitimate needs
> to override).
> Basically, setup creates the static fields, and teardowns
> decides whether to kill them or not based on a method
> that provides a data/config modification flag.
>
> However, there is a big downside for this approach... since
> the static field is the same (same base class), under surefire the same
> data dir will be used for all tests (even of different classes).
> Any idea of how to fix this???

Sorry for this many mails, they are helping me think :slight_smile:
Ok, the fields could be wiped out by using a junit4 @BeforeClass
method that nullifies them. This should work.

It just feels quite a bit convoluted. The one time setup methods
are not actually doing the one time setup, but only cleanup,
whilst setup() internally does its checks and makes sure to
reinitialize the data dir only when needed.

Sure a TestNG solution would be cleaner... thought there
seem to be some issues with it and surefire, in that one
has to use the proper surefire/testng releases combo
in order to have stuff work:
http://www.kai-mai.com/node/96

Cheers
Andrea

-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference
Don't miss this year's exciting event. There's still time to save $100.
Use priority code J8TL2D2.
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/java
one _______________________________________________
Geoserver-devel mailing list
Geoserver-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel

!DSPAM:4045,47fcce18114132090977483!

Gabriel Roldán ha scritto:

Hi Andrea,

good stuff.

I can only think of a custom "framework" to handle that, which would be something like:

public abstract class GeoServerAbstractTestClass extends TestCase{

*private* static boolean oneTimeSetupDone = false;

public *final* void setUp() throws Exception{
        if(!oneTimeSetupDone){
         oneTimeSetup(); }
       oneTimeSetupDone = true;
       setUp_Internal(); }
   /** oneTimeSetup not being static allows to use geos resources when already available */
  protected *abstract* void oneTimeSetup();
   protected *abstract* void setUp_Internal()throws Exception;

}

This way, we call oneTimeSetup in a non static context, but let to the concrete subclasses the responsability of writing their own oneTimeTearDown() *static* method.
Let me know if that makes sense or what am I missing.

Well, it looks a lot like my first attempt, and it fails because the same static variable is shared by all subclasses (so the one time initialization occurrs only once for the set of test classes you're
running in the suite).
We need a one time setup part that clears the the boolean flag before the test class starts, which is what I'm doing now... but feels
quite a bit hacky.

I'm attaching my current solution (which uses TestData as a flag since
I have to keep it around anyways). The issue I have with it is that
it smells of a hack. With TestNG the hack would not be necessary, since the same test object is used to run all tests and the @BeforeClass
annotated method does not need to be static.

Oh, to use JUnit4 consistently we'd have to switch all the tests
at once, since otherwise Eclipse (and I believe, Maven too) will
notice our classes extend junit3 TestCase class and end up using
junit3 runner instead of the junit4 one.

The same would happen with TestNG. Which I do like more on a
general point of view (the test approach seems quite a bit cleaner),
but worries me when it comes to tool support maturity (see the maven issue).

Cheers
Andrea

(attachments)

GeoServerAbstractTestSupport.java (31 KB)

On Thursday 10 April 2008 11:20:20 am Andrea Aime wrote:

Gabriel Roldán ha scritto:
> Hi Andrea,
>
> good stuff.
>
> I can only think of a custom "framework" to handle that, which would be
> something like:
>
> public abstract class GeoServerAbstractTestClass extends TestCase{
>
> *private* static boolean oneTimeSetupDone = false;
>
> public *final* void setUp() throws Exception{
> if(!oneTimeSetupDone){
> oneTimeSetup();
> }
> oneTimeSetupDone = true;
> setUp_Internal();
> }
>
> /** oneTimeSetup not being static allows to use geos resources when
> already available */
> protected *abstract* void oneTimeSetup();
>
> protected *abstract* void setUp_Internal()throws Exception;
>
> }
>
> This way, we call oneTimeSetup in a non static context, but let to the
> concrete subclasses the responsability of writing their own
> oneTimeTearDown() *static* method.
> Let me know if that makes sense or what am I missing.

Well, it looks a lot like my first attempt, and it fails because the
same static variable is shared by all subclasses (so the one time
initialization occurrs only once for the set of test classes you're
running in the suite).

agreed, it is as hacky as it is. Yet doing it explicitly as a template class
(the final setUp and the setUp_Internal for subclass specific needs)
documents the "framework" far better than stating subclasses should call
super.setUp etc.
Honestly I'm a bit hesitant of switching to TestNG just because I'm lazy and
prefer to have a single unit testing framework across the projects we have to
deal with in parallel (that is geos/gt).

We need a one time setup part that clears the the boolean flag before
the test class starts, which is what I'm doing now... but feels
quite a bit hacky.

What I don't quite understand is why having a oneTimeTearDown clearing the
flag wouldn't work? if not in the superclass itself, on the concrete
subclasses?

Gabriel

I'm attaching my current solution (which uses TestData as a flag since
I have to keep it around anyways). The issue I have with it is that
it smells of a hack. With TestNG the hack would not be necessary, since
the same test object is used to run all tests and the @BeforeClass
annotated method does not need to be static.

Oh, to use JUnit4 consistently we'd have to switch all the tests
at once, since otherwise Eclipse (and I believe, Maven too) will
notice our classes extend junit3 TestCase class and end up using
junit3 runner instead of the junit4 one.

The same would happen with TestNG. Which I do like more on a
general point of view (the test approach seems quite a bit cleaner),
but worries me when it comes to tool support maturity (see the maven
issue).

Cheers
Andrea

!DSPAM:4045,47fddbdf267478362916074!

Gabriel Roldán ha scritto:

agreed, it is as hacky as it is. Yet doing it explicitly as a template class (the final setUp and the setUp_Internal for subclass specific needs) documents the "framework" far better than stating subclasses should call super.setUp etc.
Honestly I'm a bit hesitant of switching to TestNG just because I'm lazy and prefer to have a single unit testing framework across the projects we have to deal with in parallel (that is geos/gt).

Yeah, I agree on both points. I'll go for the template class that
enforces a framework. And then I'll have to change all test classes
to use junit4 annotations.

Btw, up until geotools is fully switched to junit4, you'll have
to work with a bit of junit3 and junit4 at the same time anyways :wink:

We need a one time setup part that clears the the boolean flag before
the test class starts, which is what I'm doing now... but feels
quite a bit hacky.

What I don't quite understand is why having a oneTimeTearDown clearing the flag wouldn't work? if not in the superclass itself, on the concrete subclasses?

Ah sorry, I did not get that fully. Yes, a oneTimeTearDown would work
(I did the cleaning in oneTimeSetUp but it's the same, you have to
make sure the flag gets cleared up in between one test and the other).

Cheers
Andrea

Heck, doesn't the OneTimeTestDecorator allows for just what we want? (non
static oneTimeSetUp and oneTimeTearDown):
<http://www.jroller.com/aalmiray/entry/junit_enabling_one_time_setup&gt;

Gabriel
On Thursday 10 April 2008 11:37:44 am Gabriel Roldán wrote:

On Thursday 10 April 2008 11:20:20 am Andrea Aime wrote:
> Gabriel Roldán ha scritto:
> > Hi Andrea,
> >
> > good stuff.
> >
> > I can only think of a custom "framework" to handle that, which would be
> > something like:
> >
> > public abstract class GeoServerAbstractTestClass extends TestCase{
> >
> > *private* static boolean oneTimeSetupDone = false;
> >
> > public *final* void setUp() throws Exception{
> > if(!oneTimeSetupDone){
> > oneTimeSetup();
> > }
> > oneTimeSetupDone = true;
> > setUp_Internal();
> > }
> >
> > /** oneTimeSetup not being static allows to use geos resources when
> > already available */
> > protected *abstract* void oneTimeSetup();
> >
> > protected *abstract* void setUp_Internal()throws Exception;
> >
> > }
> >
> > This way, we call oneTimeSetup in a non static context, but let to the
> > concrete subclasses the responsability of writing their own
> > oneTimeTearDown() *static* method.
> > Let me know if that makes sense or what am I missing.
>
> Well, it looks a lot like my first attempt, and it fails because the
> same static variable is shared by all subclasses (so the one time
> initialization occurrs only once for the set of test classes you're
> running in the suite).

agreed, it is as hacky as it is. Yet doing it explicitly as a template
class (the final setUp and the setUp_Internal for subclass specific needs)
documents the "framework" far better than stating subclasses should call
super.setUp etc.
Honestly I'm a bit hesitant of switching to TestNG just because I'm lazy
and prefer to have a single unit testing framework across the projects we
have to deal with in parallel (that is geos/gt).

> We need a one time setup part that clears the the boolean flag before
> the test class starts, which is what I'm doing now... but feels
> quite a bit hacky.

What I don't quite understand is why having a oneTimeTearDown clearing the
flag wouldn't work? if not in the superclass itself, on the concrete
subclasses?

Gabriel

> I'm attaching my current solution (which uses TestData as a flag since
> I have to keep it around anyways). The issue I have with it is that
> it smells of a hack. With TestNG the hack would not be necessary, since
> the same test object is used to run all tests and the @BeforeClass
> annotated method does not need to be static.
>
> Oh, to use JUnit4 consistently we'd have to switch all the tests
> at once, since otherwise Eclipse (and I believe, Maven too) will
> notice our classes extend junit3 TestCase class and end up using
> junit3 runner instead of the junit4 one.
>
> The same would happen with TestNG. Which I do like more on a
> general point of view (the test approach seems quite a bit cleaner),
> but worries me when it comes to tool support maturity (see the maven
> issue).
>
> Cheers
> Andrea
>
>
> !DSPAM:4045,47fddbdf267478362916074!

Gabriel Roldán ha scritto:

Heck, doesn't the OneTimeTestDecorator allows for just what we want? (non static oneTimeSetUp and oneTimeTearDown):
<http://www.jroller.com/aalmiray/entry/junit_enabling_one_time_setup&gt;

I looked at it, but I did not get how to use it. First, I can't seem
to find the OneTimeTestDecorator class at all.
Second, the suite() method is static, meaning it would have to be
repeated for each single unit test class.

cheers
Andrea

On Thursday 10 April 2008 11:45:26 am Andrea Aime wrote:

Gabriel Roldán ha scritto:
> Heck, doesn't the OneTimeTestDecorator allows for just what we want? (non
> static oneTimeSetUp and oneTimeTearDown):
> <http://www.jroller.com/aalmiray/entry/junit_enabling_one_time_setup&gt;

I looked at it, but I did not get how to use it. First, I can't seem
to find the OneTimeTestDecorator class at all.

<http://sourceforge.net/projects/junit-addons&gt;

Second, the suite() method is static, meaning it would have to be
repeated for each single unit test class.

right. Is that so bad? I mean, smells less hacky.
Yet I wonder what happens when you want to run a single test case from inside
eclipse, for example... won't work.
We can go the template way so
Gabriel

cheers
Andrea

!DSPAM:4045,47fde1bb274975332866982!

Gabriel Roldán ha scritto:

On Thursday 10 April 2008 11:45:26 am Andrea Aime wrote:

Gabriel Roldán ha scritto:

Heck, doesn't the OneTimeTestDecorator allows for just what we want? (non
static oneTimeSetUp and oneTimeTearDown):
<http://www.jroller.com/aalmiray/entry/junit_enabling_one_time_setup&gt;

I looked at it, but I did not get how to use it. First, I can't seem
to find the OneTimeTestDecorator class at all.

<http://sourceforge.net/projects/junit-addons&gt;

Ah, thank you.

Second, the suite() method is static, meaning it would have to be
repeated for each single unit test class.

right. Is that so bad? I mean, smells less hacky.
Yet I wonder what happens when you want to run a single test case from inside eclipse, for example... won't work.
We can go the template way so

Well, having to remember doing that is messy, since there is nothing
enforcing you to add that static method. The framework approach, based on junit4, seems a cleaner way.

Cheers
Andrea