[Geoserver-devel] GeoServerExtensions and testing

In a review of pull request 568 Kevin asked me to look at making GeoServerExtensions easier to use with mock objects.

It is currently not too bad to use with an mock ApplicationContext:

GeoServerResourceLoader resourceLoader = new GeoServerResourceLoader(baseDirectory);

ApplicationContext context = createNiceMock(ApplicationContext.class);
expect(context.getBean(“catalog”)).andReturn(cat).anyTimes();
expect(context.getBean(“resourceLoader”)).andReturn( resourceLoader );
expect(context.getBeanNamesForType(GeoServerResourceLoader.class)).andReturn(new String{“resourceLoader”}).anyTimes();
expect(context.getBeanNamesForType((Class)anyObject())).andReturn(new String{}).anyTimes();
replay(context);

The downside is hooking it up:

new GeoServerExtensions().setApplicationContext( context );

I have added two static methods on pull request 568:

GeoServerExtensions.mock( context );
GeoServerExtensions.mock(“resourceLoader”, resourceLoader);

This covers Kevin’s request of making this easier for test cases.

These methods set an internal isMock field that is used to suppress warnings from checkContext - resulting in smaller output from our test cases. We may also be able to suppress some of the catalog created / destroyed information messages during testing as they are not adding value.

Jody Garnett

On Tue, Apr 22, 2014 at 9:25 AM, Jody Garnett <jody.garnett@anonymised.com>wrote:

In a review of pull request 568 Kevin asked me to look at making
GeoServerExtensions easier to use with mock objects.

It is currently not too bad to use with an mock ApplicationContext:

        GeoServerResourceLoader resourceLoader = new
GeoServerResourceLoader(baseDirectory);
        ApplicationContext context =
createNiceMock(ApplicationContext.class);
        expect(context.getBean("catalog")).andReturn(cat).anyTimes();
        expect(context.getBean("resourceLoader")).andReturn(
resourceLoader );

expect(context.getBeanNamesForType(GeoServerResourceLoader.class)).andReturn(new
String{"resourceLoader"}).anyTimes();

expect(context.getBeanNamesForType((Class)anyObject())).andReturn(new
String{}).anyTimes();
        replay(context);

The downside is hooking it up:

       new GeoServerExtensions().setApplicationContext( context );

I have added two static methods on pull request 568:

       GeoServerExtensions.mock( context );
       GeoServerExtensions.mock("resourceLoader", resourceLoader);

I'm against having testing code creep into the normal API so explicitly.
Can we have a helper object that resides in testing packages that does
that, without pulluting GeoServerExtension
directly?

Cheers
Andrea

--

Meet us at GEO Business 2014! in London! Visit http://goo.gl/fES3aK
for more information.

Ing. Andrea Aime
@geowolf
Technical Lead

GeoSolutions S.A.S.
Via Poggio alle Viti 1187
55054 Massarosa (LU)
Italy
phone: +39 0584 962313
fax: +39 0584 1660272
mob: +39 339 8844549

http://www.geo-solutions.it
http://twitter.com/geosolutions_it

-------------------------------------------------------

I have added two static methods on pull request 568:

       GeoServerExtensions.mock( context );
       GeoServerExtensions.mock("resourceLoader", resourceLoader);

I'm against having testing code creep into the normal API so explicitly.

I can change the method to be a bit less obvious, but we are kind of stuck
since GeoServerExtensions is acting a singleton (much like
GeoserverDataDirectory before it).

Can we have a helper object that resides in testing packages that does
that, without pulluting GeoServerExtension
directly?

I am not quite sure how to make a testing only object to help out. Even if
we create one we still need to inject it into this singleton. I found a few
test cases manipulating the internal state of GeoServerExtension and I
figured methods were better for consistency.
--
Jody

On Tue, Apr 22, 2014 at 9:45 AM, Jody Garnett <jody.garnett@anonymised.com>wrote:

I have added two static methods on pull request 568:

       GeoServerExtensions.mock( context );
       GeoServerExtensions.mock("resourceLoader", resourceLoader);

I'm against having testing code creep into the normal API so explicitly.

I can change the method to be a bit less obvious, but we are kind of stuck
since GeoServerExtensions is acting a singleton (much like
GeoserverDataDirectory before it).

Methods that can be of general use are ok, just not stuff that is clearly
geared towards testing, that has no place in normal API.

Cheers
Andrea

--

Meet us at GEO Business 2014! in London! Visit http://goo.gl/fES3aK
for more information.

Ing. Andrea Aime
@geowolf
Technical Lead

GeoSolutions S.A.S.
Via Poggio alle Viti 1187
55054 Massarosa (LU)
Italy
phone: +39 0584 962313
fax: +39 0584 1660272
mob: +39 339 8844549

http://www.geo-solutions.it
http://twitter.com/geosolutions_it

-------------------------------------------------------

I note the tension between a minimal API for development and opening the door for ease of testing.

Here is what I am testing now:

  • renamed the methods as init( name, bean ) and init( applicationContext ).
  • added a check to init(name,bean) to detect any conflict with applicationContext
  • changed the flag to isSpringContext, and the javadocs to describe additional consistency checks

Let me know if that is sufficient.

···

Jody Garnett

On Tue, Apr 22, 2014 at 5:53 PM, Andrea Aime <andrea.aime@anonymised.com> wrote:

On Tue, Apr 22, 2014 at 9:45 AM, Jody Garnett <jody.garnett@anonymised.com> wrote:

Methods that can be of general use are ok, just not stuff that is clearly geared towards testing, that has no place in normal API.

Cheers

Andrea

==
Meet us at GEO Business 2014! in London! Visit http://goo.gl/fES3aK
for more information.

Ing. Andrea Aime

@geowolf
Technical Lead

GeoSolutions S.A.S.
Via Poggio alle Viti 1187
55054 Massarosa (LU)
Italy
phone: +39 0584 962313
fax: +39 0584 1660272
mob: +39 339 8844549

http://www.geo-solutions.it
http://twitter.com/geosolutions_it


I can change the method to be a bit less obvious, but we are kind of stuck since GeoServerExtensions is acting a singleton (much like GeoserverDataDirectory before it).

I have added two static methods on pull request 568:

GeoServerExtensions.mock( context );
GeoServerExtensions.mock(“resourceLoader”, resourceLoader);

I’m against having testing code creep into the normal API so explicitly.

On Tue, Apr 22, 2014 at 10:24 AM, Jody Garnett <jody.garnett@anonymised.com>wrote:

I note the tension between a minimal API for development and opening the
door for ease of testing.

Here is what I am testing now:
* renamed the methods as init( name, bean ) and init( applicationContext ).
* added a check to init(name,bean) to detect any conflict with
applicationContext
* changed the flag to isSpringContext, and the javadocs to describe
additional consistency checks

Let me know if that is sufficient.

Looks better to me. It would be nice if Justin could have a look as well

Cheers
Andrea

--

Meet us at GEO Business 2014! in London! Visit http://goo.gl/fES3aK
for more information.

Ing. Andrea Aime
@geowolf
Technical Lead

GeoSolutions S.A.S.
Via Poggio alle Viti 1187
55054 Massarosa (LU)
Italy
phone: +39 0584 962313
fax: +39 0584 1660272
mob: +39 339 8844549

http://www.geo-solutions.it
http://twitter.com/geosolutions_it

-------------------------------------------------------

Yep, I will finish testing and hold off hitting merge until we hear from Justin.
I doubt there is much good to be done here, GeoServerExtensions is still an improvement on what was before.

···

Jody Garnett

On Tue, Apr 22, 2014 at 6:28 PM, Andrea Aime <andrea.aime@anonymised.com> wrote:

On Tue, Apr 22, 2014 at 10:24 AM, Jody Garnett <jody.garnett@anonymised.com> wrote:

I note the tension between a minimal API for development and opening the door for ease of testing.

Here is what I am testing now:

  • renamed the methods as init( name, bean ) and init( applicationContext ).
  • added a check to init(name,bean) to detect any conflict with applicationContext
  • changed the flag to isSpringContext, and the javadocs to describe additional consistency checks

Let me know if that is sufficient.

Looks better to me. It would be nice if Justin could have a look as well

Cheers

Andrea

==
Meet us at GEO Business 2014! in London! Visit http://goo.gl/fES3aK
for more information.

Ing. Andrea Aime

@geowolf
Technical Lead

GeoSolutions S.A.S.
Via Poggio alle Viti 1187
55054 Massarosa (LU)
Italy
phone: +39 0584 962313
fax: +39 0584 1660272
mob: +39 339 8844549

http://www.geo-solutions.it
http://twitter.com/geosolutions_it


Sorry, I am having trouble figuring out the issue here, is there a pull request or patch to provide some context?

Is the problem that you don’t like doing this?

new GeoServerExtensions().setApplicationContext( context );

Why not just throw it in a test utility method and call it a day? Why do you have to modify GeoServerExtensions directly?

···

On Tue, Apr 22, 2014 at 2:33 AM, Jody Garnett <jody.garnett@anonymised.com> wrote:

Yep, I will finish testing and hold off hitting merge until we hear from Justin.
I doubt there is much good to be done here, GeoServerExtensions is still an improvement on what was before.


Start Your Social Network Today - Download eXo Platform
Build your Enterprise Intranet with eXo Platform Software
Java Based Open Source Intranet - Social, Extensible, Cloud Ready
Get Started Now And Turn Your Intranet Into A Collaboration Platform
http://p.sf.net/sfu/ExoPlatform


Geoserver-devel mailing list
Geoserver-devel@anonymised.comsts.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel

Justin Deoliveira
Vice President, Engineering | Boundless
jdeolive@anonymised.com
@j_deolive

Jody Garnett

On Tue, Apr 22, 2014 at 6:28 PM, Andrea Aime <andrea.aime@anonymised.com> wrote:

On Tue, Apr 22, 2014 at 10:24 AM, Jody Garnett <jody.garnett@anonymised.com…> wrote:

I note the tension between a minimal API for development and opening the door for ease of testing.

Here is what I am testing now:

  • renamed the methods as init( name, bean ) and init( applicationContext ).
  • added a check to init(name,bean) to detect any conflict with applicationContext
  • changed the flag to isSpringContext, and the javadocs to describe additional consistency checks

Let me know if that is sufficient.

Looks better to me. It would be nice if Justin could have a look as well

Cheers

Andrea

==
Meet us at GEO Business 2014! in London! Visit http://goo.gl/fES3aK
for more information.

Ing. Andrea Aime

@geowolf
Technical Lead

GeoSolutions S.A.S.
Via Poggio alle Viti 1187
55054 Massarosa (LU)
Italy
phone: +39 0584 962313
fax: +39 0584 1660272
mob: +39 339 8844549

http://www.geo-solutions.it
http://twitter.com/geosolutions_it


You are correct that that is a workaround if you have an application context.

The key motivation is Kevin asking for for a way to quickly set GeoServerResourceLoader for test cases that want to look it up using GeoServerExtensions.bean(GeoServerResourceLoader.class).

I added GeoServerExtensions.init( name, bean ) to make writing these test cases easier.

···

Jody Garnett

On Tue, Apr 22, 2014 at 11:16 PM, Justin Deoliveira <jdeolive@anonymised.com> wrote:

Sorry, I am having trouble figuring out the issue here, is there a pull request or patch to provide some context?

Is the problem that you don’t like doing this?

new GeoServerExtensions().setApplicationContext( context );

Why not just throw it in a test utility method and call it a day? Why do you have to modify GeoServerExtensions directly?

On Tue, Apr 22, 2014 at 2:33 AM, Jody Garnett <jody.garnett@anonymised.com> wrote:

Yep, I will finish testing and hold off hitting merge until we hear from Justin.
I doubt there is much good to be done here, GeoServerExtensions is still an improvement on what was before.


Start Your Social Network Today - Download eXo Platform
Build your Enterprise Intranet with eXo Platform Software
Java Based Open Source Intranet - Social, Extensible, Cloud Ready
Get Started Now And Turn Your Intranet Into A Collaboration Platform
http://p.sf.net/sfu/ExoPlatform


Geoserver-devel mailing list
Geoserver-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel

Justin Deoliveira
Vice President, Engineering | Boundless
jdeolive@anonymised.com
@j_deolive

Jody Garnett

On Tue, Apr 22, 2014 at 6:28 PM, Andrea Aime <andrea.aime@anonymised.com> wrote:

On Tue, Apr 22, 2014 at 10:24 AM, Jody Garnett <jody.garnett@anonymised.com…> wrote:

I note the tension between a minimal API for development and opening the door for ease of testing.

Here is what I am testing now:

  • renamed the methods as init( name, bean ) and init( applicationContext ).
  • added a check to init(name,bean) to detect any conflict with applicationContext
  • changed the flag to isSpringContext, and the javadocs to describe additional consistency checks

Let me know if that is sufficient.

Looks better to me. It would be nice if Justin could have a look as well

Cheers

Andrea

==
Meet us at GEO Business 2014! in London! Visit http://goo.gl/fES3aK
for more information.

Ing. Andrea Aime

@geowolf
Technical Lead

GeoSolutions S.A.S.
Via Poggio alle Viti 1187
55054 Massarosa (LU)
Italy
phone: +39 0584 962313
fax: +39 0584 1660272
mob: +39 339 8844549

http://www.geo-solutions.it
http://twitter.com/geosolutions_it


On Tue, Apr 22, 2014 at 7:23 AM, Jody Garnett <jody.garnett@anonymised.com>wrote:

You are correct that that is a workaround if you have an application
context.

The key motivation is Kevin asking for for a way to quickly set
GeoServerResourceLoader for test cases that want to look it up using
GeoServerExtensions.bean(GeoServerResourceLoader.class).

I added GeoServerExtensions.init( name, bean ) to make writing these test
cases easier.

Well my preference would be to not change the main api, and add a test

utility class that makes this easy. Also note that during the big test
change over last year we added some test utility classes to main to make
mocking up a GeoServer (and I am speaking true mocks here) easier. Look at
GeoServerMockTestSupport in org.geoserver.data.test. Perhaps you could put
what you need there.

Jody Garnett

On Tue, Apr 22, 2014 at 11:16 PM, Justin Deoliveira <
jdeolive@anonymised.com> wrote:

Sorry, I am having trouble figuring out the issue here, is there a pull
request or patch to provide some context?

Is the problem that you don't like doing this?

      new GeoServerExtensions().setApplicationContext( context );

Why not just throw it in a test utility method and call it a day? Why do
you have to modify GeoServerExtensions directly?

On Tue, Apr 22, 2014 at 2:33 AM, Jody Garnett <jody.garnett@anonymised.com>wrote:

Yep, I will finish testing and hold off hitting merge until we hear from
Justin.
I doubt there is much good to be done here, GeoServerExtensions is still
an improvement on what was before.

Jody Garnett

On Tue, Apr 22, 2014 at 6:28 PM, Andrea Aime <
andrea.aime@anonymised.com> wrote:

On Tue, Apr 22, 2014 at 10:24 AM, Jody Garnett <jody.garnett@anonymised.com>wrote:

I note the tension between a minimal API for development and opening
the door for ease of testing.

Here is what I am testing now:
* renamed the methods as init( name, bean ) and init(
applicationContext ).
* added a check to init(name,bean) to detect any conflict with
applicationContext
* changed the flag to isSpringContext, and the javadocs to describe
additional consistency checks

Let me know if that is sufficient.

Looks better to me. It would be nice if Justin could have a look as well

Cheers
Andrea

--

Meet us at GEO Business 2014! in London! Visit http://goo.gl/fES3aK
for more information.

Ing. Andrea Aime
@geowolf
Technical Lead

GeoSolutions S.A.S.
Via Poggio alle Viti 1187
55054 Massarosa (LU)
Italy
phone: +39 0584 962313
fax: +39 0584 1660272
mob: +39 339 8844549

http://www.geo-solutions.it
http://twitter.com/geosolutions_it

-------------------------------------------------------

------------------------------------------------------------------------------
Start Your Social Network Today - Download eXo Platform
Build your Enterprise Intranet with eXo Platform Software
Java Based Open Source Intranet - Social, Extensible, Cloud Ready
Get Started Now And Turn Your Intranet Into A Collaboration Platform
http://p.sf.net/sfu/ExoPlatform
_______________________________________________
Geoserver-devel mailing list
Geoserver-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel

--
*Justin Deoliveira*
Vice President, Engineering | Boundless
jdeolive@anonymised.com
@j_deolive <https://twitter.com/j_deolive&gt;

--
*Justin Deoliveira*
Vice President, Engineering | Boundless
jdeolive@anonymised.com
@j_deolive <https://twitter.com/j_deolive&gt;

I have already looked - as part of trying to address Kevin’s feedback, I am trying to do this quickly to move on to my next pull request.

GeoServerMockTestSupport happens a little late for factories that use GeoServerExtensions during
We previously had some code using the GeoserverDataDirectory singleton to access a GeoServerResourceLoader. I have tried to move this to GeoSeverExtensions.

Still if you like I can make a GeoServerExtensionsUtil in the same package and use package visibility to get access to GeoServerExtensions internals (to edit the singleton cache).

···

Jody Garnett

On Tue, Apr 22, 2014 at 11:42 PM, Justin Deoliveira <jdeolive@anonymised.com> wrote:

On Tue, Apr 22, 2014 at 7:23 AM, Jody Garnett <jody.garnett@anonymised.com> wrote:

You are correct that that is a workaround if you have an application context.

The key motivation is Kevin asking for for a way to quickly set GeoServerResourceLoader for test cases that want to look it up using GeoServerExtensions.bean(GeoServerResourceLoader.class).

I added GeoServerExtensions.init( name, bean ) to make writing these test cases easier.

Well my preference would be to not change the main api, and add a test utility class that makes this easy. Also note that during the big test change over last year we added some test utility classes to main to make mocking up a GeoServer (and I am speaking true mocks here) easier. Look at GeoServerMockTestSupport in org.geoserver.data.test. Perhaps you could put what you need there.

Justin Deoliveira
Vice President, Engineering | Boundless
jdeolive@anonymised.com
@j_deolive

Jody Garnett

On Tue, Apr 22, 2014 at 11:16 PM, Justin Deoliveira <jdeolive@anonymised.com> wrote:

Sorry, I am having trouble figuring out the issue here, is there a pull request or patch to provide some context?

Is the problem that you don’t like doing this?

new GeoServerExtensions().setApplicationContext( context );

Why not just throw it in a test utility method and call it a day? Why do you have to modify GeoServerExtensions directly?

On Tue, Apr 22, 2014 at 2:33 AM, Jody Garnett <jody.garnett@anonymised.com> wrote:

Yep, I will finish testing and hold off hitting merge until we hear from Justin.
I doubt there is much good to be done here, GeoServerExtensions is still an improvement on what was before.


Start Your Social Network Today - Download eXo Platform
Build your Enterprise Intranet with eXo Platform Software
Java Based Open Source Intranet - Social, Extensible, Cloud Ready
Get Started Now And Turn Your Intranet Into A Collaboration Platform
http://p.sf.net/sfu/ExoPlatform


Geoserver-devel mailing list
Geoserver-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel

Justin Deoliveira
Vice President, Engineering | Boundless
jdeolive@anonymised.com
@j_deolive

Jody Garnett

On Tue, Apr 22, 2014 at 6:28 PM, Andrea Aime <andrea.aime@anonymised.com> wrote:

On Tue, Apr 22, 2014 at 10:24 AM, Jody Garnett <jody.garnett@anonymised.com…> wrote:

I note the tension between a minimal API for development and opening the door for ease of testing.

Here is what I am testing now:

  • renamed the methods as init( name, bean ) and init( applicationContext ).
  • added a check to init(name,bean) to detect any conflict with applicationContext
  • changed the flag to isSpringContext, and the javadocs to describe additional consistency checks

Let me know if that is sufficient.

Looks better to me. It would be nice if Justin could have a look as well

Cheers

Andrea

==
Meet us at GEO Business 2014! in London! Visit http://goo.gl/fES3aK
for more information.

Ing. Andrea Aime

@geowolf
Technical Lead

GeoSolutions S.A.S.
Via Poggio alle Viti 1187
55054 Massarosa (LU)
Italy
phone: +39 0584 962313
fax: +39 0584 1660272
mob: +39 339 8844549

http://www.geo-solutions.it
http://twitter.com/geosolutions_it


On Tue, Apr 22, 2014 at 7:59 AM, Jody Garnett <jody.garnett@anonymised.com>wrote:

I have already looked - as part of trying to address Kevin's feedback, I
am trying to do this quickly to move on to my next pull request.

GeoServerMockTestSupport happens a little late for factories that use
GeoServerExtensions during
We previously had some code using the GeoserverDataDirectory singleton to
access a GeoServerResourceLoader. I have tried to move this to
GeoSeverExtensions.

Still if you like I can make a GeoServerExtensionsUtil in the same package
and use package visibility to get access to GeoServerExtensions internals
(to edit the singleton cache).

You're not being clear about the motivation here, or at least i can't infer
it from this conversation. Can you just point me at a specific test case
that Kevin has written.

--

Jody Garnett

On Tue, Apr 22, 2014 at 11:42 PM, Justin Deoliveira <
jdeolive@anonymised.com> wrote:

On Tue, Apr 22, 2014 at 7:23 AM, Jody Garnett <jody.garnett@anonymised.com>wrote:

You are correct that that is a workaround if you have an application
context.

The key motivation is Kevin asking for for a way to quickly set
GeoServerResourceLoader for test cases that want to look it up using
GeoServerExtensions.bean(GeoServerResourceLoader.class).

I added GeoServerExtensions.init( name, bean ) to make writing these
test cases easier.

Well my preference would be to not change the main api, and add a test

utility class that makes this easy. Also note that during the big test
change over last year we added some test utility classes to main to make
mocking up a GeoServer (and I am speaking true mocks here) easier. Look at
GeoServerMockTestSupport in org.geoserver.data.test. Perhaps you could put
what you need there.

Jody Garnett

On Tue, Apr 22, 2014 at 11:16 PM, Justin Deoliveira <
jdeolive@anonymised.com> wrote:

Sorry, I am having trouble figuring out the issue here, is there a pull
request or patch to provide some context?

Is the problem that you don't like doing this?

      new GeoServerExtensions().setApplicationContext( context );

Why not just throw it in a test utility method and call it a day? Why
do you have to modify GeoServerExtensions directly?

On Tue, Apr 22, 2014 at 2:33 AM, Jody Garnett <jody.garnett@anonymised.com>wrote:

Yep, I will finish testing and hold off hitting merge until we hear
from Justin.
I doubt there is much good to be done here, GeoServerExtensions is
still an improvement on what was before.

Jody Garnett

On Tue, Apr 22, 2014 at 6:28 PM, Andrea Aime <
andrea.aime@anonymised.com> wrote:

On Tue, Apr 22, 2014 at 10:24 AM, Jody Garnett <
jody.garnett@anonymised.com> wrote:

I note the tension between a minimal API for development and opening
the door for ease of testing.

Here is what I am testing now:
* renamed the methods as init( name, bean ) and init(
applicationContext ).
* added a check to init(name,bean) to detect any conflict with
applicationContext
* changed the flag to isSpringContext, and the javadocs to describe
additional consistency checks

Let me know if that is sufficient.

Looks better to me. It would be nice if Justin could have a look as
well

Cheers
Andrea

--

Meet us at GEO Business 2014! in London! Visit http://goo.gl/fES3aK
for more information.

Ing. Andrea Aime
@geowolf
Technical Lead

GeoSolutions S.A.S.
Via Poggio alle Viti 1187
55054 Massarosa (LU)
Italy
phone: +39 0584 962313
fax: +39 0584 1660272
mob: +39 339 8844549

http://www.geo-solutions.it
http://twitter.com/geosolutions_it

-------------------------------------------------------

------------------------------------------------------------------------------
Start Your Social Network Today - Download eXo Platform
Build your Enterprise Intranet with eXo Platform Software
Java Based Open Source Intranet - Social, Extensible, Cloud Ready
Get Started Now And Turn Your Intranet Into A Collaboration Platform
http://p.sf.net/sfu/ExoPlatform
_______________________________________________
Geoserver-devel mailing list
Geoserver-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel

--
*Justin Deoliveira*
Vice President, Engineering | Boundless
jdeolive@anonymised.com
@j_deolive <https://twitter.com/j_deolive&gt;

--
*Justin Deoliveira*
Vice President, Engineering | Boundless
jdeolive@anonymised.com
@j_deolive <https://twitter.com/j_deolive&gt;

--
*Justin Deoliveira*
Vice President, Engineering | Boundless
jdeolive@anonymised.com
@j_deolive <https://twitter.com/j_deolive&gt;

You're not being clear about the motivation here, or at least i can't

infer it from this conversation. Can you just point me at a specific test
case that Kevin has written.

In the current codebase look for any/all examples of
GeoserverDataDirectory.setResourceLoader.

* MockCreator.createCatalog - this is the key gotcha that effects many test
cases
* DefaultGeoServerLoaderTest
* FileExistsValidatorTest
* PyCatalogModTest

Here is an example use from PyCatalogModTest.java:

Previously:

*GeoserverDataDirectory.setResourceLoader(scriptMgr.getDataDirectory().getResourceLoader());*

The replacement would be:

*GeoServerExtensions.init("resourceLoader",
scriptMgr.getDataDirectory().getResourceLoader() );*

This is due to code now using GeoServerExtensions (rather than
GeoserverDataDirectory) to look things up.

My suggestion on the pull request was just that the static calls to the GeoServerExtensions singleton might be a problem developing more unitary unit tests (I’d run into the same issue the day before) and it might be worth handling it in a way that made it easier to mock while we were changing things as long as it didn’t add to much work. I was thinking along the lines of just giving the classes protected/package getResourceLoader methods that could be overridden for the objects to be tested. A better way of doing this, or if there already is a way that hidden somewhere would be nice, but it’s not worth holding up the pull request for and I’ve no problem accepting it with the static calls in place.

···

On 22 April 2014 07:17, Jody Garnett <jody.garnett@anonymised.com> wrote:


Start Your Social Network Today - Download eXo Platform
Build your Enterprise Intranet with eXo Platform Software
Java Based Open Source Intranet - Social, Extensible, Cloud Ready
Get Started Now And Turn Your Intranet Into A Collaboration Platform
http://p.sf.net/sfu/ExoPlatform


Geoserver-devel mailing list
Geoserver-devel@anonymised.comsts.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel

Kevin Smith

Junior Software Engineer | Boundless

ksmith@anonymised.com

+1-778-785-7459

@boundlessgeo

In the current codebase look for any/all examples of GeoserverDataDirectory.setResourceLoader.

  • MockCreator.createCatalog - this is the key gotcha that effects many test cases

  • DefaultGeoServerLoaderTest

  • FileExistsValidatorTest

  • PyCatalogModTest

Here is an example use from PyCatalogModTest.java:

Previously:

GeoserverDataDirectory.setResourceLoader(scriptMgr.getDataDirectory().getResourceLoader());

The replacement would be:

GeoServerExtensions.init(“resourceLoader”, scriptMgr.getDataDirectory().getResourceLoader() );

This is due to code now using GeoServerExtensions (rather than GeoserverDataDirectory) to look things up.

You’re not being clear about the motivation here, or at least i can’t infer it from this conversation. Can you just point me at a specific test case that Kevin has written.

Thanks for the background Kevin, wish I had put this one off (or at least not tied it to the work I am doing). Note this approach is not specific to accessing GeoServerResourceLoader as we have several singletons.

I will move the static init methods to a separate testing GeoServerExtensionsSupport class so we have a way to accomplish what you would like - that does not add to the public API.

···

Jody Garnett

On Wed, Apr 23, 2014 at 5:03 AM, Kevin Smith <ksmith@anonymised.com> wrote:

My suggestion on the pull request was just that the static calls to the GeoServerExtensions singleton might be a problem developing more unitary unit tests (I’d run into the same issue the day before) and it might be worth handling it in a way that made it easier to mock while we were changing things as long as it didn’t add to much work. I was thinking along the lines of just giving the classes protected/package getResourceLoader methods that could be overridden for the objects to be tested. A better way of doing this, or if there already is a way that hidden somewhere would be nice, but it’s not worth holding up the pull request for and I’ve no problem accepting it with the static calls in place.

On 22 April 2014 07:17, Jody Garnett <jody.garnett@anonymised.com.403…> wrote:


Start Your Social Network Today - Download eXo Platform
Build your Enterprise Intranet with eXo Platform Software
Java Based Open Source Intranet - Social, Extensible, Cloud Ready
Get Started Now And Turn Your Intranet Into A Collaboration Platform
http://p.sf.net/sfu/ExoPlatform


Geoserver-devel mailing list
Geoserver-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel

Kevin Smith

Junior Software Engineer | Boundless

ksmith@anonymised.com

+1-778-785-7459

@boundlessgeo

In the current codebase look for any/all examples of GeoserverDataDirectory.setResourceLoader.

  • MockCreator.createCatalog - this is the key gotcha that effects many test cases

  • DefaultGeoServerLoaderTest

  • FileExistsValidatorTest

  • PyCatalogModTest

Here is an example use from PyCatalogModTest.java:

Previously:

GeoserverDataDirectory.setResourceLoader(scriptMgr.getDataDirectory().getResourceLoader());

The replacement would be:

GeoServerExtensions.init(“resourceLoader”, scriptMgr.getDataDirectory().getResourceLoader() );

This is due to code now using GeoServerExtensions (rather than GeoserverDataDirectory) to look things up.

You’re not being clear about the motivation here, or at least i can’t infer it from this conversation. Can you just point me at a specific test case that Kevin has written.

On Tue, Apr 22, 2014 at 2:22 PM, Jody Garnett <jody.garnett@anonymised.com>wrote:

Thanks for the background Kevin, wish I had put this one off (or at least
not tied it to the work I am doing). Note this approach is not specific to
accessing GeoServerResourceLoader as we have several singletons.

I will move the static init methods to a separate testing
GeoServerExtensionsSupport class so we have a way to accomplish what you
would like - that does not add to the public API.

Well if it's easier to throw them on GeoServerExtensions i am ok with
that... but cleaner imo would be a helper that makes it easy to mock up or
hack the application context directly and then just set that on
GeoServerExtensions. You're call.

Jody Garnett

On Wed, Apr 23, 2014 at 5:03 AM, Kevin Smith <ksmith@anonymised.com>wrote:

My suggestion on the pull request was just that the static calls to the
GeoServerExtensions singleton might be a problem developing more unitary
unit tests (I'd run into the same issue the day before) and it might be
worth handling it in a way that made it easier to mock while we were
changing things as long as it didn't add to much work. I was thinking
along the lines of just giving the classes protected/package
getResourceLoader methods that could be overridden for the objects to be
tested. A better way of doing this, or if there already is a way that
hidden somewhere would be nice, but it's not worth holding up the pull
request for and I've no problem accepting it with the static calls in place.

On 22 April 2014 07:17, Jody Garnett <jody.garnett@anonymised.com> wrote:

You're not being clear about the motivation here, or at least i can't

infer it from this conversation. Can you just point me at a specific test
case that Kevin has written.

In the current codebase look for any/all examples of
GeoserverDataDirectory.setResourceLoader.

* MockCreator.createCatalog - this is the key gotcha that effects many
test cases
* DefaultGeoServerLoaderTest
* FileExistsValidatorTest
* PyCatalogModTest

Here is an example use from PyCatalogModTest.java:

Previously:

*GeoserverDataDirectory.setResourceLoader(scriptMgr.getDataDirectory().getResourceLoader());*

The replacement would be:

*GeoServerExtensions.init("resourceLoader",
scriptMgr.getDataDirectory().getResourceLoader() );*

This is due to code now using GeoServerExtensions (rather than
GeoserverDataDirectory) to look things up.

------------------------------------------------------------------------------
Start Your Social Network Today - Download eXo Platform
Build your Enterprise Intranet with eXo Platform Software
Java Based Open Source Intranet - Social, Extensible, Cloud Ready
Get Started Now And Turn Your Intranet Into A Collaboration Platform
http://p.sf.net/sfu/ExoPlatform
_______________________________________________
Geoserver-devel mailing list
Geoserver-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel

--

Kevin Smith

Junior Software Engineer | Boundless

ksmith@anonymised.com

+1-778-785-7459

@boundlessgeo <https://twitter.com/boundlessgeo&gt;

--
*Justin Deoliveira*
Vice President, Engineering | Boundless
jdeolive@anonymised.com
@j_deolive <https://twitter.com/j_deolive&gt;

At the start of this conversation I offered the following example of using a mock application context:

GeoServerResourceLoader resourceLoader = new GeoServerResourceLoader(baseDirectory);

ApplicationContext context = createNiceMock(ApplicationContext.class);
expect(context.getBean(“resourceLoader”)).andReturn( resourceLoader );

expect(context.getBeanNamesForType(GeoServerResourceLoader.class)).andReturn(new String{“resourceLoader”}).anyTimes();
expect(context.getBeanNamesForType((Class)anyObject())).andReturn(new String{}).anyTimes();
replay(context);

The only part that is hard to understand is the anyObject → new String entry, basically saying “no” to any unknown singleton lookups.

If you can name a good place to put this boiler plate code I am fine with that.

···

Jody Garnett

On Wed, Apr 23, 2014 at 6:51 AM, Justin Deoliveira <jdeolive@anonymised.com> wrote:

On Tue, Apr 22, 2014 at 2:22 PM, Jody Garnett <jody.garnett@anonymised.com> wrote:

Thanks for the background Kevin, wish I had put this one off (or at least not tied it to the work I am doing). Note this approach is not specific to accessing GeoServerResourceLoader as we have several singletons.

I will move the static init methods to a separate testing GeoServerExtensionsSupport class so we have a way to accomplish what you would like - that does not add to the public API.

Well if it’s easier to throw them on GeoServerExtensions i am ok with that… but cleaner imo would be a helper that makes it easy to mock up or hack the application context directly and then just set that on GeoServerExtensions. You’re call.

Justin Deoliveira
Vice President, Engineering | Boundless
jdeolive@anonymised.com
@j_deolive

Jody Garnett

On Wed, Apr 23, 2014 at 5:03 AM, Kevin Smith <ksmith@anonymised.com> wrote:

My suggestion on the pull request was just that the static calls to the GeoServerExtensions singleton might be a problem developing more unitary unit tests (I’d run into the same issue the day before) and it might be worth handling it in a way that made it easier to mock while we were changing things as long as it didn’t add to much work. I was thinking along the lines of just giving the classes protected/package getResourceLoader methods that could be overridden for the objects to be tested. A better way of doing this, or if there already is a way that hidden somewhere would be nice, but it’s not worth holding up the pull request for and I’ve no problem accepting it with the static calls in place.

On 22 April 2014 07:17, Jody Garnett <jody.garnett@anonymised.com> wrote:


Start Your Social Network Today - Download eXo Platform
Build your Enterprise Intranet with eXo Platform Software
Java Based Open Source Intranet - Social, Extensible, Cloud Ready
Get Started Now And Turn Your Intranet Into A Collaboration Platform
http://p.sf.net/sfu/ExoPlatform


Geoserver-devel mailing list
Geoserver-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geoserver-devel

Kevin Smith

Junior Software Engineer | Boundless

ksmith@anonymised.com

+1-778-785-7459

@boundlessgeo

In the current codebase look for any/all examples of GeoserverDataDirectory.setResourceLoader.

  • MockCreator.createCatalog - this is the key gotcha that effects many test cases

  • DefaultGeoServerLoaderTest

  • FileExistsValidatorTest

  • PyCatalogModTest

Here is an example use from PyCatalogModTest.java:

Previously:

GeoserverDataDirectory.setResourceLoader(scriptMgr.getDataDirectory().getResourceLoader());

The replacement would be:

GeoServerExtensions.init(“resourceLoader”, scriptMgr.getDataDirectory().getResourceLoader() );

This is due to code now using GeoServerExtensions (rather than GeoserverDataDirectory) to look things up.

You’re not being clear about the motivation here, or at least i can’t infer it from this conversation. Can you just point me at a specific test case that Kevin has written.