Torben and Jody,

I have conducted a full review and have merged PR 742. While I am averse 
to last-minute changes, Jody has a compelling argument that this is an 
investment in the future ease of cherry-picking onto stable.

Review details:

- Full GeoTools builds (-Dall) with and without -Ponline both work 
(online fixtures configured for postgis).

- I searched all pom files for excludes that might be affected by the 
renaming and found none (I found one for spatialite but not an 
OnlineTestCase). I also found no other unexpected side effects of -Ponline.

- Full GeoServer build and app-schema-online-test against postgis worked 
(app-schema-test uses some GeoTools test jars).

- There are seven source files that are small enough that it seems that 
git does not recognise the renaming. We lose a tiny bit of history.

Thanks, Torben. Thorough work.

Kind regards,
Ben.

On 19/02/15 07:08, Ben Caradoc-Davies wrote:
> I agree that this is a welcome change; how refreshing that our test
> classes finally conform to our own long standing and well-thought-out
> policy.
>
> One problem is that this is an API change for any external project that
> reuses these test classes. Another consequence is that every build
> server will have its configured online tests silently disabled by this
> change because these builds may not be using -Ponline.
>
> I am running a full build to see if this has any unexpected downstream
> effects on, for example, GeoServer.
>
> I will also be delighted if ares starts running online tests. AFAIK the
> CSIRO Jenkins is the only one that does these.
>
> Kind regards,
> Ben.
>
> On 19/02/15 06:22, Jody Garnett wrote:
>> Larger picture is we have got some databases setup that ares can see and I
>> would like to set up jobs to run our online tests (seperate nightly online
>> tests using the -Ponline profile).
>>
>> I was just real surprised that the -Ponline profile was not setup for JDBC
>> tests...
>>
>> While we can put off the rename until after RC1, we then set up difficulty
>> back porting any and all test fixes (like we would no longer be able to
>> directly cherry pick changes across to the stable branch).
>>
>> So I am uncomfortable but still +1: we some reward for the risk, and make
>> it easier to back port fixes to the test cases.
>>
>>
>>
>> --
>> Jody Garnett
>>
>> On 18 February 2015 at 08:56, Torben Barsballe <tbarsba...@boundlessgeo.com>
>> wrote:
>>
>>> I aggree that this big a change (at least it is a non-functional change)
>>> is probably a bad idea right before a release, but Jody asked if I could
>>> get it done in time, so here we are.
>>>
>>> Andrea:
>>>
>>>      - According to our documentation
>>>      
>>> <http://docs.geotools.org/latest/userguide/build/maven/testing.html#online-testing>,
>>>      any tests requiring an online resurce should end in OnlineTest.java, 
>>> and
>>>      such tests will only run under the "-P online" maven profile; currently
>>>      this is not followed, hence the rename.
>>>      - If we want to have online tests running on a build box (eg. ares),
>>>      then controling them with a flag is necessary, since just having/not 
>>> having
>>>      the properties files could affect other builds that were not intended 
>>> to
>>>      run online tests (Particularily in cases of concurrent builds, or 
>>> builds
>>>      not cleaning up after themselves quickly enough. The primary reason for
>>>      this change is to support doing automated online tests.
>>>
>>> Torben
>>>
>>> On Wed, Feb 18, 2015 at 1:40 AM, Christian Mueller <
>>> christian.muel...@os-solutions.at> wrote:
>>>
>>>> Hi
>>>>
>>>> The drivers for DB2 have to be installed manually too.
>>>>
>>>> Cheers
>>>> Christian
>>>>
>>>> On Wed, Feb 18, 2015 at 5:54 AM, Andrea Aime <
>>>> andrea.a...@geo-solutions.it> wrote:
>>>>
>>>>> I'm in the "last minute change hater's club" too.  What's the damage in
>>>>> doing it on trunk only after the cut,  and backport later,  or just do not
>>>>> backport at all if those are considered API? (arguably,  the base classes
>>>>> are API as any JDBC store depends on them)
>>>>>
>>>>> Cheers
>>>>> Andrea
>>>>> Il 18/feb/2015 05:29 "Ben Caradoc-Davies" <b...@transient.nz> ha scritto:
>>>>>
>>>>> Torben,
>>>>>>
>>>>>> I am not convinced that it is a good idea to rename 291 classes on the
>>>>>> eve of an RC1 release. We are in a freeze, after all. This proposal
>>>>>> certainly merits discussion.
>>>>>>
>>>>>> Kind regards,
>>>>>> Ben.
>>>>>>
>>>>>> On 18/02/15 14:29, Torben Barsballe wrote:
>>>>>>> Here is a pull request that updates *all* classes inheriting from
>>>>>>> OnlineTest to end in OnlineTest.java:
>>>>>>> https://github.com/geotools/geotools/pull/742
>>>>>>> Hopefully this is still in time for RC1.
>>>>>>>
>>>>>>> Torben
>>>>>>>
>>>>>>> On Tue, Feb 17, 2015 at 2:47 PM, Torben Barsballe <
>>>>>>> tbarsba...@boundlessgeo.com> wrote:
>>>>>>>
>>>>>>>> Renaming all database tests to end in OnlineTest is easy, and can
>>>>>> get done
>>>>>>>> in time for RC1.
>>>>>>>>
>>>>>>>> We should probably also look and see if there are other "Online"
>>>>>> tests
>>>>>>>> that should be renamed, but this will likely require a bit of
>>>>>> digging and
>>>>>>>> probably won't be ready for RC1.
>>>>>>>>
>>>>>>>> Torben
>>>>>>>>
>>>>>>>> On Tue, Feb 17, 2015 at 2:24 PM, Jody Garnett <
>>>>>> jody.garn...@gmail.com>
>>>>>>>> wrote:
>>>>>>>>
>>>>>>>>> Good research, that will be quite the pull request :) Can we get it
>>>>>> done
>>>>>>>>> in time for RC1?
>>>>>>>>>
>>>>>>>>> --
>>>>>>>>> Jody Garnett
>>>>>>>>>
>>>>>>>>> On 17 February 2015 at 14:07, Torben Barsballe <
>>>>>>>>> tbarsba...@boundlessgeo.com> wrote:
>>>>>>>>>
>>>>>>>>>> I am working on getting online database tests set up and runing on
>>>>>>>>>> ares/Jenkins.
>>>>>>>>>> According to our documentation
>>>>>>>>>> <
>>>>>> http://docs.geotools.org/latest/userguide/build/maven/testing.html#online-testing
>>>>>>> ,
>>>>>>>>>> any tests requiring an online resurce should end in
>>>>>> OnlineTest.java, and
>>>>>>>>>> will only run under the "-P online" maven profile.
>>>>>>>>>>
>>>>>>>>>> It turns out that all of our database tests do not follow this
>>>>>> naming
>>>>>>>>>> convention, and instead derive their name from the jdbc test class
>>>>>> which
>>>>>>>>>> they extend.
>>>>>>>>>>
>>>>>>>>>> This means that the database tests will run as long as you have the
>>>>>>>>>> appropriate database fixture your ~/.geotools directory, completely
>>>>>>>>>> independant of the maven "online" profile.
>>>>>>>>>>
>>>>>>>>>> In order to properly support enabling/disabling online tests, it
>>>>>> seems
>>>>>>>>>> like it will be necessary to append OnlineTest to the name of all
>>>>>> tests
>>>>>>>>>> which require online resources...
>>>>>>>>>>
>>>>>>>>>> Torben
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>> ------------------------------------------------------------------------------
>>>>>>>>>> Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
>>>>>>>>>> from Actuate! Instantly Supercharge Your Business Reports and
>>>>>> Dashboards
>>>>>>>>>> with Interactivity, Sharing, Native Excel Exports, App Integration
>>>>>> & more
>>>>>>>>>> Get technology previously reserved for billion-dollar
>>>>>> corporations, FREE
>>>>>>>>>>
>>>>>>>>>>
>>>>>> http://pubads.g.doubleclick.net/gampad/clk?id=190641631&iu=/4140/ostg.clktrk
>>>>>>>>>> _______________________________________________
>>>>>>>>>> GeoTools-Devel mailing list
>>>>>>>>>> GeoTools-Devel@lists.sourceforge.net
>>>>>>>>>> https://lists.sourceforge.net/lists/listinfo/geotools-devel
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>> ------------------------------------------------------------------------------
>>>>>>> Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
>>>>>>> from Actuate! Instantly Supercharge Your Business Reports and
>>>>>> Dashboards
>>>>>>> with Interactivity, Sharing, Native Excel Exports, App Integration &
>>>>>> more
>>>>>>> Get technology previously reserved for billion-dollar corporations,
>>>>>> FREE
>>>>>>>
>>>>>> http://pubads.g.doubleclick.net/gampad/clk?id=190641631&iu=/4140/ostg.clktrk
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> _______________________________________________
>>>>>>> GeoTools-Devel mailing list
>>>>>>> GeoTools-Devel@lists.sourceforge.net
>>>>>>> https://lists.sourceforge.net/lists/listinfo/geotools-devel
>>>>>>>
>>>>>>
>>>>>> --
>>>>>> Ben Caradoc-Davies <b...@transient.nz>
>>>>>> Software Engineer
>>>>>> Transient Software <http://transient.nz>
>>>>>> New Zealand
>>>>>>
>>>>>>
>>>>>> ------------------------------------------------------------------------------
>>>>>> Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
>>>>>> from Actuate! Instantly Supercharge Your Business Reports and Dashboards
>>>>>> with Interactivity, Sharing, Native Excel Exports, App Integration &
>>>>>> more
>>>>>> Get technology previously reserved for billion-dollar corporations, FREE
>>>>>>
>>>>>> http://pubads.g.doubleclick.net/gampad/clk?id=190641631&iu=/4140/ostg.clktrk
>>>>>> _______________________________________________
>>>>>> GeoTools-Devel mailing list
>>>>>> GeoTools-Devel@lists.sourceforge.net
>>>>>> https://lists.sourceforge.net/lists/listinfo/geotools-devel
>>>>>>
>>>>>
>>>>>
>>>>> ------------------------------------------------------------------------------
>>>>> Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
>>>>> from Actuate! Instantly Supercharge Your Business Reports and Dashboards
>>>>> with Interactivity, Sharing, Native Excel Exports, App Integration & more
>>>>> Get technology previously reserved for billion-dollar corporations, FREE
>>>>>
>>>>> http://pubads.g.doubleclick.net/gampad/clk?id=190641631&iu=/4140/ostg.clktrk
>>>>> _______________________________________________
>>>>> GeoTools-Devel mailing list
>>>>> GeoTools-Devel@lists.sourceforge.net
>>>>> https://lists.sourceforge.net/lists/listinfo/geotools-devel
>>>>>
>>>>>
>>>>
>>>>
>>>> --
>>>> DI Christian Mueller MSc (GIS), MSc (IT-Security)
>>>> OSS Open Source Solutions GmbH
>>>>
>>>>
>>>>
>>>> ------------------------------------------------------------------------------
>>>> Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
>>>> from Actuate! Instantly Supercharge Your Business Reports and Dashboards
>>>> with Interactivity, Sharing, Native Excel Exports, App Integration & more
>>>> Get technology previously reserved for billion-dollar corporations, FREE
>>>>
>>>> http://pubads.g.doubleclick.net/gampad/clk?id=190641631&iu=/4140/ostg.clktrk
>>>> _______________________________________________
>>>> GeoTools-Devel mailing list
>>>> GeoTools-Devel@lists.sourceforge.net
>>>> https://lists.sourceforge.net/lists/listinfo/geotools-devel
>>>>
>>>>
>>>
>>>
>>> ------------------------------------------------------------------------------
>>> Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
>>> from Actuate! Instantly Supercharge Your Business Reports and Dashboards
>>> with Interactivity, Sharing, Native Excel Exports, App Integration & more
>>> Get technology previously reserved for billion-dollar corporations, FREE
>>>
>>> http://pubads.g.doubleclick.net/gampad/clk?id=190641631&iu=/4140/ostg.clktrk
>>> _______________________________________________
>>> GeoTools-Devel mailing list
>>> GeoTools-Devel@lists.sourceforge.net
>>> https://lists.sourceforge.net/lists/listinfo/geotools-devel
>>>
>>>
>>
>>
>>
>> ------------------------------------------------------------------------------
>> Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
>> from Actuate! Instantly Supercharge Your Business Reports and Dashboards
>> with Interactivity, Sharing, Native Excel Exports, App Integration & more
>> Get technology previously reserved for billion-dollar corporations, FREE
>> http://pubads.g.doubleclick.net/gampad/clk?id=190641631&iu=/4140/ostg.clktrk
>>
>>
>>
>> _______________________________________________
>> GeoTools-Devel mailing list
>> GeoTools-Devel@lists.sourceforge.net
>> https://lists.sourceforge.net/lists/listinfo/geotools-devel
>>
>

-- 
Ben Caradoc-Davies <b...@transient.nz>
Software Engineer
Transient Software <http://transient.nz>
New Zealand

------------------------------------------------------------------------------
Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
from Actuate! Instantly Supercharge Your Business Reports and Dashboards
with Interactivity, Sharing, Native Excel Exports, App Integration & more
Get technology previously reserved for billion-dollar corporations, FREE
http://pubads.g.doubleclick.net/gampad/clk?id=190641631&iu=/4140/ostg.clktrk
_______________________________________________
GeoTools-Devel mailing list
GeoTools-Devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/geotools-devel

Reply via email to