Re: [HACKERS] no test programs in contrib

[Michael Paquier]

On Sat, Nov 29, 2014 at 5:54 AM, Alvaro Herrera
 wrote:
> Peter Eisentraut wrote:
>> On 11/27/14 3:10 PM, Tom Lane wrote:
>> > I'm not too happy with that approach, because packagers are going to
>> > tend to think they should package any files installed by install-world.
>> > The entire point of this change, IMO, is that the test modules should
>> > *not* get installed, certainly not by normal install targets.  Being
>> > consistent with the existing contrib packaging is exactly not what we
>> > want.
>>
>> I share this objection.
>
> Okay, the attached version does it that way.
>
> I also attach some changes for the MSVC build stuff.  I tested it and it
> builds fine AFAICT, but it doesn't install because Install.pm wants to
> install contrib modules from contrib/ (which seems reasonable) but my
> hack adds the src/test/modules/ as contrib modules also, so Install.pm
> goes bonkers.  I'm not even sure *what* we're supposed to build -- there
> is no distinction in these programs as there is in the makefiles about
> what to install.  So if some Windows developer can look into this, I'd
> appreciate it.
>
>> But the existing main regression tests are able to run against an
>> existing installation while using the modules autoinc.so and refint.so
>> without installing them.  I think the problem is that we are able to
>> load a .so file from just about anywhere, but we can't load a full
>> extension in the same way.  There have been discussions about that, in
>> the context of being able to test an externally developed extension
>> before/without installing it.  This is pretty much the same case.
>
> I'm leaving that problem for someone else to solve.
>
> Andres Freund wrote:
>> On 2014-11-27 15:51:51 -0500, Tom Lane wrote:
>> >
>> > If we follow that reasoning we'll end up removing nothing from contrib.
>> > There is no reason that end users should need to be performing such
>> > testing; anyone who does have reason to do it will have a source tree
>> > at hand.
>>
>> Actually I don't think that's true for test_decoding - there's quite a
>> bit of actual things that you can do with it. At the very least it's
>> useful to roughly measure the impact logical replication would have on a
>> database, but it's also helpful to look at the changes. And even if the
>> format isn't super nice, thanks to Robert's insistence it's actually
>> safely parseable if necessary.
>
> Argh.  Okay, the attached doesn't move test_decoding either.
>
> I think it's fine anyway -- I'm sure we will come up with a few
> additional test modules, such as the one for the commit_ts patch.
When src/test/modules is compiled directly after running ./configure,
make complains about utils/errcodes.h missing:
In file included from worker_spi.c:23:
In file included from ../../../../src/include/postgres.h:48:
../../../../src/include/utils/elog.h:69:10: fatal error:
'utils/errcodes.h' file not found
#include "utils/errcodes.h"

Shouldn't src/test/modules/Makefile includes .PHONY with
submake-errcodes like for example in the patch attached?
Regards,
-- 
Michael
diff --git a/src/test/modules/Makefile b/src/test/modules/Makefile
index 93d93af..7ca3eb0 100644
--- a/src/test/modules/Makefile
+++ b/src/test/modules/Makefile
@@ -11,6 +11,7 @@ SUBDIRS = \
 		  test_shm_mq \
 		  test_parser
 
+.PHONY: submake-errcodes
 all: submake-errcodes
 
 submake-errcodes:

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] no test programs in contrib

[Alvaro Herrera]

Michael Paquier wrote:

> It would be good to be consistent on Windows with what is now done on other
> platforms: those modules should not be installed by default, but it would
> be good to make install.pm a bit smarter with for example an option "full",
> aka install server + client + test modules. Building them is worth it in
> any case as they can be used with modulescheck.

Sure, I agree with that.

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] no test programs in contrib

[Michael Paquier]

On Sat, Nov 29, 2014 at 5:54 AM, Alvaro Herrera 
wrote:
>
> I also attach some changes for the MSVC build stuff.  I tested it and it
> builds fine AFAICT, but it doesn't install because Install.pm wants to
> install contrib modules from contrib/ (which seems reasonable) but my
> hack adds the src/test/modules/ as contrib modules also, so Install.pm
> goes bonkers.  I'm not even sure *what* we're supposed to build -- there
> is no distinction in these programs as there is in the makefiles about
> what to install.  So if some Windows developer can look into this, I'd
> appreciate it.
>

It would be good to be consistent on Windows with what is now done on other
platforms: those modules should not be installed by default, but it would
be good to make install.pm a bit smarter with for example an option "full",
aka install server + client + test modules. Building them is worth it in
any case as they can be used with modulescheck.
My 2c.
-- 
Michael

Re: [alvhe...@2ndquadrant.com: Re: [HACKERS] no test programs in contrib]

[Andrew Dunstan]

On 12/17/2014 01:02 PM, Tom Lane wrote:

Andrew Dunstan  writes:

On 12/17/2014 11:34 AM, Andrew Dunstan wrote:

Oh, darn, I thought we had a version check. Will fix.

OK, I have committed a fix. Revised script is at


Pulled into dromedary, thanks.


I have set crake to do runs that should clear the errors:
 cd root && for f in REL*; do touch $f/crake.force-one-run; done

Cool, was just wondering what was the easiest way to force that.
Maybe this should be documented on the buildfarm how-to page?




Done.

cheers

andrew


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [alvhe...@2ndquadrant.com: Re: [HACKERS] no test programs in contrib]

[Tom Lane]

Andrew Dunstan  writes:
> On 12/17/2014 11:34 AM, Andrew Dunstan wrote:
>> Oh, darn, I thought we had a version check. Will fix.

> OK, I have committed a fix. Revised script is at 
> 

Pulled into dromedary, thanks.

> I have set crake to do runs that should clear the errors:
> cd root && for f in REL*; do touch $f/crake.force-one-run; done

Cool, was just wondering what was the easiest way to force that.
Maybe this should be documented on the buildfarm how-to page?

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [alvhe...@2ndquadrant.com: Re: [HACKERS] no test programs in contrib]

[Andrew Dunstan]

On 12/17/2014 11:34 AM, Andrew Dunstan wrote:


On 12/17/2014 10:23 AM, Tom Lane wrote:

Andrew Dunstan  writes:

On 12/16/2014 04:44 PM, Tom Lane wrote:
I've put this in dromedary as well (though the HEAD build that's 
running
right this moment is still using the 4.13 script).  I take it I 
don't need

to adjust the configuration file?

Nope, no config changes required.

As seems blindingly obvious in hindsight, both crake and dromedary are
now red in every branch but HEAD.





Oh, darn, I thought we had a version check. Will fix.





OK, I have committed a fix. Revised script is at 



I have set crake to do runs that should clear the errors:

   cd root && for f in REL*; do touch $f/crake.force-one-run; done

cheers

andrew



--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [alvhe...@2ndquadrant.com: Re: [HACKERS] no test programs in contrib]

[Andrew Dunstan]

On 12/17/2014 10:23 AM, Tom Lane wrote:

Andrew Dunstan  writes:

On 12/16/2014 04:44 PM, Tom Lane wrote:

I've put this in dromedary as well (though the HEAD build that's running
right this moment is still using the 4.13 script).  I take it I don't need
to adjust the configuration file?

Nope, no config changes required.

As seems blindingly obvious in hindsight, both crake and dromedary are
now red in every branch but HEAD.





Oh, darn, I thought we had a version check. Will fix.

cheers

andrew






--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [alvhe...@2ndquadrant.com: Re: [HACKERS] no test programs in contrib]

[Tom Lane]

Andrew Dunstan  writes:
> On 12/16/2014 04:44 PM, Tom Lane wrote:
>> I've put this in dromedary as well (though the HEAD build that's running
>> right this moment is still using the 4.13 script).  I take it I don't need
>> to adjust the configuration file?

> Nope, no config changes required.

As seems blindingly obvious in hindsight, both crake and dromedary are
now red in every branch but HEAD.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [alvhe...@2ndquadrant.com: Re: [HACKERS] no test programs in contrib]

[Alvaro Herrera]

Michael Paquier wrote:
> On Wed, Dec 17, 2014 at 6:02 AM, Alvaro Herrera 
> wrote:
> >
> > Andrew Dunstan wrote:

> > > Any brave buildfarm owners on *nix can try it by replacing their copy of
> > > run_build.pl with the bleeding edge version. We can't put it in a client
> > > release until we fix up the MSVC side of things.
> >
> > I guess I will have to find someone to assist me in architecting a
> > solution for the MSVC stuff.
> >
> What's the matter here? Do we need to extend vcregress.pl with a new mode
> to test stuff in src/test/modules?

Please see my message
http://www.postgresql.org/message-id/20141128205453.ga1...@alvh.no-ip.org

The -msvc patch attached there adds some support to Mkvcbuild.pm and
vcregress.pl, but it doesn't completely work (fails to install).

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [alvhe...@2ndquadrant.com: Re: [HACKERS] no test programs in contrib]

[Michael Paquier]

On Wed, Dec 17, 2014 at 6:02 AM, Alvaro Herrera 
wrote:
>
> Andrew Dunstan wrote:
>
> > I have pushed this change, and crake will be running the code. See <
> https://github.com/PGBuildFarm/client-code/commit/d656c1c3ce46f290791c5ba5ede2f8ac8dfa342e
> >
>
> Crake just uploaded its first test results with the testmodules stuff
> working:
>
> http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=crake&dt=2014-12-16%2020%3A46%3A04
>
> Thanks for setting it up.
>
> > Any brave buildfarm owners on *nix can try it by replacing their copy of
> > run_build.pl with the bleeding edge version. We can't put it in a client
> > release until we fix up the MSVC side of things.
>
> I guess I will have to find someone to assist me in architecting a
> solution for the MSVC stuff.
>
What's the matter here? Do we need to extend vcregress.pl with a new mode
to test stuff in src/test/modules?
-- 
Michael

Re: [alvhe...@2ndquadrant.com: Re: [HACKERS] no test programs in contrib]

[Andrew Dunstan]

On 12/16/2014 04:44 PM, Tom Lane wrote:

Andrew Dunstan  writes:

I have pushed this change, and crake will be running the code. See

Any brave buildfarm owners on *nix can try it by replacing their copy of
run_build.pl with the bleeding edge version. We can't put it in a client
release until we fix up the MSVC side of things.

I've put this in dromedary as well (though the HEAD build that's running
right this moment is still using the 4.13 script).  I take it I don't need
to adjust the configuration file?


Nope, no config changes required.

cheers

andrew


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [alvhe...@2ndquadrant.com: Re: [HACKERS] no test programs in contrib]

[Tom Lane]

Andrew Dunstan  writes:
> I have pushed this change, and crake will be running the code. See 
> 
>  
> Any brave buildfarm owners on *nix can try it by replacing their copy of 
> run_build.pl with the bleeding edge version. We can't put it in a client 
> release until we fix up the MSVC side of things.

I've put this in dromedary as well (though the HEAD build that's running
right this moment is still using the 4.13 script).  I take it I don't need
to adjust the configuration file?

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [alvhe...@2ndquadrant.com: Re: [HACKERS] no test programs in contrib]

[Andrew Dunstan]

On 12/16/2014 04:02 PM, Alvaro Herrera wrote:

Andrew Dunstan wrote:


I have pushed this change, and crake will be running the code. See 


Crake just uploaded its first test results with the testmodules stuff
working:
http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=crake&dt=2014-12-16%2020%3A46%3A04

Thanks for setting it up.


Any brave buildfarm owners on *nix can try it by replacing their copy of
run_build.pl with the bleeding edge version. We can't put it in a client
release until we fix up the MSVC side of things.

I guess I will have to find someone to assist me in architecting a
solution for the MSVC stuff.



I might be able to help in a couple of days.

cheers

andrew


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [alvhe...@2ndquadrant.com: Re: [HACKERS] no test programs in contrib]

[Alvaro Herrera]

Andrew Dunstan wrote:

> I have pushed this change, and crake will be running the code. See 
> 

Crake just uploaded its first test results with the testmodules stuff
working:
http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=crake&dt=2014-12-16%2020%3A46%3A04

Thanks for setting it up.

> Any brave buildfarm owners on *nix can try it by replacing their copy of
> run_build.pl with the bleeding edge version. We can't put it in a client
> release until we fix up the MSVC side of things.

I guess I will have to find someone to assist me in architecting a
solution for the MSVC stuff.

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [alvhe...@2ndquadrant.com: Re: [HACKERS] no test programs in contrib]

[Andrew Dunstan]

On 12/16/2014 11:22 AM, Andrew Dunstan wrote:




On 12/16/2014 09:31 AM, Alvaro Herrera wrote:

Hi Andrew,

Did you have a chance to review this?




Oh, darn, not yet. I will try to take a look today.



I have pushed this change, and crake will be running the code. See 
 
Any brave buildfarm owners on *nix can try it by replacing their copy of 
run_build.pl with the bleeding edge version. We can't put it in a client 
release until we fix up the MSVC side of things.


cheers

andrew





--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [alvhe...@2ndquadrant.com: Re: [HACKERS] no test programs in contrib]

[Andrew Dunstan]

On 12/16/2014 09:31 AM, Alvaro Herrera wrote:

Hi Andrew,

Did you have a chance to review this?




Oh, darn, not yet. I will try to take a look today.


cheers

andrew


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [alvhe...@2ndquadrant.com: Re: [HACKERS] no test programs in contrib]

[Alvaro Herrera]

Hi Andrew,

Did you have a chance to review this?

Alvaro Herrera wrote:
> Andrew Dunstan wrote:
> > 
> > On 11/29/2014 10:09 PM, Alvaro Herrera wrote:

> > >Anyway I just pushed this src/test/modules/ patch, which has
> > >implications for buildfarm: these new test modules are not invoked
> > >except explicitely.  How would go about getting members to run "cd
> > >src/test/modules ; make check ; make installcheck"?  I imagine it's
> > >impossible to do it unless each member maintainer update the buildfarm
> > >client script, right?
> > 
> > Yes.
> > 
> > Why are we going to run both check and installcheck? And what output files
> > are created? The buildfarm will need to know.
> 
> Well, initially the patch moved test_decoding to src/test/modules, which
> requires make check, but I left that in contrib due to complaints, and
> all remaining modules are happy to use make installcheck.  Attached is a
> patch to run_build.pl that adds src/test/modules build, install and
> check.  I also added the vcregress call (just copied it from the contrib
> one, really), but of course that doesn't work at all yet since MSVC
> doesn't build it.  Would you give it a look?  I would like to have
> buildfarm doing this before moving on with more stuff.



> diff --git a/run_build.pl b/run_build.pl
> index a358d9c..77fcf62 100755
> --- a/run_build.pl
> +++ b/run_build.pl
> @@ -665,6 +665,8 @@ make_bin_check();
>  # contrib is builtunder standard build step for msvc
>  make_contrib() unless ($using_msvc);
>  
> +make_testmodules();
> +
>  make_doc() if (check_optional_step('build_docs'));
>  
>  make_install();
> @@ -672,6 +674,8 @@ make_install();
>  # contrib is installed under standard install for msvc
>  make_contrib_install() unless ($using_msvc);
>  
> +make_testmodules_install();
> +
>  process_module_hooks('configure');
>  
>  process_module_hooks('build');
> @@ -753,6 +757,19 @@ foreach my $locale (@locales)
>  make_contrib_install_check($locale);
>  }
>  
> + if (step_wanted('testmodules-install-check'))
> +{
> + print time_str(),"restarting db ($locale)...\n" if $verbose;
> +
> + stop_db($locale);
> + start_db($locale);
> +
> +print time_str(),"running make test-modules installcheck 
> ($locale)...\n"
> +  if $verbose;
> +
> +make_testmodules_install_check($locale);
> +}
> +
>  print time_str(),"stopping db ($locale)...\n" if $verbose;
>  
>  stop_db($locale);
> @@ -1062,6 +1079,22 @@ sub make_contrib
>  $steps_completed .= " Contrib";
>  }
>  
> +sub make_testmodules
> +{
> + return unless step_wanted('testmodules');
> + print time_str(),"running make src/test/modules ...\n" if $verbose;
> +
> + my $make_cmd = $make;
> + $make_cmd = "$make -j $make_jobs"
> +   if ($make_jobs > 1 && ($branch eq 'HEAD' || $branch ge 'REL9_1'));
> + my @makeout = `cd $pgsql/src/test/modules && $make_cmd 2>&1`;
> + my $status = $? >> 8;
> + writelog('make-testmodules',\@makeout);
> +print " make testmodules log ===\n",@makeout if 
> ($verbose > 1);
> + send_result('TestModules',$status,\@makeout) if $status;
> + $steps_completed .= " TestModules";
> +}
> +
>  sub make_contrib_install
>  {
>  return
> @@ -1081,6 +1114,23 @@ sub make_contrib_install
>  $steps_completed .= " ContribInstall";
>  }
>  
> +sub make_testmodules_install
> +{
> + return
> +   unless (step_wanted('testmodules')
> + and step_wanted('install'));
> + print time_str(),"running make testmodules install ...\n"
> +   if $verbose;
> +
> + my @makeout = `cd $pgsql/src/test/modules && $make install 2>&1`;
> + my $status = $? >>8;
> + writelog('install-testmodules',\@makeout);
> +print " make testmodules install log ===\n",@makeout
> +   if ($verbose > 1);
> + send_result('TestModulesInstall',$status,\@makeout) if $status;
> + $steps_completed .= " TestModulesInstall";
> +}
> +
>  sub initdb
>  {
>  my $locale = shift;
> @@ -1317,6 +1367,50 @@ sub make_contrib_install_check
>  $steps_completed .= " ContribCheck-$locale";
>  }
>  
> +sub make_testmodules_install_check
> +{
> + my $locale = shift;
> +return unless step_wanted('testmodules-install-check');
> +my @checklog;
> +unless ($using_msvc)
> +{
> +@checklog =
> +  `cd $pgsql/src/test/modules && $make USE_MODULE_DB=1 installcheck 
> 2>&1`;
> +}
> +else
> +{
> +chdir "$pgsql/src/tools/msvc";
> +@checklog = `perl vcregress.pl modulescheck 2>&1`;
> +chdir $branch_root;
> +}
> +my $status = $? >>8;
> +my @logs = glob("$pgsql/src/test/modules/*/regression.diffs");
> +push(@logs,"$installdir/logfile");
> +foreach my $logfile (@logs)
> +{
> +next unless (-e $logfile);
> +push(@checklog,"\n\n= $logfile 
> ===\n");
> +my $handle;
> +   

Re: [HACKERS] no test programs in contrib

[Alvaro Herrera]

Peter Eisentraut wrote:
> On 11/27/14 3:10 PM, Tom Lane wrote:
> > I'm not too happy with that approach, because packagers are going to
> > tend to think they should package any files installed by install-world.
> > The entire point of this change, IMO, is that the test modules should
> > *not* get installed, certainly not by normal install targets.  Being
> > consistent with the existing contrib packaging is exactly not what we
> > want.
> 
> I share this objection.

Okay, the attached version does it that way.

I also attach some changes for the MSVC build stuff.  I tested it and it
builds fine AFAICT, but it doesn't install because Install.pm wants to
install contrib modules from contrib/ (which seems reasonable) but my
hack adds the src/test/modules/ as contrib modules also, so Install.pm
goes bonkers.  I'm not even sure *what* we're supposed to build -- there
is no distinction in these programs as there is in the makefiles about
what to install.  So if some Windows developer can look into this, I'd
appreciate it.

> But the existing main regression tests are able to run against an
> existing installation while using the modules autoinc.so and refint.so
> without installing them.  I think the problem is that we are able to
> load a .so file from just about anywhere, but we can't load a full
> extension in the same way.  There have been discussions about that, in
> the context of being able to test an externally developed extension
> before/without installing it.  This is pretty much the same case.

I'm leaving that problem for someone else to solve.

Andres Freund wrote:
> On 2014-11-27 15:51:51 -0500, Tom Lane wrote:
> > 
> > If we follow that reasoning we'll end up removing nothing from contrib.
> > There is no reason that end users should need to be performing such
> > testing; anyone who does have reason to do it will have a source tree
> > at hand.
> 
> Actually I don't think that's true for test_decoding - there's quite a
> bit of actual things that you can do with it. At the very least it's
> useful to roughly measure the impact logical replication would have on a
> database, but it's also helpful to look at the changes. And even if the
> format isn't super nice, thanks to Robert's insistence it's actually
> safely parseable if necessary.

Argh.  Okay, the attached doesn't move test_decoding either.

I think it's fine anyway -- I'm sure we will come up with a few
additional test modules, such as the one for the commit_ts patch.

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
commit be4885b5ee0308909bf896298b47acbf84b38606
Author: Alvaro Herrera 
Date:   Fri Nov 28 17:41:29 2014 -0300

Move test modules from contrib to src/test/modules

This is advance preparation for introducing even more test modules; the
easy solution is to add them to contrib, but that's bloated enough that
it seems a good time to think of something different.

Moved modules are dummy_seclabel, test_shm_mq, test_parser and
worker_spi.

(test_decoding was also a candidate, but there was too much opposition
to moving that one.  We can always reconsider later.)

diff --git a/contrib/Makefile b/contrib/Makefile
index b37d0dd..195d447 100644
--- a/contrib/Makefile
+++ b/contrib/Makefile
@@ -16,7 +16,6 @@ SUBDIRS = \
 		dblink		\
 		dict_int	\
 		dict_xsyn	\
-		dummy_seclabel	\
 		earthdistance	\
 		file_fdw	\
 		fuzzystrmatch	\
@@ -51,12 +50,9 @@ SUBDIRS = \
 		tablefunc	\
 		tcn		\
 		test_decoding	\
-		test_parser	\
-		test_shm_mq	\
 		tsearch2	\
 		unaccent	\
-		vacuumlo	\
-		worker_spi
+		vacuumlo
 
 ifeq ($(with_openssl),yes)
 SUBDIRS += sslinfo
diff --git a/doc/src/sgml/contrib.sgml b/doc/src/sgml/contrib.sgml
index ec68f10..a698d0f 100644
--- a/doc/src/sgml/contrib.sgml
+++ b/doc/src/sgml/contrib.sgml
@@ -113,7 +113,6 @@ CREATE EXTENSION module_name FROM unpackaged;
  &dblink;
  &dict-int;
  &dict-xsyn;
- &dummy-seclabel;
  &earthdistance;
  &file-fdw;
  &fuzzystrmatch;
@@ -141,8 +140,6 @@ CREATE EXTENSION module_name FROM unpackaged;
  &tablefunc;
  &tcn;
  &test-decoding;
- &test-parser;
- &test-shm-mq;
  &tsearch2;
  &unaccent;
  &uuid-ossp;
diff --git a/doc/src/sgml/dummy-seclabel.sgml b/doc/src/sgml/dummy-seclabel.sgml
deleted file mode 100644
index d064705..000
--- a/doc/src/sgml/dummy-seclabel.sgml
+++ /dev/null
@@ -1,74 +0,0 @@
-
-
-
- dummy_seclabel
-
- 
-  dummy_seclabel
- 
-
- 
-  The dummy_seclabel module exists only to support regression
-  testing of the SECURITY LABEL statement.  It is not intended
-  to be used in production.
- 
-
- 
-  Rationale
-
-  
-   The SECURITY LABEL statement allows the user to assign security
-   labels to database objects; however, security labels can only be assigned
-   when specifically allowed by a loadable module, so this module is provided
-   to allow proper regression testing.
-  
-
-  
-   Security label providers intended to be used in production will 

Re: [HACKERS] no test programs in contrib

[Josh Berkus]

On 11/27/2014 12:51 PM, Tom Lane wrote:
>> So test_decoding is fairly useful for users demonstrating that decoding
>> > works, especially if they're also testing an external decoding module
>> > and are unsure of where their replication problem is located, or what's
>> > wrong with their HBA settings.  For that reason it's important that
>> > test_decoding be available via OS packages, which would give me some
>> > reluctance to move it out of /contrib.
> If we follow that reasoning we'll end up removing nothing from contrib.
> There is no reason that end users should need to be performing such
> testing; anyone who does have reason to do it will have a source tree
> at hand.

1) Decoding extension "Slony-II", installed from PGXN, won't connect
2) Install test_decoding
3) Check if decoding is working and you can connect

That's the scenario I'm looking at.  It's useful for "is there something
wrong with my decoding setup or is the decoding plugin broken?"

And I can imagine quite a few users who don't have source installs
needing to check that.  That doesn't mean test_decoding needs to stay in
contrib, just that it needs to be somewhere which goes into some common
package.

-- 
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] no test programs in contrib

[Andres Freund]

On 2014-11-27 15:51:51 -0500, Tom Lane wrote:
> Josh Berkus  writes:
> > So test_decoding is fairly useful for users demonstrating that decoding
> > works, especially if they're also testing an external decoding module
> > and are unsure of where their replication problem is located, or what's
> > wrong with their HBA settings.  For that reason it's important that
> > test_decoding be available via OS packages, which would give me some
> > reluctance to move it out of /contrib.
> 
> If we follow that reasoning we'll end up removing nothing from contrib.
> There is no reason that end users should need to be performing such
> testing; anyone who does have reason to do it will have a source tree
> at hand.

Actually I don't think that's true for test_decoding - there's quite a
bit of actual things that you can do with it. At the very least it's
useful to roughly measure the impact logical replication would have on a
database, but it's also helpful to look at the changes. And even if the
format isn't super nice, thanks to Robert's insistence it's actually
safely parseable if necessary.

> > dummy_seclabel might serve the same purpose for users who are having
> > issues with SEPostgres etc.  I don't know enough about it ...
> 
> And as for dummy_seclabel, the same applies in spades, considering
> that the number of users of SEPostgres can probably be counted without
> running out of fingers.

I agree that dummy_seclabel really doesn't have any applications besides
regression tests.

Greetings,

Andres Freund

-- 
 Andres Freund http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] no test programs in contrib

[Peter Eisentraut]

On 11/27/14 3:10 PM, Tom Lane wrote:
> I'm not too happy with that approach, because packagers are going to
> tend to think they should package any files installed by install-world.
> The entire point of this change, IMO, is that the test modules should
> *not* get installed, certainly not by normal install targets.  Being
> consistent with the existing contrib packaging is exactly not what we
> want.

I share this objection.

> Maybe we should only allow check-world to run these tests, and not
> installcheck-world?  That's kind of annoying, but what you are
> doing now seems to defeat the purpose altogether.

Maybe this will have to do for now.

But the existing main regression tests are able to run against an
existing installation while using the modules autoinc.so and refint.so
without installing them.  I think the problem is that we are able to
load a .so file from just about anywhere, but we can't load a full
extension in the same way.  There have been discussions about that, in
the context of being able to test an externally developed extension
before/without installing it.  This is pretty much the same case.




-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] no test programs in contrib

[Kouhei Kaigai]

> -Original Message-
> From: pgsql-hackers-ow...@postgresql.org
> [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Josh Berkus
> Sent: Friday, November 28, 2014 5:48 AM
> To: Alvaro Herrera; Pg Hackers
> Subject: Re: [HACKERS] no test programs in contrib
> 
> On 11/24/2014 05:49 AM, Alvaro Herrera wrote:
> > test_parser (a toy text search parser, added in 2007) dummy_seclabel
> > (for SECURITY LABEL regression testing, added Sept 2010) worker_spi
> > (for bgworkers, added Dec 2012) test_shm_mq (test program for shared
> > memory queues, added Jan 2014) test_decoding (test program for logical
> > decoding, added March 2014)
> 
> So test_decoding is fairly useful for users demonstrating that decoding
> works, especially if they're also testing an external decoding module and
> are unsure of where their replication problem is located, or what's wrong
> with their HBA settings.  For that reason it's important that test_decoding
> be available via OS packages, which would give me some reluctance to move
> it out of /contrib.
> 
> dummy_seclabel might serve the same purpose for users who are having issues
> with SEPostgres etc.  I don't know enough about it ...
> Stephen/Kaigai?
> 
Its original purpose is to run regression test on the platform without
selinux. So, it does not intend to use the dummy_seclabel for something
useful except for regression test.

Thanks,
--
NEC OSS Promotion Center / PG-Strom Project
KaiGai Kohei 

> --
> Josh Berkus
> PostgreSQL Experts Inc.
> http://pgexperts.com
> 
> 
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] no test programs in contrib

[Alvaro Herrera]

Tom Lane wrote:

> I'm not too happy with that approach, because packagers are going to
> tend to think they should package any files installed by install-world.
> The entire point of this change, IMO, is that the test modules should
> *not* get installed, certainly not by normal install targets.  Being
> consistent with the existing contrib packaging is exactly not what we
> want.
> 
> Maybe we should only allow check-world to run these tests, and not
> installcheck-world?  That's kind of annoying, but what you are
> doing now seems to defeat the purpose altogether.

Hadn't thought of the packaging angle of this.  I don't think packagers
really are as dumb as you suggest, but anyway implementing this idea
turned out to be simpler than I expected; here's a preliminary patch.

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
diff --git a/GNUmakefile.in b/GNUmakefile.in
index 8dbbcee..7c3c657 100644
--- a/GNUmakefile.in
+++ b/GNUmakefile.in
@@ -32,7 +32,7 @@ install:
 install-docs:
 	$(MAKE) -C doc install
 
-$(call recurse,install-world,doc src config contrib src/test/modules,install)
+$(call recurse,install-world,doc src config contrib,install)
 install-world:
 	+@echo "PostgreSQL, contrib, and documentation installation complete."
 
@@ -66,7 +66,7 @@ check check-tests: all
 check check-tests installcheck installcheck-parallel installcheck-tests:
 	$(MAKE) -C src/test/regress $@
 
-$(call recurse,check-world,src/test src/pl src/interfaces/ecpg contrib src/bin,check)
+$(call recurse,check-world,src/test src/pl src/interfaces/ecpg contrib src/bin src/test/modules,check)
 
 $(call recurse,installcheck-world,src/test src/pl src/interfaces/ecpg contrib src/bin,installcheck)
 
diff --git a/src/test/Makefile b/src/test/Makefile
index 5d997b8..ff061c1 100644
--- a/src/test/Makefile
+++ b/src/test/Makefile
@@ -14,4 +14,10 @@ include $(top_builddir)/src/Makefile.global
 
 SUBDIRS = regress isolation modules
 
-$(recurse)
+standard_recurse_targets := $(filter-out installcheck install, $(standard_targets))
+
+# We want to recurse to all subdirs for all standard targets, except that
+# installcheck and install should not recurse into the subdirectory "modules".
+$(call recurse,$(standard_recurse_targets))
+$(call recurse,installcheck,regress isolation)
+$(call recurse,install,regress isolation)

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] no test programs in contrib

[Tom Lane]

Josh Berkus  writes:
> So test_decoding is fairly useful for users demonstrating that decoding
> works, especially if they're also testing an external decoding module
> and are unsure of where their replication problem is located, or what's
> wrong with their HBA settings.  For that reason it's important that
> test_decoding be available via OS packages, which would give me some
> reluctance to move it out of /contrib.

If we follow that reasoning we'll end up removing nothing from contrib.
There is no reason that end users should need to be performing such
testing; anyone who does have reason to do it will have a source tree
at hand.

> dummy_seclabel might serve the same purpose for users who are having
> issues with SEPostgres etc.  I don't know enough about it ...

And as for dummy_seclabel, the same applies in spades, considering
that the number of users of SEPostgres can probably be counted without
running out of fingers.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] no test programs in contrib

[Josh Berkus]

On 11/24/2014 05:49 AM, Alvaro Herrera wrote:
> test_parser (a toy text search parser, added in 2007)
> dummy_seclabel (for SECURITY LABEL regression testing, added Sept 2010)
> worker_spi (for bgworkers, added Dec 2012)
> test_shm_mq (test program for shared memory queues, added Jan 2014)
> test_decoding (test program for logical decoding, added March 2014)

So test_decoding is fairly useful for users demonstrating that decoding
works, especially if they're also testing an external decoding module
and are unsure of where their replication problem is located, or what's
wrong with their HBA settings.  For that reason it's important that
test_decoding be available via OS packages, which would give me some
reluctance to move it out of /contrib.

dummy_seclabel might serve the same purpose for users who are having
issues with SEPostgres etc.  I don't know enough about it ...
Stephen/Kaigai?

-- 
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] no test programs in contrib

[Tom Lane]

Alvaro Herrera  writes:
> I have also changed things so that:

> 1. test modules are not installed by "make install", not checked by
> "make installcheck", not checked by "make check".

> 2. test modules are checked by "make check-world" (this is consistent
> with handling of contrib).

> 3. test modules are checked by "make installcheck-world" (this is
> consistent with handling of contrib)

> 4. test modules are installed by "make install-world".  This is
> consistent with contrib, and it's necessary so that "make
> installcheck-world" passes.

I'm not too happy with that approach, because packagers are going to
tend to think they should package any files installed by install-world.
The entire point of this change, IMO, is that the test modules should
*not* get installed, certainly not by normal install targets.  Being
consistent with the existing contrib packaging is exactly not what we
want.

Maybe we should only allow check-world to run these tests, and not
installcheck-world?  That's kind of annoying, but what you are
doing now seems to defeat the purpose altogether.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] no test programs in contrib

[Alvaro Herrera]

Peter Eisentraut wrote:
> On 11/26/14 9:27 AM, Alvaro Herrera wrote:
> > I haven't done anything about documentation.  I thought a new chapter
> > after "Additional Supplied Modules", perhaps entitled "Additional Sample
> > Modules" would be appropriate.
> 
> I would remove the SGML files and put simple README files into each
> directory.

They are so small that it makes sense to do it like that.  Here's a
patch for this.

I have also changed things so that:

1. test modules are not installed by "make install", not checked by
"make installcheck", not checked by "make check".

2. test modules are checked by "make check-world" (this is consistent
with handling of contrib).

3. test modules are checked by "make installcheck-world" (this is
consistent with handling of contrib)

4. test modules are installed by "make install-world".  This is
consistent with contrib, and it's necessary so that "make
installcheck-world" passes.


I moved the contents from SGML files into READMEs, and removed the
references from other SGML files, turning them into
contrib/ instead (these are release-9.4 and so on, which is
why I reference the old locations.  I assume release-9.5 will mention
the moves).

There's some untested new code in vcregress.pl, but nothing else
about msvc has been done.

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
diff --git a/GNUmakefile.in b/GNUmakefile.in
index 69e0824..8dbbcee 100644
--- a/GNUmakefile.in
+++ b/GNUmakefile.in
@@ -32,7 +32,7 @@ install:
 install-docs:
 	$(MAKE) -C doc install
 
-$(call recurse,install-world,doc src config contrib,install)
+$(call recurse,install-world,doc src config contrib src/test/modules,install)
 install-world:
 	+@echo "PostgreSQL, contrib, and documentation installation complete."
 
diff --git a/contrib/Makefile b/contrib/Makefile
index b37d0dd..efee109 100644
--- a/contrib/Makefile
+++ b/contrib/Makefile
@@ -16,7 +16,6 @@ SUBDIRS = \
 		dblink		\
 		dict_int	\
 		dict_xsyn	\
-		dummy_seclabel	\
 		earthdistance	\
 		file_fdw	\
 		fuzzystrmatch	\
@@ -50,13 +49,9 @@ SUBDIRS = \
 		spi		\
 		tablefunc	\
 		tcn		\
-		test_decoding	\
-		test_parser	\
-		test_shm_mq	\
 		tsearch2	\
 		unaccent	\
-		vacuumlo	\
-		worker_spi
+		vacuumlo
 
 ifeq ($(with_openssl),yes)
 SUBDIRS += sslinfo
diff --git a/contrib/pg_upgrade/test.sh b/contrib/pg_upgrade/test.sh
index 7bbd2c7..41614b2 100644
--- a/contrib/pg_upgrade/test.sh
+++ b/contrib/pg_upgrade/test.sh
@@ -65,6 +65,7 @@ if [ "$1" = '--install' ]; then
 	"$MAKE" -s -C ../.. install DESTDIR="$temp_install"
 	"$MAKE" -s -C ../pg_upgrade_support install DESTDIR="$temp_install"
 	"$MAKE" -s -C . install DESTDIR="$temp_install"
+	"$MAKE" -s -C ../../src/test/modules install DESTDIR="$temp_install"
 
 	# platform-specific magic to find the shared libraries; see pg_regress.c
 	LD_LIBRARY_PATH=$libdir:$LD_LIBRARY_PATH
@@ -143,7 +144,14 @@ set -x
 
 "$oldbindir"/initdb -N
 "$oldbindir"/pg_ctl start -l "$logdir/postmaster1.log" -o "$POSTMASTER_OPTS" -w
-if "$MAKE" -C "$oldsrc" installcheck; then
+"$MAKE" -C "$oldsrc" installcheck
+make_installcheck_status=$?
+if [ $make_installcheck_status -eq 0 ]; then
+	"$MAKE" -C "$oldsrc"/src/test/modules installcheck
+	make_installcheck_status=$?
+fi
+
+if [ $make_installcheck_status -eq 0 ]; then
 	pg_dumpall -f "$temp_root"/dump1.sql || pg_dumpall1_status=$?
 	if [ "$newsrc" != "$oldsrc" ]; then
 		oldpgversion=`psql -A -t -d regression -c "SHOW server_version_num"`
@@ -165,12 +173,9 @@ if "$MAKE" -C "$oldsrc" installcheck; then
 		sed "s;$oldsrc;$newsrc;g" "$temp_root"/dump1.sql.orig >"$temp_root"/dump1.sql
 	fi
 else
-	make_installcheck_status=$?
-fi
-"$oldbindir"/pg_ctl -m fast stop
-if [ -n "$make_installcheck_status" ]; then
 	exit 1
 fi
+"$oldbindir"/pg_ctl -m fast stop
 if [ -n "$psql_fix_sql_status" ]; then
 	exit 1
 fi
diff --git a/doc/src/sgml/contrib.sgml b/doc/src/sgml/contrib.sgml
index ec68f10..8836b0c 100644
--- a/doc/src/sgml/contrib.sgml
+++ b/doc/src/sgml/contrib.sgml
@@ -113,7 +113,6 @@ CREATE EXTENSION module_name FROM unpackaged;
  &dblink;
  &dict-int;
  &dict-xsyn;
- &dummy-seclabel;
  &earthdistance;
  &file-fdw;
  &fuzzystrmatch;
@@ -140,9 +139,6 @@ CREATE EXTENSION module_name FROM unpackaged;
  &sslinfo;
  &tablefunc;
  &tcn;
- &test-decoding;
- &test-parser;
- &test-shm-mq;
  &tsearch2;
  &unaccent;
  &uuid-ossp;
diff --git a/doc/src/sgml/dummy-seclabel.sgml b/doc/src/sgml/dummy-seclabel.sgml
deleted file mode 100644
index d064705..000
--- a/doc/src/sgml/dummy-seclabel.sgml
+++ /dev/null
@@ -1,74 +0,0 @@
-
-
-
- dummy_seclabel
-
- 
-  dummy_seclabel
- 
-
- 
-  The dummy_seclabel module exists only to support regression
-  testing of the SECURITY LABEL statement.  It is not intended
-  to be used in production.
- 
-
- 
-  Rationale
-
-  
-   The SECURITY LABEL statement allows the user to assign security
-   labels to database objects; however, security labels can only be assigned
-   when spe

Re: [HACKERS] no test programs in contrib

[Peter Eisentraut]

On 11/26/14 9:27 AM, Alvaro Herrera wrote:
> I haven't done anything about documentation.  I thought a new chapter
> after "Additional Supplied Modules", perhaps entitled "Additional Sample
> Modules" would be appropriate.

I would remove the SGML files and put simple README files into each
directory.


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] no test programs in contrib

[Alvaro Herrera]

Robert Haas wrote:
> On Wed, Nov 26, 2014 at 9:27 AM, Alvaro Herrera
>  wrote:

> > Because many of them had either "test" in their names or some other
> > now-useless particle, I renamed them:
> >
> > worker_spi  ->  bgworker
> > test_decoding   ->  logical_decoding
> > dummy_seclabel  ->  seclabel
> > test_shm_mq ->  shm_mq
> > test_parser ->  tsparser

> I like the move.  I dislike the renaming.

Do you dislike the new names, or the fact that they are changing at all?

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] no test programs in contrib

[Alvaro Herrera]

Tom Lane wrote:
> Alvaro Herrera  writes:
> > This is pretty bulky, but really the vast majority of the changes here
> > are just "git mv".
> 
> For ease of review, is there a way to get git to show just the diffs that
> *aren't* git mv?  (That is, show changes in a file's content without
> respect to its having moved?)

I think git diff -D -M -B does that.  Here's such a diff, which I
obtained from "git show" (otherwise you'd need to "git add" all the new
files, I think, which would be pretty annoying)

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
commit 3528d3323dd6cf086c9326432293827cd003c783
Author: Alvaro Herrera 
Date:   Wed Nov 26 10:16:54 2014 -0300

src/test/modules

diff --git a/GNUmakefile.in b/GNUmakefile.in
index 69e0824..3a57495 100644
--- a/GNUmakefile.in
+++ b/GNUmakefile.in
@@ -65,6 +65,7 @@ check check-tests: all
 
 check check-tests installcheck installcheck-parallel installcheck-tests:
 	$(MAKE) -C src/test/regress $@
+	$(MAKE) -C src/test/modules $@
 
 $(call recurse,check-world,src/test src/pl src/interfaces/ecpg contrib src/bin,check)
 
diff --git a/contrib/Makefile b/contrib/Makefile
index b37d0dd..efee109 100644
--- a/contrib/Makefile
+++ b/contrib/Makefile
@@ -16,7 +16,6 @@ SUBDIRS = \
 		dblink		\
 		dict_int	\
 		dict_xsyn	\
-		dummy_seclabel	\
 		earthdistance	\
 		file_fdw	\
 		fuzzystrmatch	\
@@ -50,13 +49,9 @@ SUBDIRS = \
 		spi		\
 		tablefunc	\
 		tcn		\
-		test_decoding	\
-		test_parser	\
-		test_shm_mq	\
 		tsearch2	\
 		unaccent	\
-		vacuumlo	\
-		worker_spi
+		vacuumlo
 
 ifeq ($(with_openssl),yes)
 SUBDIRS += sslinfo
diff --git a/contrib/pg_upgrade/test.sh b/contrib/pg_upgrade/test.sh
index 7bbd2c7..dcc84f3 100644
--- a/contrib/pg_upgrade/test.sh
+++ b/contrib/pg_upgrade/test.sh
@@ -65,6 +65,7 @@ if [ "$1" = '--install' ]; then
 	"$MAKE" -s -C ../.. install DESTDIR="$temp_install"
 	"$MAKE" -s -C ../pg_upgrade_support install DESTDIR="$temp_install"
 	"$MAKE" -s -C . install DESTDIR="$temp_install"
+	"$MAKE" -s -C ../../src/test/modules install DESTDIR="$temp_install"
 
 	# platform-specific magic to find the shared libraries; see pg_regress.c
 	LD_LIBRARY_PATH=$libdir:$LD_LIBRARY_PATH
diff --git a/src/test/Makefile b/src/test/Makefile
index 0fd7eab..5d997b8 100644
--- a/src/test/Makefile
+++ b/src/test/Makefile
@@ -12,6 +12,6 @@ subdir = src/test
 top_builddir = ../..
 include $(top_builddir)/src/Makefile.global
 
-SUBDIRS = regress isolation
+SUBDIRS = regress isolation modules
 
 $(recurse)
diff --git a/contrib/worker_spi/Makefile b/src/test/modules/bgworker/Makefile
similarity index 76%
rename from contrib/worker_spi/Makefile
rename to src/test/modules/bgworker/Makefile
index 5cce4d1..673281a 100644
--- a/contrib/worker_spi/Makefile
+++ b/src/test/modules/bgworker/Makefile
@@ -1,4 +1,4 @@
-# contrib/worker_spi/Makefile
+# src/test/modules/bgworker/Makefile
 
 MODULES = worker_spi
 
@@ -11,8 +11,8 @@ PG_CONFIG = pg_config
 PGXS := $(shell $(PG_CONFIG) --pgxs)
 include $(PGXS)
 else
-subdir = contrib/worker_spi
-top_builddir = ../..
+subdir = src/test/modules/bgworker
+top_builddir = ../../../..
 include $(top_builddir)/src/Makefile.global
 include $(top_srcdir)/contrib/contrib-global.mk
 endif
diff --git a/contrib/worker_spi/worker_spi--1.0.sql b/src/test/modules/bgworker/worker_spi--1.0.sql
similarity index 100%
rename from contrib/worker_spi/worker_spi--1.0.sql
rename to src/test/modules/bgworker/worker_spi--1.0.sql
diff --git a/contrib/worker_spi/worker_spi.c b/src/test/modules/bgworker/worker_spi.c
similarity index 100%
rename from contrib/worker_spi/worker_spi.c
rename to src/test/modules/bgworker/worker_spi.c
diff --git a/contrib/worker_spi/worker_spi.control b/src/test/modules/bgworker/worker_spi.control
similarity index 100%
rename from contrib/worker_spi/worker_spi.control
rename to src/test/modules/bgworker/worker_spi.control
diff --git a/contrib/test_decoding/.gitignore b/src/test/modules/logical_decoding/.gitignore
similarity index 100%
rename from contrib/test_decoding/.gitignore
rename to src/test/modules/logical_decoding/.gitignore
diff --git a/contrib/test_decoding/Makefile b/src/test/modules/logical_decoding/Makefile
similarity index 78%
rename from contrib/test_decoding/Makefile
rename to src/test/modules/logical_decoding/Makefile
index 438be44..0bc85d8 100644
--- a/contrib/test_decoding/Makefile
+++ b/src/test/modules/logical_decoding/Makefile
@@ -1,4 +1,4 @@
-# contrib/test_decoding/Makefile
+# src/test/modules/logical_decoding/Makefile
 
 MODULES = test_decoding
 PGFILEDESC = "test_decoding - example of a logical decoding output plugin"
@@ -12,8 +12,8 @@ PG_CONFIG = pg_config
 PGXS := $(shell $(PG_CONFIG) --pgxs)
 include $(PGXS)
 else
-subdir = contrib/test_decoding
-top_builddir = ../..
+subdir = src/test/modules/logical_decoding
+top_builddir = ../../../..
 include $(top_builddir)/src/Makefile.global
 include $(top_srcdir)/contrib/contrib-global.mk
 endif
@@ -35,2

Re: [HACKERS] no test programs in contrib

[Andres Freund]

On 2014-11-26 10:08:57 -0500, Tom Lane wrote:
> Alvaro Herrera  writes:
> > This is pretty bulky, but really the vast majority of the changes here
> > are just "git mv".
> 
> For ease of review, is there a way to get git to show just the diffs that
> *aren't* git mv?  (That is, show changes in a file's content without
> respect to its having moved?)

Yes, that's possible. git diff/show/whatever -M

Greetings,

Andres Freund

-- 
 Andres Freund http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] no test programs in contrib

[Tom Lane]

Alvaro Herrera  writes:
> This is pretty bulky, but really the vast majority of the changes here
> are just "git mv".

For ease of review, is there a way to get git to show just the diffs that
*aren't* git mv?  (That is, show changes in a file's content without
respect to its having moved?)

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] no test programs in contrib

[Alvaro Herrera]

This is pretty bulky, but really the vast majority of the changes here
are just "git mv".

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services


test_modules.patch.gz
Description: application/gzip

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] no test programs in contrib

[Fabrízio de Royes Mello]

On Wed, Nov 26, 2014 at 12:27 PM, Alvaro Herrera 
wrote:
>
> Here's a patch.  This creates a new subdir src/test/modules and places
> the five initially proposed modules in there.  They continue to have
> their makefile with the same ifdef USE_PGXS pattern; they are no longer
> installed by default.
>
> Because many of them had either "test" in their names or some other
> now-useless particle, I renamed them:
>
> worker_spi  ->  bgworker
> test_decoding   ->  logical_decoding
> dummy_seclabel  ->  seclabel
> test_shm_mq ->  shm_mq
> test_parser ->  tsparser
>
> The renaming is not complete: the extensions continue to have the old
> names, for instance.  If the consensus is to rename them completely I
> can finish that, or we can decide to keep the original names, but they
> all seem inappropriate to me.
>
> I haven't done anything about documentation.  I thought a new chapter
> after "Additional Supplied Modules", perhaps entitled "Additional Sample
> Modules" would be appropriate.
>
> I tweaked make targets check, installcheck, installcheck-world,
> check-world: they all run the additional tests now.  For buildfarm, the
> client code will need to be updated to have a new stage for
> src/test/modules running "make check".
>
> I haven't touched MSVC yet.
>
> Opinions on this approach please?
>

The patch is missing...

Regards,

--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Timbira: http://www.timbira.com.br
>> Blog: http://fabriziomello.github.io
>> Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello
>> Github: http://github.com/fabriziomello

Re: [HACKERS] no test programs in contrib

[Robert Haas]

On Wed, Nov 26, 2014 at 9:27 AM, Alvaro Herrera
 wrote:
> Here's a patch.  This creates a new subdir src/test/modules and places
> the five initially proposed modules in there.  They continue to have
> their makefile with the same ifdef USE_PGXS pattern; they are no longer
> installed by default.
>
> Because many of them had either "test" in their names or some other
> now-useless particle, I renamed them:
>
> worker_spi  ->  bgworker
> test_decoding   ->  logical_decoding
> dummy_seclabel  ->  seclabel
> test_shm_mq ->  shm_mq
> test_parser ->  tsparser
>
> The renaming is not complete: the extensions continue to have the old
> names, for instance.  If the consensus is to rename them completely I
> can finish that, or we can decide to keep the original names, but they
> all seem inappropriate to me.
>
> I haven't done anything about documentation.  I thought a new chapter
> after "Additional Supplied Modules", perhaps entitled "Additional Sample
> Modules" would be appropriate.
>
> I tweaked make targets check, installcheck, installcheck-world,
> check-world: they all run the additional tests now.  For buildfarm, the
> client code will need to be updated to have a new stage for
> src/test/modules running "make check".
>
> I haven't touched MSVC yet.
>
> Opinions on this approach please?

I like the move.  I dislike the renaming.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] no test programs in contrib

[Alvaro Herrera]

Here's a patch.  This creates a new subdir src/test/modules and places
the five initially proposed modules in there.  They continue to have
their makefile with the same ifdef USE_PGXS pattern; they are no longer
installed by default.

Because many of them had either "test" in their names or some other
now-useless particle, I renamed them:

worker_spi  ->  bgworker
test_decoding   ->  logical_decoding
dummy_seclabel  ->  seclabel
test_shm_mq ->  shm_mq
test_parser ->  tsparser

The renaming is not complete: the extensions continue to have the old
names, for instance.  If the consensus is to rename them completely I
can finish that, or we can decide to keep the original names, but they
all seem inappropriate to me.

I haven't done anything about documentation.  I thought a new chapter
after "Additional Supplied Modules", perhaps entitled "Additional Sample
Modules" would be appropriate.

I tweaked make targets check, installcheck, installcheck-world,
check-world: they all run the additional tests now.  For buildfarm, the
client code will need to be updated to have a new stage for
src/test/modules running "make check".

I haven't touched MSVC yet.

Opinions on this approach please?

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] no test programs in contrib

[Tom Lane]

Peter Eisentraut  writes:
> On 11/24/14 10:46 AM, Tom Lane wrote:
>> I think that test_parser is arguably useful as a skeleton/example for
>> user-written TS parsers, so I'd lean towards leaving it where it is,
>> but the others could move to src/test/ IMO.

> I think a useful dividing line would be, is it normally useful to
> install?  A skeleton is still useful if it is in a different place in
> the source tree (arguably more useful).  It's not useful if it's
> installed as a *.so.

I agree that where it is in the source tree isn't all that exciting
(for any purpose other than back-patching).  What is exciting is what
the context and build infrastructure look like.  The fact that test_parser
is packaged as a .so and can be built with PGXS makes it very easy to copy
as a skeleton for a user-written parser --- you don't need to invent your
own Makefile, in particular.  Now, maybe we'd retain those properties if
it were under src/test/, but that was not immediately clear to me.

Agreed that it shouldn't be installed as part of a standard binary
distribution, though.  We could potentially fix that while keeping it
in contrib, but maybe relocating it would be clearer.

What do we do about docs, though?  These things do need some user-facing
docs, or people won't even know they exist to be copied.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] no test programs in contrib

[Andres Freund]

On 2014-11-25 16:07:52 -0500, Peter Eisentraut wrote:
> On 11/24/14 9:35 AM, Andres Freund wrote:
> > I actually think that test_decoding is somewhat useful in other cases as
> > well, so it might be prudent to leave it there.
> 
> For what?
> 
> > src/test/ is good, but I think there should be another subdirectory
> > inside. testcases/?
> 
> What tests are not test cases?

There's infrastructure for tests in there already. It seems like a bad
idea to me to have individual tests on the same level as
src/test/regress and src/test/isolation.

regress/

Greetings,

Andres Freund

-- 
 Andres Freund http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] no test programs in contrib

[Peter Eisentraut]

On 11/24/14 10:46 AM, Tom Lane wrote:
> I think that test_parser is arguably useful as a skeleton/example for
> user-written TS parsers, so I'd lean towards leaving it where it is,
> but the others could move to src/test/ IMO.

I think a useful dividing line would be, is it normally useful to
install?  A skeleton is still useful if it is in a different place in
the source tree (arguably more useful).  It's not useful if it's
installed as a *.so.

> Usually that's when there is (a) a lot of history and (b) concern about
> back-patching fixes.  Neither of those arguments seem real strong for
> these modules, with the possible exception of test_parser.

Have we ever really tried to use the various git options that are meant
to help with that (in a recent git version)?

(If not, now we'd have a chance to try.)



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] no test programs in contrib

[Peter Eisentraut]

On 11/24/14 9:35 AM, Andres Freund wrote:
> I actually think that test_decoding is somewhat useful in other cases as
> well, so it might be prudent to leave it there.

For what?

> src/test/ is good, but I think there should be another subdirectory
> inside. testcases/?

What tests are not test cases?




-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] no test programs in contrib

[Peter Eisentraut]

On 11/24/14 8:49 AM, Alvaro Herrera wrote:
> What would you say if we were to move them to src/test/?

Yes please.

> Now, I know there is some resistance to the idea of moving source code
> around.

I think clarifying "contrib" is more important than that.



-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] no test programs in contrib

[Noah Misch]

On Mon, Nov 24, 2014 at 10:49:45AM -0300, Alvaro Herrera wrote:
> What's the general opinion on having test programs somewhere other than
> contrib/ ?

General opinion: slightly favorable.

> We currently have a number of subdirectories for test-only programs:
> 
> test_parser (a toy text search parser, added in 2007)
> dummy_seclabel (for SECURITY LABEL regression testing, added Sept 2010)
> worker_spi (for bgworkers, added Dec 2012)
> test_shm_mq (test program for shared memory queues, added Jan 2014)
> test_decoding (test program for logical decoding, added March 2014)

> What would you say if we were to move them to src/test/?  I could also
> see putting them in a brand new top-level directory, say testing/ or
> testprg/.

It's revealing that two of the first three responses each doubted the fit of
one of those moves.  I think that shows the lines aren't so bright after all,
and this specific proposal is not strong enough.  The line between a test
module and a sample-code module is blurry.

> Now, I know there is some resistance to the idea of moving source code
> around.  If this proposal is objected to, would people object the idea
> of putting the commit timestamp test module in src/test/commit_ts
> instead of the patch author's proposal, contrib/test_committs?

I'd rather defend moving source code or defend continuing to dump in contrib
than defend a src/test/modules defined as "test-oriented modules added after
November 2014."

Incidentally, +1 on "test_commit_ts" in preference to "test_committs".


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] no test programs in contrib

[Tom Lane]

Alvaro Herrera  writes:
> We currently have a number of subdirectories for test-only programs:

> test_parser (a toy text search parser, added in 2007)
> dummy_seclabel (for SECURITY LABEL regression testing, added Sept 2010)
> worker_spi (for bgworkers, added Dec 2012)
> test_shm_mq (test program for shared memory queues, added Jan 2014)
> test_decoding (test program for logical decoding, added March 2014)

> What would you say if we were to move them to src/test/?  I could also
> see putting them in a brand new top-level directory, say testing/ or
> testprg/.

I think that test_parser is arguably useful as a skeleton/example for
user-written TS parsers, so I'd lean towards leaving it where it is,
but the others could move to src/test/ IMO.

> Now, I know there is some resistance to the idea of moving source code
> around.

Usually that's when there is (a) a lot of history and (b) concern about
back-patching fixes.  Neither of those arguments seem real strong for
these modules, with the possible exception of test_parser.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] no test programs in contrib

[Andres Freund]

Hi,

On 2014-11-24 10:49:45 -0300, Alvaro Herrera wrote:
> I'm now contemplating the addition on a new one in the commit-timestamps
> patch, and I'm starting to feel that these are all misplaced.  I think
> we have been dumping them to contrib not because they really belong
> there, but because of the lack of a better place.

Agreed.

> As opposed to the
> rest of the stuff in contrib/, they don't serve any useful purpose on
> themselves; they are just demonstrating some coding techniques, or
> testing that some framework work as intended.

I actually think that test_decoding is somewhat useful in other cases as
well, so it might be prudent to leave it there.

> What would you say if we were to move them to src/test/?  I could also
> see putting them in a brand new top-level directory, say testing/ or
> testprg/.

src/test/ is good, but I think there should be another subdirectory
inside. testcases/?

Greetings,

Andres Freund

-- 
 Andres Freund http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] no test programs in contrib

[Petr Jelinek]

On 24/11/14 14:49, Alvaro Herrera wrote:

I'm now contemplating the addition on a new one in the commit-timestamps
patch, and I'm starting to feel that these are all misplaced.  I think
we have been dumping them to contrib not because they really belong
there, but because of the lack of a better place.  As opposed to the
rest of the stuff in contrib/, they don't serve any useful purpose on
themselves; they are just demonstrating some coding techniques, or
testing that some framework work as intended.  It seems impolite to
continue to pollute contrib with these; and my crystal ball says they
will continue to grow much more rapidly than normal, useful contrib
modules.



Completely agree.


What would you say if we were to move them to src/test/?  I could also
see putting them in a brand new top-level directory, say testing/ or
testprg/.

Now, I know there is some resistance to the idea of moving source code
around.  If this proposal is objected to, would people object the idea
of putting the commit timestamp test module in src/test/commit_ts
instead of the patch author's proposal, contrib/test_committs?



I'd go for src/test, but I think common subdirectory there is needed 
(src/test//commit_ts). Not sure what the  could 
be, maybe something like "standalone" as those tests get their own pg 
instance?


--
 Petr Jelinek  http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] no test programs in contrib

[Alvaro Herrera]

What's the general opinion on having test programs somewhere other than
contrib/ ?

We currently have a number of subdirectories for test-only programs:

test_parser (a toy text search parser, added in 2007)
dummy_seclabel (for SECURITY LABEL regression testing, added Sept 2010)
worker_spi (for bgworkers, added Dec 2012)
test_shm_mq (test program for shared memory queues, added Jan 2014)
test_decoding (test program for logical decoding, added March 2014)

I'm now contemplating the addition on a new one in the commit-timestamps
patch, and I'm starting to feel that these are all misplaced.  I think
we have been dumping them to contrib not because they really belong
there, but because of the lack of a better place.  As opposed to the
rest of the stuff in contrib/, they don't serve any useful purpose on
themselves; they are just demonstrating some coding techniques, or
testing that some framework work as intended.  It seems impolite to
continue to pollute contrib with these; and my crystal ball says they
will continue to grow much more rapidly than normal, useful contrib
modules.

What would you say if we were to move them to src/test/?  I could also
see putting them in a brand new top-level directory, say testing/ or
testprg/.

Now, I know there is some resistance to the idea of moving source code
around.  If this proposal is objected to, would people object the idea
of putting the commit timestamp test module in src/test/commit_ts
instead of the patch author's proposal, contrib/test_committs?

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers