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