Re: [HACKERS] no test programs in contrib
On Sat, Nov 29, 2014 at 5:54 AM, Alvaro Herrera alvhe...@2ndquadrant.com 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
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: [alvhe...@2ndquadrant.com: Re: [HACKERS] no test programs in contrib]
Michael Paquier wrote: On Wed, Dec 17, 2014 at 6:02 AM, Alvaro Herrera alvhe...@2ndquadrant.com 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]
Andrew Dunstan and...@dunslane.net 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]
On 12/17/2014 10:23 AM, Tom Lane wrote: Andrew Dunstan and...@dunslane.net 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]
On 12/17/2014 11:34 AM, Andrew Dunstan wrote: On 12/17/2014 10:23 AM, Tom Lane wrote: Andrew Dunstan and...@dunslane.net 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 https://raw.githubusercontent.com/PGBuildFarm/client-code/fca43683c9ec0a3d4dbbe636b7530010b8ef5213/run_build.pl 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 and...@dunslane.net 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 https://raw.githubusercontent.com/PGBuildFarm/client-code/fca43683c9ec0a3d4dbbe636b7530010b8ef5213/run_build.pl 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: [HACKERS] no test programs in contrib
On Sat, Nov 29, 2014 at 5:54 AM, Alvaro Herrera alvhe...@2ndquadrant.com 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]
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 21`;
+ 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 21`;
+ 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
21`;
+}
+else
+{
+chdir $pgsql/src/tools/msvc;
+@checklog = `perl vcregress.pl modulescheck 21`;
+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;
+open($handle,$logfile);
+while($handle)
+{
+push(@checklog,$_);
+}
+close($handle);
+}
+if ($status)
+{
+my @trace =
+
Re: [alvhe...@2ndquadrant.com: Re: [HACKERS] no test programs in contrib]
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]
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 https://github.com/PGBuildFarm/client-code/commit/d656c1c3ce46f290791c5ba5ede2f8ac8dfa342e 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 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=crakedt=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]
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 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=crakedt=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]
Andrew Dunstan and...@dunslane.net writes: I have pushed this change, and crake will be running the code. See https://github.com/PGBuildFarm/client-code/commit/d656c1c3ce46f290791c5ba5ede2f8ac8dfa342e 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]
On 12/16/2014 04:44 PM, Tom Lane wrote: Andrew Dunstan and...@dunslane.net writes: I have pushed this change, and crake will be running the code. See https://github.com/PGBuildFarm/client-code/commit/d656c1c3ce46f290791c5ba5ede2f8ac8dfa342e 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]
On Wed, Dec 17, 2014 at 6:02 AM, Alvaro Herrera alvhe...@2ndquadrant.com 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=crakedt=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: [HACKERS] no test programs in contrib
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
On 2014-11-27 15:51:51 -0500, Tom Lane wrote: Josh Berkus j...@agliodbs.com 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
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
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 alvhe...@alvh.no-ip.org 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 replaceablemodule_name/ FROM unpackaged; dblink; dict-int; dict-xsyn; - dummy-seclabel; earthdistance; file-fdw; fuzzystrmatch; @@ -141,8 +140,6 @@ CREATE EXTENSION replaceablemodule_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 @@ -!-- doc/src/sgml/dummy-seclabel.sgml -- - -sect1 id=dummy-seclabel xreflabel=dummy_seclabel - titledummy_seclabel/title - - indexterm zone=dummy-seclabel - primarydummy_seclabel/primary - /indexterm - - para - The filenamedummy_seclabel/ module exists only to support regression - testing of the commandSECURITY LABEL/ statement. It is not intended - to be used in production. - /para - - sect2 - titleRationale/title - - para - The commandSECURITY LABEL/ statement allows the user to assign security - labels to database objects; however, security labels can only be assigned
Re: [HACKERS] no test programs in contrib
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 filenamecontrib// 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 replaceablemodule_name/ FROM unpackaged; dblink; dict-int; dict-xsyn; - dummy-seclabel; earthdistance; file-fdw; fuzzystrmatch; @@ -140,9 +139,6 @@ CREATE EXTENSION replaceablemodule_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 @@ -!-- doc/src/sgml/dummy-seclabel.sgml -- - -sect1 id=dummy-seclabel xreflabel=dummy_seclabel - titledummy_seclabel/title - - indexterm zone=dummy-seclabel - primarydummy_seclabel/primary - /indexterm - - para - The filenamedummy_seclabel/ module exists only to support regression - testing of the commandSECURITY LABEL/ statement. It is not intended - to be used in production. - /para - - sect2 - titleRationale/title - - para - The commandSECURITY LABEL/
Re: [HACKERS] no test programs in contrib
Alvaro Herrera alvhe...@2ndquadrant.com 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
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
Josh Berkus j...@agliodbs.com 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
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
-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 kai...@ak.jp.nec.com -- 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
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
On Wed, Nov 26, 2014 at 9:27 AM, Alvaro Herrera alvhe...@2ndquadrant.com 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
On Wed, Nov 26, 2014 at 12:27 PM, Alvaro Herrera alvhe...@2ndquadrant.com 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
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
On 2014-11-26 10:08:57 -0500, Tom Lane wrote: Alvaro Herrera alvhe...@2ndquadrant.com 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 wrote: Alvaro Herrera alvhe...@2ndquadrant.com 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 alvhe...@alvh.no-ip.org 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
Re: [HACKERS] no test programs in contrib
Robert Haas wrote: On Wed, Nov 26, 2014 at 9:27 AM, Alvaro Herrera alvhe...@2ndquadrant.com 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
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
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
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
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
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 pete...@gmx.net 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
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/something/commit_ts). Not sure what the something 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
Re: [HACKERS] no test programs in contrib
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
Alvaro Herrera alvhe...@2ndquadrant.com 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
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
