Thread: TAP test breakage on MacOS X
make check-world dies ingloriously for me, like this: /bin/sh ../../../config/install-sh -c -d tmp_check/log make -C ../../.. DESTDIR='/Users/rhaas/pgsql/src/bin/initdb'/tmp_check/install install >'/Users/rhaas/pgsql/src/bin/initdb'/tmp_check/log/install.log 2>&1 cd . && TESTDIR='/Users/rhaas/pgsql/src/bin/initdb' PATH="/Users/rhaas/pgsql/src/bin/initdb/tmp_check/install/Users/rhaas/project/bin:$PATH" DYLD_LIBRARY_PATH='/Users/rhaas/pgsql/src/bin/initdb/tmp_check/install/Users/rhaas/project/lib' PGPORT='65432' prove --ext='.pl' -I ../../../src/test/perl/ --verbose t/ t/001_initdb.pl .. 1..14 1..3 ok 1 - initdb --help exit code 0 ok 2 - initdb --help goes to stdout ok 3 - initdb --help nothing to stderr ok 1 - initdb --help 1..3 ok 1 - initdb --version exit code 0 ok 2 - initdb --version goes to stdout ok 3 - initdb--version nothing to stderr ok 2 - initdb --version 1..2 ok 1 - initdb with invalid option nonzero exit code ok 2 - initdb with invalid optionprints error message # Looks like your test exited with 256 just after 2. not ok 3 - initdb options handling # Failed test 'initdb options handling' # at /opt/local/lib/perl5/5.12.5/Test/Builder.pm line 229. ok 4 - basic initdb ok 5 - existing data directory ok 6 - nosync ok 7 - sync only ok 8 - sync missing data directory ok 9 - existing empty data directory ok 10 - separate xlog directory ok 11 - relative xlog directory not allowed ok 12 - existing empty xlog directory ok 13 - existing nonempty xlog directory ok 14 - select default dictionary # Looks like you failed 1 test of 14. Dubious, test returned 1 (wstat 256, 0x100) Failed 1/14 subtests Test Summary Report ------------------- t/001_initdb.pl (Wstat: 256 Tests: 14 Failed: 1) Failed test: 3 Non-zero exit status: 1 Files=1, Tests=14, 23 wallclock secs ( 0.02 usr 0.01 sys + 9.57 cusr3.37 csys = 12.97 CPU) Result: FAIL make[2]: *** [check] Error 1 make[1]: *** [check-initdb-recurse] Error 2 make: *** [check-world-src/bin-recurse] Error 2 -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On 10/2/14 3:19 PM, Robert Haas wrote: > 1..2 > ok 1 - initdb with invalid option nonzero exit code > ok 2 - initdb with invalid option prints error message > # Looks like your test exited with 256 just after 2. > not ok 3 - initdb options handling > > # Failed test 'initdb options handling' > # at /opt/local/lib/perl5/5.12.5/Test/Builder.pm line 229. Is this repeatable? Bisectable? Has it ever worked? Have you tried a clean build? Did you upgrade something in your operating system? It appears to work "everywhere else". If none of this gets us closer to an answer, I can try to produce a patch that produces more details for such failures.
On 2014-10-02 17:09:43 -0400, Peter Eisentraut wrote: > On 10/2/14 3:19 PM, Robert Haas wrote: > > 1..2 > > ok 1 - initdb with invalid option nonzero exit code > > ok 2 - initdb with invalid option prints error message > > # Looks like your test exited with 256 just after 2. > > not ok 3 - initdb options handling > > > > # Failed test 'initdb options handling' > > # at /opt/local/lib/perl5/5.12.5/Test/Builder.pm line 229. > > Is this repeatable? Bisectable? Has it ever worked? Have you tried a > clean build? Did you upgrade something in your operating system? > > It appears to work "everywhere else". > > If none of this gets us closer to an answer, I can try to produce a > patch that produces more details for such failures. FWIW, the current amount of details on errors is clearly insufficient. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
On Thu, Oct 2, 2014 at 5:09 PM, Peter Eisentraut <peter_e@gmx.net> wrote: > On 10/2/14 3:19 PM, Robert Haas wrote: >> 1..2 >> ok 1 - initdb with invalid option nonzero exit code >> ok 2 - initdb with invalid option prints error message >> # Looks like your test exited with 256 just after 2. >> not ok 3 - initdb options handling >> >> # Failed test 'initdb options handling' >> # at /opt/local/lib/perl5/5.12.5/Test/Builder.pm line 229. > > Is this repeatable? Bisectable? Has it ever worked? Have you tried a > clean build? Did you upgrade something in your operating system? I don't think it's ever worked. I just didn't get around to reporting it before now. > If none of this gets us closer to an answer, I can try to produce a > patch that produces more details for such failures. A test that fails for no reason that can be gleaned from the output is not an improvement over not having a test at all. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Robert Haas <robertmhaas@gmail.com> writes:
> make check-world dies ingloriously for me, like this:
FWIW, it works fine for me on my Mac laptop, using the Perl 5.16.2 that
comes standard with OSX 10.9.5. I did have to install IPC::Run from
CPAN though.
> # Failed test 'initdb options handling'
> # at /opt/local/lib/perl5/5.12.5/Test/Builder.pm line 229.
This output seems to be pretty clear proof that you're not using
Apple's Perl. What is it exactly (where did you get it from)?
Also, noticing that what you're using is evidently Perl 5.12, I'm
wondering whether our TAP test scripts require a fairly new Perl version.
I recall some of my Salesforce colleagues griping that the TAP scripts
didn't work with older Perls.
regards, tom lane
I wrote:
> Also, noticing that what you're using is evidently Perl 5.12, I'm
> wondering whether our TAP test scripts require a fairly new Perl version.
> I recall some of my Salesforce colleagues griping that the TAP scripts
> didn't work with older Perls.
Well, they definitely don't work with Perl 5.10.0: on an older Mac
box I get
/bin/sh ../../../config/install-sh -c -d tmp_check/log
make -C ../../.. DESTDIR='/Users/tgl/pgsql/src/bin/initdb'/tmp_check/install install
>'/Users/tgl/pgsql/src/bin/initdb'/tmp_check/log/install.log2>&1
cd . && TESTDIR='/Users/tgl/pgsql/src/bin/initdb'
PATH="/Users/tgl/pgsql/src/bin/initdb/tmp_check/install/Users/tgl/testversion/bin:$PATH"
DYLD_LIBRARY_PATH='/Users/tgl/pgsql/src/bin/initdb/tmp_check/install/Users/tgl/testversion/lib'PGPORT='65440' prove
--ext='.pl'-I ../../../src/test/perl/ --verbose t/
Unknown option: ext
make: *** [check] Error 1
How badly do we need --ext? It looks to me from a quick perusal of the
prove man page that we could replace both --ext and the t/ argument with
an explicit list of test file names, which'd be easy enough to generate
with $(wildcard).
On RHEL6, which has perl 5.10.1, we seem to dodge that bullet
but nonetheless most of the tests get skipped because:
1..0 # SKIP version of Test::More is too old to support subplans
skipped: version of Test::More is too old to support subplans
So I remain of the opinion that the TAP tests need a lot of portability
work before they're going to be good for much.
regards, tom lane
On Thu, Oct 2, 2014 at 10:08 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Robert Haas <robertmhaas@gmail.com> writes: >> make check-world dies ingloriously for me, like this: > > FWIW, it works fine for me on my Mac laptop, using the Perl 5.16.2 that > comes standard with OSX 10.9.5. I did have to install IPC::Run from > CPAN though. > >> # Failed test 'initdb options handling' >> # at /opt/local/lib/perl5/5.12.5/Test/Builder.pm line 229. > > This output seems to be pretty clear proof that you're not using > Apple's Perl. What is it exactly (where did you get it from)? > > Also, noticing that what you're using is evidently Perl 5.12, I'm > wondering whether our TAP test scripts require a fairly new Perl version. > I recall some of my Salesforce colleagues griping that the TAP scripts > didn't work with older Perls. I installed it via MacPorts. [rhaas pgsql]$ which perl /opt/local/bin/perl [rhaas pgsql]$ perl -v This is perl 5, version 12, subversion 5 (v5.12.5) built for darwin-thread-multi-2level Copyright 1987-2012, Larry Wall Perl may be copied only under the terms of either the Artistic License or the GNU General Public License, which may be found in the Perl 5 source kit. Complete documentation for Perl, including FAQ lists, should be found on this system using "man perl" or "perldoc perl". If you have access to the Internet, point your browser at http://www.perl.org/, the Perl Home Page. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Thu, 2014-10-02 at 21:18 -0400, Robert Haas wrote: > > If none of this gets us closer to an answer, I can try to produce a > > patch that produces more details for such failures. > > A test that fails for no reason that can be gleaned from the output is > not an improvement over not having a test at all. I understand that this isn't great, and it's certainly something I'm looking into. But it's like pg_regress saying that psql crashed and leaving you to find out why. I don't think saying that the entire regression test suite is useless because of that is fair. The TAP tests are arguably already much easier to debug than pg_regress ever was.
On Mon, Oct 6, 2014 at 8:15 PM, Peter Eisentraut <peter_e@gmx.net> wrote: > On Thu, 2014-10-02 at 21:18 -0400, Robert Haas wrote: >> > If none of this gets us closer to an answer, I can try to produce a >> > patch that produces more details for such failures. >> >> A test that fails for no reason that can be gleaned from the output is >> not an improvement over not having a test at all. > > I understand that this isn't great, and it's certainly something I'm > looking into. But it's like pg_regress saying that psql crashed and > leaving you to find out why. I don't think saying that the entire > regression test suite is useless because of that is fair. The TAP tests > are arguably already much easier to debug than pg_regress ever was. Well, maybe. I wasn't able, after about 5 minutes of searching, to locate either a log file with details of the failure or the code that revealed what the test, the expected result, and the actual result were. It's possible that all that information is there and I just don't know where to look; it took me a while to learn where the various logs (postmaster.log, initdb.log, results) left behind by pg_regress were, too. If that information is not there, then I'd say it's not easier to debug. If it is and I don't know where to look ... well then I just need to get educated. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Robert Haas <robertmhaas@gmail.com> writes:
> On Mon, Oct 6, 2014 at 8:15 PM, Peter Eisentraut <peter_e@gmx.net> wrote:
>> The TAP tests
>> are arguably already much easier to debug than pg_regress ever was.
> Well, maybe. I wasn't able, after about 5 minutes of searching, to
> locate either a log file with details of the failure or the code that
> revealed what the test, the expected result, and the actual result
> were. It's possible that all that information is there and I just
> don't know where to look; it took me a while to learn where the
> various logs (postmaster.log, initdb.log, results) left behind by
> pg_regress were, too. If that information is not there, then I'd say
> it's not easier to debug. If it is and I don't know where to look ...
> well then I just need to get educated.
The given case seemed pretty opaque to me too. Could we maybe
have some documentation about how to debug TAP failures? Or in
other words, if they're "arguably" easier to debug, how about
presenting that argument?
Also to the point: does the buildfarm script know how to collect
the information needed to debug a TAP failure?
regards, tom lane
On 10/07/2014 12:15 AM, Tom Lane wrote: > Robert Haas <robertmhaas@gmail.com> writes: >> On Mon, Oct 6, 2014 at 8:15 PM, Peter Eisentraut <peter_e@gmx.net> wrote: >>> The TAP tests >>> are arguably already much easier to debug than pg_regress ever was. >> Well, maybe. I wasn't able, after about 5 minutes of searching, to >> locate either a log file with details of the failure or the code that >> revealed what the test, the expected result, and the actual result >> were. It's possible that all that information is there and I just >> don't know where to look; it took me a while to learn where the >> various logs (postmaster.log, initdb.log, results) left behind by >> pg_regress were, too. If that information is not there, then I'd say >> it's not easier to debug. If it is and I don't know where to look ... >> well then I just need to get educated. > The given case seemed pretty opaque to me too. Could we maybe > have some documentation about how to debug TAP failures? Or in > other words, if they're "arguably" easier to debug, how about > presenting that argument? > > Also to the point: does the buildfarm script know how to collect > the information needed to debug a TAP failure? > > No. In fact, it doesn't yet know how to run those tests. That's on my TODO list. cheers andrew
On 10/07/2014 09:53 AM, Andrew Dunstan wrote: > > On 10/07/2014 12:15 AM, Tom Lane wrote: >> Robert Haas <robertmhaas@gmail.com> writes: >>> On Mon, Oct 6, 2014 at 8:15 PM, Peter Eisentraut <peter_e@gmx.net> >>> wrote: >>>> The TAP tests >>>> are arguably already much easier to debug than pg_regress ever was. >>> Well, maybe. I wasn't able, after about 5 minutes of searching, to >>> locate either a log file with details of the failure or the code that >>> revealed what the test, the expected result, and the actual result >>> were. It's possible that all that information is there and I just >>> don't know where to look; it took me a while to learn where the >>> various logs (postmaster.log, initdb.log, results) left behind by >>> pg_regress were, too. If that information is not there, then I'd say >>> it's not easier to debug. If it is and I don't know where to look ... >>> well then I just need to get educated. >> The given case seemed pretty opaque to me too. Could we maybe >> have some documentation about how to debug TAP failures? Or in >> other words, if they're "arguably" easier to debug, how about >> presenting that argument? >> >> Also to the point: does the buildfarm script know how to collect >> the information needed to debug a TAP failure? >> >> > > > No. In fact, it doesn't yet know how to run those tests. That's on my > TODO list. > > > OK, I have a preliminary cut at adding these tests to the client. See <http://www.pgbuildfarm.org/cgi-bin/show_stage_log.pl?nm=crake&dt=2014-10-07%2015%3A38%3A04&stg=bin-check> for an example run. The patch is at <https://github.com/PGBuildFarm/client-code/commit/6f644b779c90b16f96e4454b807e804bde48b563> I don't much like the idea of doing an install/initdb/start for every directory in src/bin, though. Can't we at least manage a single installation directory for all these? Also I notice that the tests remove their data directories. That could make collecting any diagnosis data more difficult. Right now, I have no idea what I'm looking for anyway. cheers andrew
Andrew Dunstan <andrew@dunslane.net> writes:
> I don't much like the idea of doing an install/initdb/start for every
> directory in src/bin, though. Can't we at least manage a single
> installation directory for all these?
Peter had a patch to eliminate the overhead of multiple subinstalls;
not sure where that stands, but presumably it would address your issue.
regards, tom lane
On Mon, Oct 6, 2014 at 8:15 PM, Peter Eisentraut <peter_e@gmx.net> wrote: > On Thu, 2014-10-02 at 21:18 -0400, Robert Haas wrote: >> > If none of this gets us closer to an answer, I can try to produce a >> > patch that produces more details for such failures. >> >> A test that fails for no reason that can be gleaned from the output is >> not an improvement over not having a test at all. > > I understand that this isn't great, and it's certainly something I'm > looking into. But it's like pg_regress saying that psql crashed and > leaving you to find out why. I don't think saying that the entire > regression test suite is useless because of that is fair. The TAP tests > are arguably already much easier to debug than pg_regress ever was. I looked into this a bit more. This fixes it: diff --git a/src/test/perl/TestLib.pm b/src/test/perl/TestLib.pm index 545b2f3..2ffb7eb 100644 --- a/src/test/perl/TestLib.pm +++ b/src/test/perl/TestLib.pm @@ -205,6 +205,7 @@ sub program_options_handling_ok '2>', \$stderr; ok(!$result, "$cmd withinvalid option nonzero exit code"); isnt($stderr, '', "$cmd with invalid option prints error message"); + run [ '/usr/bin/true' ]; };} Not that I'm recommending that fix, of course. The problem is that running initdb --not-a-valid-option leaves $? set to 256, as seems entirely unsurprising. After running the anonymous block passed to it, Test::Builder::subtest calls Test::Build::finalize which calls Test::Build::_ending, which sets $real_exit_code to $? - i.e. 256. That function later throws up if $real_exit_code isn't 0, hence the failure. Off-hand, I'm not quite sure why this works for anyone. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On 10/9/14 3:38 PM, Robert Haas wrote: > The problem is that running initdb --not-a-valid-option leaves $? set > to 256, as seems entirely unsurprising. After running the anonymous > block passed to it, Test::Builder::subtest calls Test::Build::finalize > which calls Test::Build::_ending, which sets $real_exit_code to $? - > i.e. 256. That function later throws up if $real_exit_code isn't 0, > hence the failure. > > Off-hand, I'm not quite sure why this works for anyone. Thank you for this analysis. The problem is that the version of Test::Simple that ships with Perl 5.12 is broken in this particular way.I have committed a fix to skip the tests in thatcase. You might want to review whether this is really the Perl installation that you want to use. It looks like it is coming from MacPorts, and they have stopped updating their perl package. I have run the PostgreSQL build against all Perl major versions between 5.8 and 5.20, and there have been no more unaddressed version-related issues.
On 10/07/2014 01:57 PM, Tom Lane wrote: > Andrew Dunstan <andrew@dunslane.net> writes: >> I don't much like the idea of doing an install/initdb/start for every >> directory in src/bin, though. Can't we at least manage a single >> installation directory for all these? > Peter had a patch to eliminate the overhead of multiple subinstalls; > not sure where that stands, but presumably it would address your issue. Is there any progress on this. I'm reluctant to add this to the buildfarm client until it's solved. These tests currently take a heck of a lot longer than any other test suite. cheers andrew
Andrew Dunstan <andrew@dunslane.net> writes: > On 10/07/2014 01:57 PM, Tom Lane wrote: >> Peter had a patch to eliminate the overhead of multiple subinstalls; >> not sure where that stands, but presumably it would address your issue. > Is there any progress on this. I'm reluctant to add this to the > buildfarm client until it's solved. These tests currently take a heck of > a lot longer than any other test suite. While I'd like to see that patch committed to cut the runtime of "make check" in contrib, it's hardly the only stumbling block between us and enabling TAP tests in the buildfarm. The pathname length problem I noted in http://www.postgresql.org/message-id/16477.1413831571@sss.pgh.pa.us seems like a show-stopper as well, since undoubtedly a number of buildfarm critters are using buildroots with paths long enough to trigger it. The larger issue though is that even with both the above things fixed, the TAP tests would still be an expensive no-op on the majority of buildfarm members. AFAICT, I do not own a single machine on which the current TAP tests will consent to run, and in most cases that's after going out of my way to fetch CPAN modules that aren't in the vendor Perl installs. Peter's mostly been fixing the portability issues by disabling tests, which I guess is better than no fix at all, but it leaves darn little useful functionality. I think we need a serious discussion about choosing a baseline Perl version on which we need the TAP tests to work, and then some effort to make the tests actually work (not just skip tests) on all versions beyond that. regards, tom lane
On 10/26/2014 12:29 PM, Tom Lane wrote: > Andrew Dunstan <andrew@dunslane.net> writes: >> On 10/07/2014 01:57 PM, Tom Lane wrote: >>> Peter had a patch to eliminate the overhead of multiple subinstalls; >>> not sure where that stands, but presumably it would address your issue. >> Is there any progress on this. I'm reluctant to add this to the >> buildfarm client until it's solved. These tests currently take a heck of >> a lot longer than any other test suite. > While I'd like to see that patch committed to cut the runtime of "make > check" in contrib, it's hardly the only stumbling block between us and > enabling TAP tests in the buildfarm. > > The pathname length problem I noted in > http://www.postgresql.org/message-id/16477.1413831571@sss.pgh.pa.us > seems like a show-stopper as well, since undoubtedly a number of > buildfarm critters are using buildroots with paths long enough to > trigger it. +1 for fixing that, although it seems like a problem in what's being tested rather than in the test suite. > > The larger issue though is that even with both the above things fixed, > the TAP tests would still be an expensive no-op on the majority of > buildfarm members. AFAICT, I do not own a single machine on which the > current TAP tests will consent to run, and in most cases that's after > going out of my way to fetch CPAN modules that aren't in the vendor Perl > installs. Peter's mostly been fixing the portability issues by disabling > tests, which I guess is better than no fix at all, but it leaves darn > little useful functionality. > > I think we need a serious discussion about choosing a baseline Perl > version on which we need the TAP tests to work, and then some effort > to make the tests actually work (not just skip tests) on all versions > beyond that. > > As far as the buildfarm goes, we could make it a cheap noop by checking for the presence of the required modules (AFAIK that's Test::More, IPC::CMD and IPC::Run). I agree that just having it not run tests on most platforms is hardly a solution. cheers andrew
Andrew Dunstan <andrew@dunslane.net> writes: > On 10/26/2014 12:29 PM, Tom Lane wrote: >> The pathname length problem I noted in >> http://www.postgresql.org/message-id/16477.1413831571@sss.pgh.pa.us >> seems like a show-stopper as well, since undoubtedly a number of >> buildfarm critters are using buildroots with paths long enough to >> trigger it. > +1 for fixing that, although it seems like a problem in what's being > tested rather than in the test suite. I agree, but nonetheless we don't want the buildfarm turning mostly red because we enable TAP before fixing this. >> The larger issue though is that even with both the above things fixed, >> the TAP tests would still be an expensive no-op on the majority of >> buildfarm members. > As far as the buildfarm goes, we could make it a cheap noop by checking > for the presence of the required modules (AFAIK that's Test::More, > IPC::CMD and IPC::Run). You'd probably have to check not just presence but version; but yeah, that is a potential solution to the cycle-wastage problem. > I agree that just having it not run tests on most platforms is hardly a > solution. It doesn't do much to make the tests actually useful, for sure ... regards, tom lane
On Sun, Oct 26, 2014 at 12:29 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > The larger issue though is that even with both the above things fixed, > the TAP tests would still be an expensive no-op on the majority of > buildfarm members. AFAICT, I do not own a single machine on which the > current TAP tests will consent to run, and in most cases that's after > going out of my way to fetch CPAN modules that aren't in the vendor Perl > installs. Peter's mostly been fixing the portability issues by disabling > tests, which I guess is better than no fix at all, but it leaves darn > little useful functionality. I agree, emphatically. Honestly, if we can't get these tests running everywhere with reasonable effort, we should just rip them out. We've gone to a lot of trouble in general to make sure that our source code can be ported even to systems that arguably nobody uses any more, and instrumental to that effort is keeping the system requirements to install and test PostgreSQL minimal. At this point, I wouldn't mind moving the goalposts from C89 to C89 + a bunch of C99 features that are available on all the platforms we have buildfarm coverage for, and I wouldn't mind require perl to compile and install, full stop. But this patch has gone much further than that: you need a new-enough version of perl, and a new-enough version of a bunch of modules that aren't installed by default, and maybe not even in the vendor install, and the documentation on how to make it work is an embarrassment: http://www.postgresql.org/docs/devel/static/regress-tap.html Beyond all that, I have serious doubts about whether, even if we eventually get these tests mostly working in most places, whether they will actually catch any bugs. For example, consider dropdb. The TAP tests cover the following points: - When run with --help or --version, it should exit with code 0, print something on stderr, and print nothing on stdout. - When run with --not-a-valid-option, it should exit with a non-zero exit code, print something on stderr, and print nothing on stdout. - "dropdb foobar1" should cause "statement: DROP DATABASE foobar1" to show up in the postgresql log file. - "dropdb nonexistent" should exit non-zero. These are certainly good things to test, but I'd argue that once you've verified that they are working, they're unlikely to get broken again in the future. I'm generally supportive of the idea that we need more automated tests, but these tests seem pretty low-value to me, even if they were running everywhere, and even moreso if they aren't. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On 10/27/2014 05:41 PM, Robert Haas wrote: > On Sun, Oct 26, 2014 at 12:29 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >> The larger issue though is that even with both the above things fixed, >> the TAP tests would still be an expensive no-op on the majority of >> buildfarm members. AFAICT, I do not own a single machine on which the >> current TAP tests will consent to run, and in most cases that's after >> going out of my way to fetch CPAN modules that aren't in the vendor Perl >> installs. Peter's mostly been fixing the portability issues by disabling >> tests, which I guess is better than no fix at all, but it leaves darn >> little useful functionality. > > I agree, emphatically. Honestly, if we can't get these tests running > everywhere with reasonable effort, we should just rip them out. We've > gone to a lot of trouble in general to make sure that our source code > can be ported even to systems that arguably nobody uses any more, and > instrumental to that effort is keeping the system requirements to > install and test PostgreSQL minimal. At this point, I wouldn't mind > moving the goalposts from C89 to C89 + a bunch of C99 features that > are available on all the platforms we have buildfarm coverage for, and > I wouldn't mind require perl to compile and install, full stop. But > this patch has gone much further than that: you need a new-enough > version of perl, and a new-enough version of a bunch of modules that > aren't installed by default, and maybe not even in the vendor install, > and the documentation on how to make it work is an embarrassment: > > http://www.postgresql.org/docs/devel/static/regress-tap.html > > Beyond all that, I have serious doubts about whether, even if we > eventually get these tests mostly working in most places, whether they > will actually catch any bugs. The existing tests are not very useful, but it certainly would be nice to have some infrastructure like TAP to write useful tests. For example, I recently wrote a test suite for SSL, using TAP (http://www.postgresql.org/message-id/53DF9AFE.4090706@vmware.com), and it would be nice to have some TAP tests for hot standby, replication, PITR etc. I'm not sure if the current infrastructure is very good for that, but we need something. If the current infrastructure is beyond repair, then let's discuss what we could replace it with. - Heikki
On 10/27/2014 11:41 AM, Robert Haas wrote: > On Sun, Oct 26, 2014 at 12:29 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >> The larger issue though is that even with both the above things fixed, >> the TAP tests would still be an expensive no-op on the majority of >> buildfarm members. AFAICT, I do not own a single machine on which the >> current TAP tests will consent to run, and in most cases that's after >> going out of my way to fetch CPAN modules that aren't in the vendor Perl >> installs. Peter's mostly been fixing the portability issues by disabling >> tests, which I guess is better than no fix at all, but it leaves darn >> little useful functionality. > I agree, emphatically. Honestly, if we can't get these tests running > everywhere with reasonable effort, we should just rip them out. We've > gone to a lot of trouble in general to make sure that our source code > can be ported even to systems that arguably nobody uses any more, and > instrumental to that effort is keeping the system requirements to > install and test PostgreSQL minimal. At this point, I wouldn't mind > moving the goalposts from C89 to C89 + a bunch of C99 features that > are available on all the platforms we have buildfarm coverage for, and > I wouldn't mind require perl to compile and install, full stop. But > this patch has gone much further than that: you need a new-enough > version of perl, and a new-enough version of a bunch of modules that > aren't installed by default, and maybe not even in the vendor install, > and the documentation on how to make it work is an embarrassment: > > http://www.postgresql.org/docs/devel/static/regress-tap.html > > Beyond all that, I have serious doubts about whether, even if we > eventually get these tests mostly working in most places, whether they > will actually catch any bugs. For example, consider dropdb. The TAP > tests cover the following points: > > - When run with --help or --version, it should exit with code 0, print > something on stderr, and print nothing on stdout. > - When run with --not-a-valid-option, it should exit with a non-zero > exit code, print something on stderr, and print nothing on stdout. > - "dropdb foobar1" should cause "statement: DROP DATABASE foobar1" to > show up in the postgresql log file. > - "dropdb nonexistent" should exit non-zero. > > These are certainly good things to test, but I'd argue that once > you've verified that they are working, they're unlikely to get broken > again in the future. I'm generally supportive of the idea that we > need more automated tests, but these tests seem pretty low-value to > me, even if they were running everywhere, and even moreso if they > aren't. > Yeah. I think at the very least they should be removed from the check-world and installcheck-world targets until this is sorted out. cheers andrew
Heikki Linnakangas <hlinnakangas@vmware.com> writes:
> On 10/27/2014 05:41 PM, Robert Haas wrote:
>> Beyond all that, I have serious doubts about whether, even if we
>> eventually get these tests mostly working in most places, whether they
>> will actually catch any bugs.
> The existing tests are not very useful, but it certainly would be nice
> to have some infrastructure like TAP to write useful tests.
Yeah. I think this is an "if you build it they will come" situation.
The existing tests are really only proof-of-concept; if we can get the
infrastructure to the point of being widely usable, we can certainly
use it to write many more-useful tests.
So I'm not for giving up and ripping it out in the near future. But I do
think there's a lot of test-infrastructure work left here to get to the
point of being "widely usable", and I'm not sure if anyone is committed to
making that happen.
But let's stop talking in generalities and try to quantify where we think
the bar needs to be set. Is it okay to depend on non-core CPAN modules
at all? How old versions of Perl, Test::More, etc do we need it to work
with?
For my own purposes, I'd be satisfied if it ran on RHEL6 and reasonably
recent OS X. For RHEL6, I'd not want to have to install any CPAN modules
other than the versions supplied by Red Hat (which means no Test::More
subplans). This platform is only four years old, if you can't cope with
that then you're definitely too bleeding-edge to be a portable test
infrastructure.
regards, tom lane
Andrew Dunstan <andrew@dunslane.net> writes:
> Yeah. I think at the very least they should be removed from the
> check-world and installcheck-world targets until this is sorted out.
+1 for doing that in the 9.4 branch; I'm surprised we've not already
heard bitching from packagers about how far we've moved the goalposts
for what's required to get "make check-world" to work.
As I said in my other reply, I'm not giving up on this (yet) so I'd
be happy with leaving it in check-world in the master branch. But it's
not ready for prime time, and 9.4 needs to ship soon.
regards, tom lane
On Mon, Oct 27, 2014 at 12:31:56PM -0400, Tom Lane wrote: > Heikki Linnakangas <hlinnakangas@vmware.com> writes: > > On 10/27/2014 05:41 PM, Robert Haas wrote: > >> Beyond all that, I have serious doubts about whether, even if we > >> eventually get these tests mostly working in most places, whether they > >> will actually catch any bugs. > > > The existing tests are not very useful, but it certainly would be nice > > to have some infrastructure like TAP to write useful tests. > > Yeah. I think this is an "if you build it they will come" situation. > The existing tests are really only proof-of-concept; if we can get the > infrastructure to the point of being widely usable, we can certainly > use it to write many more-useful tests. Agreed. Having this framework when the pg_upgrade test suite originated would have prevented acquiring parallel implementations in Perl and shell. > But let's stop talking in generalities and try to quantify where we think > the bar needs to be set. Is it okay to depend on non-core CPAN modules > at all? How old versions of Perl, Test::More, etc do we need it to work > with? > > For my own purposes, I'd be satisfied if it ran on RHEL6 and reasonably > recent OS X. For RHEL6, I'd not want to have to install any CPAN modules > other than the versions supplied by Red Hat (which means no Test::More > subplans). This platform is only four years old, if you can't cope with > that then you're definitely too bleeding-edge to be a portable test > infrastructure. I want developers to have an inviting framework for writing tests, but I also want everyone to run the tests. Setting this bar entails picking a compromise between those goals. Consequences of losing subplans include (1) poorer visual structure in the output; (2) more kinds of test code changes will require updating the top-level test counts; (3) can't skip tests as a group. Those losses seem minor enough. My gut feeling is to target running on perl 5.10 with bundled module versions only and on perl 5.8 with additional modules. That is to say, assume 5.10 with respect to bundled modules and assume 5.8 with respect to core language features. Concretely, that means discontinuing use of subplans and replacing IPC::Run with IPC::Cmd. (Windows systems probably need to install IPC::Run so IPC::Cmd can use it internally; that is okay.) If those restrictions make the test case developer experience much worse, though, I'll backpedal toward less portability.
On 10/7/14 1:57 PM, Tom Lane wrote: > Peter had a patch to eliminate the overhead of multiple subinstalls; > not sure where that stands, but presumably it would address your issue. It will need some cleverness to sort out the parallel make issues that were brought up in the review thread.
On 10/27/14 11:41 AM, Robert Haas wrote: > Beyond all that, I have serious doubts about whether, even if we > eventually get these tests mostly working in most places, whether they > will actually catch any bugs. Well, they caught the fact that pg_basebackup can't back up tablespaces with names longer than 99 characters, for example. But it's wrong to expect the primary value of tests to be to detect previously unknown bugs. Yes, that has been the experience in this project. We have software that we think works, and then we send it out to test on N obscure platforms, and we find interesting things. But the other dimension is that tests allow you to make changes with confidence. For example, the tablespace mapping logic in pg_basebackup has been whacked around about three times. I don't have any doubt that it still works correctly, because it has extensive test coverage. If someone sends in a patch to support cpio in pg_basebackup (hey, no symlink name length limits) and wants to refactor the entire source code to achieve that, I'll have no problem with that. Yes, some of the tests are low in value, perhaps a bit silly. But it's a foundation, and I expect to add more in the future. The good news is that because some of these "silly" tests actually exercise important internal functionality of the test mechanisms (e.g., create temporary directory, run program, record exit status, capture stdout and stderr separately, check output files), I am pretty confident that we can go far with the current infrastructure without needing any more external modules or something like that. So that also means that not all tests need to be run everywhere all the time. We have the option to run specific tests. It could be useful to have better groupings, but that has been discussed before and it hasn't come to a solution. > These are certainly good things to test, but I'd argue that once > you've verified that they are working, they're unlikely to get broken > again in the future. Famous last words ... ;-)
On 10/28/14 12:46 AM, Noah Misch wrote: > Agreed. Having this framework when the pg_upgrade test suite originated would > have prevented acquiring parallel implementations in Perl and shell. Yes, that was certainly a motivation, and I would like to continue work on pg_upgrade testing. > Concretely, that means > discontinuing use of subplans and replacing IPC::Run with IPC::Cmd. (Windows > systems probably need to install IPC::Run so IPC::Cmd can use it internally; > that is okay.) If those restrictions make the test case developer experience > much worse, though, I'll backpedal toward less portability. I agree we should get rid of subplans. When I started out, they seemed useful, to avoid this sort of thing found in the postgresql-common test suite: use Test::More tests => ($#MAJORS == 0) ? 1 : 103 * 3; But as we are learning that they are a serious hindrance to portability, I think we can do without them. I have looked into IPC::Cmd, but the documentation keeps telling me that to do anything interesting I have to have IPC::Run anyway. I'll give it a try, though.
On Tue, Oct 28, 2014 at 9:01 PM, Peter Eisentraut <peter_e@gmx.net> wrote: > Well, they caught the fact that pg_basebackup can't back up tablespaces > with names longer than 99 characters, for example. > > But it's wrong to expect the primary value of tests to be to detect > previously unknown bugs. Yes, that has been the experience in this > project. We have software that we think works, and then we send it out > to test on N obscure platforms, and we find interesting things. > > But the other dimension is that tests allow you to make changes with > confidence. For example, the tablespace mapping logic in pg_basebackup > has been whacked around about three times. I don't have any doubt that > it still works correctly, because it has extensive test coverage. If > someone sends in a patch to support cpio in pg_basebackup (hey, no > symlink name length limits) and wants to refactor the entire source code > to achieve that, I'll have no problem with that. > > Yes, some of the tests are low in value, perhaps a bit silly. But it's > a foundation, and I expect to add more in the future. The good news is > that because some of these "silly" tests actually exercise important > internal functionality of the test mechanisms (e.g., create temporary > directory, run program, record exit status, capture stdout and stderr > separately, check output files), I am pretty confident that we can go > far with the current infrastructure without needing any more external > modules or something like that. I'm sympathetic to that line of reasoning, but I really think that if you want to keep this infrastructure, it needs to be made portable. I don't buy the argument that it isn't important to be able to run all of the tests everywhere all the time. I grant that, for example, it's reasonable not to run tests for Linux-style locales on Windows. But barring that sort of corner case, I really think they should all run everywhere. Now, if it turns out that there are one or two really old buildfarm critters that can't run all the tests, then I think I can let that go, but the goal should be for 100% coverage. Right now the *normal* situation is that the tests, when run on a typical developer's machine, either (1) don't run, because the developer's machine has some wart that they realize will break the tests; or (2) don't pass, because the developer's machine has some wart that they don't realize will break the tests, but it actually does. That means that the tests, as they exist today, are basically your toy. Most people aren't running them, and if they do, they fail, and nobody understands why. Until that changes, the tests really don't have any value to the project, and possibly negative value, if they encourage people to start ignoring test failures. I also think that the dependencies for the tests to actually run need to be clearly and explicitly documented. If the tests don't run on my machine, and I want them to run, there's nothing in the documentation I can go and look at to figure out what I need to do to fix that. In my mind, that's a big problem. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On 10/28/14 9:09 PM, Peter Eisentraut wrote: > I agree we should get rid of subplans. I removed subtests. Everything now runs (without skipping) with Perl 5.8+ plus IPC::Run. > I have looked into IPC::Cmd, but the documentation keeps telling me that > to do anything interesting I have to have IPC::Run anyway. I'll give it > a try, though. I tried this, but I'm not optimistic about it. While parts of IPC::Cmd are actually a bit nicer than IPC::Run, other parts are weird. For example, with most packages and functions in Perl that run a command, you can pass the command as a string or as a list (or array reference).The latter is preferred because it avoids issues withquoting, word splitting, spaces, etc. In IPC::Run, I can use the "run" function in the latter way, but I cannot use the "run_forked" function like that, and I need that one to get the exit code of a command. It's possible to work around that, but I'm getting the feeling that this is not very well designed. Also, IPC::Cmd is a wrapper module, and it passes the hard work down to other modules, depending on what's available. I think that sounds like a portability problem waiting to happen.
On Wed, Oct 29, 2014 at 08:14:07PM -0400, Peter Eisentraut wrote: > On 10/28/14 9:09 PM, Peter Eisentraut wrote: > > I have looked into IPC::Cmd, but the documentation keeps telling me that > > to do anything interesting I have to have IPC::Run anyway. I'll give it > > a try, though. > > I tried this, but I'm not optimistic about it. While parts of IPC::Cmd > are actually a bit nicer than IPC::Run, other parts are weird. For > example, with most packages and functions in Perl that run a command, > you can pass the command as a string or as a list (or array reference). > The latter is preferred because it avoids issues with quoting, word > splitting, spaces, etc. In IPC::Run, I can use the "run" function in > the latter way, but I cannot use the "run_forked" function like that, > and I need that one to get the exit code of a command. It's possible to > work around that, but I'm getting the feeling that this is not very well > designed. Ick; I concur with your judgment on those aspects of the IPC::Cmd design. Thanks for investigating. So, surviving options include: 1. Require IPC::Run. 2. Write our own run() that reports the raw exit code. 3. Distill the raw exit code from the IPC::Cmd::run() error string. 4. Pass IPC::Run::run_forked() a subroutine that execs an argument list. Any others worth noting? > Also, IPC::Cmd is a wrapper module, and it passes the hard work down to > other modules, depending on what's available. I think that sounds like > a portability problem waiting to happen. Assuming test suite code doesn't modify $IPC::Cmd::USE_* variables, the two relevant backends are IPC::Run on Windows and IPC::Open3 everywhere else. That's not bad.
Noah Misch <noah@leadboat.com> writes:
> Ick; I concur with your judgment on those aspects of the IPC::Cmd design.
> Thanks for investigating. So, surviving options include:
> 1. Require IPC::Run.
> 2. Write our own run() that reports the raw exit code.
> 3. Distill the raw exit code from the IPC::Cmd::run() error string.
> 4. Pass IPC::Run::run_forked() a subroutine that execs an argument list.
> Any others worth noting?
From a user-inconvenience standpoint, I think requiring IPC::Run is not
horrid, so long as we add an --enable-something configure option to drive
whether the TAP tests are run or skipped. However, it has to work with
whatever version of IPC::Run the platform's Perl installation comes up
with, and in that regard I'm a bit worried. I'm sitting here watching a
trial run on prairiedog (OSX 10.4.11, perl 5.8.6), and there seems to be
something extremely wrong from a performance standpoint. For example:
t/001_pg_controldata.pl ..
1..13
ok 1 - pg_controldata --help exit code 0
ok 2 - pg_controldata --help goes to stdout
ok 3 - pg_controldata --help nothing to stderr
ok 4 - pg_controldata --version exit code 0
ok 5 - pg_controldata --version goes to stdout
ok 6 - pg_controldata --version nothing to stderr
ok 7 - pg_controldata with invalid option nonzero exit code
ok 8 - pg_controldata with invalid option prints error message
ok 9 - pg_controldata without arguments fails
ok 10 - pg_controldata with nonexistent directory fails
ok 11 - pg_controldata /Users/tgl/pgsql/src/bin/pg_controldata/tmp_testOtBj/data exit code 0
ok 12 - pg_controldata /Users/tgl/pgsql/src/bin/pg_controldata/tmp_testOtBj/data no stderr
ok 13 - pg_controldata produces output: matches
ok
All tests successful.
Files=1, Tests=13, 32 wallclock secs ( 0.23 usr 0.05 sys + 21.97 cusr 7.82 csys = 30.07 CPU)
Result: PASS
Yup, you read that right, it took 32 seconds to run those dozen utterly
trivial tests. As far as I could tell by eyeball, pretty much all of the
time went into test 11, which is odd since it seems not significantly
different from the others. I think there's something wacky about IPC::Run
on this platform.
regards, tom lane
On 2014-10-30 01:57:15 -0400, Noah Misch wrote: > On Wed, Oct 29, 2014 at 08:14:07PM -0400, Peter Eisentraut wrote: > > On 10/28/14 9:09 PM, Peter Eisentraut wrote: > > > I have looked into IPC::Cmd, but the documentation keeps telling me that > > > to do anything interesting I have to have IPC::Run anyway. I'll give it > > > a try, though. > > > > I tried this, but I'm not optimistic about it. While parts of IPC::Cmd > > are actually a bit nicer than IPC::Run, other parts are weird. For > > example, with most packages and functions in Perl that run a command, > > you can pass the command as a string or as a list (or array reference). > > The latter is preferred because it avoids issues with quoting, word > > splitting, spaces, etc. In IPC::Run, I can use the "run" function in > > the latter way, but I cannot use the "run_forked" function like that, > > and I need that one to get the exit code of a command. It's possible to > > work around that, but I'm getting the feeling that this is not very well > > designed. > > Ick; I concur with your judgment on those aspects of the IPC::Cmd design. > Thanks for investigating. So, surviving options include: > > 1. Require IPC::Run. > 2. Write our own run() that reports the raw exit code. > 3. Distill the raw exit code from the IPC::Cmd::run() error string. > 4. Pass IPC::Run::run_forked() a subroutine that execs an argument list. > > Any others worth noting? 5. Include a copy of IPC::Run in the repository till it's common enough. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
I wrote:
> Yup, you read that right, it took 32 seconds to run those dozen utterly
> trivial tests. As far as I could tell by eyeball, pretty much all of the
> time went into test 11, which is odd since it seems not significantly
> different from the others. I think there's something wacky about IPC::Run
> on this platform.
Hm ... on my RHEL6 machine, it takes about 2.5 seconds to run the
pg_controldata TAP tests, and again it looks like practically all of that
is going into test 11. Given the speed differential between the two
machines, the time taken by prairiedog is roughly in line with that.
So the real question seems to be "why is IPC::Run's performance so
inconsistent"? Is there something I'm not understanding that would cause
test 11 to require much more time than the others?
regards, tom lane
I wrote:
> Yup, you read that right, it took 32 seconds to run those dozen utterly
> trivial tests. As far as I could tell by eyeball, pretty much all of the
> time went into test 11, which is odd since it seems not significantly
> different from the others. I think there's something wacky about IPC::Run
> on this platform.
Oh, never mind: after digging into the test source I eventually realized
that there's an initdb happening between tests 10 and 11, and that's
what's eating the time. It might be a thought to print something to
indicate that a time-consuming step is happening; but the lack of any
feedback here isn't exactly IPC::Run's fault.
Anyway, I can confirm Peter's statement that the current tests work even
on quite old platforms, as long as you install IPC::Run.
regards, tom lane
On 10/30/2014 05:23 PM, Tom Lane wrote: > I wrote: >> Yup, you read that right, it took 32 seconds to run those dozen utterly >> trivial tests. As far as I could tell by eyeball, pretty much all of the >> time went into test 11, which is odd since it seems not significantly >> different from the others. I think there's something wacky about IPC::Run >> on this platform. > Oh, never mind: after digging into the test source I eventually realized > that there's an initdb happening between tests 10 and 11, and that's > what's eating the time. It might be a thought to print something to > indicate that a time-consuming step is happening; but the lack of any > feedback here isn't exactly IPC::Run's fault. > > Anyway, I can confirm Peter's statement that the current tests work even > on quite old platforms, as long as you install IPC::Run. > > So, I'm a bit confused. Is the --enable-tap-tests config setting still on the table? cheers andrew
Andrew Dunstan <andrew@dunslane.net> writes:
> On 10/30/2014 05:23 PM, Tom Lane wrote:
>> Anyway, I can confirm Peter's statement that the current tests work even
>> on quite old platforms, as long as you install IPC::Run.
> So, I'm a bit confused. Is the --enable-tap-tests config setting still
> on the table?
I think it should be. You should not have to have either prove or
IPC::Run (or, IIRC, even Perl) in order to do make check-world.
regards, tom lane
On 10/30/14, 4:55 PM, Tom Lane wrote: > Andrew Dunstan <andrew@dunslane.net> writes: >> On 10/30/2014 05:23 PM, Tom Lane wrote: >>> Anyway, I can confirm Peter's statement that the current tests work even >>> on quite old platforms, as long as you install IPC::Run. > >> So, I'm a bit confused. Is the --enable-tap-tests config setting still >> on the table? > > I think it should be. You should not have to have either prove or > IPC::Run (or, IIRC, even Perl) in order to do make check-world. Could configure detect if we have IPC::Run? ISTM it'd be nice to make this automatic instead of requiring it to be specificallyenabled. -- Jim Nasby, Data Architect, Blue Treble Consulting Data in Trouble? Get it in Treble! http://BlueTreble.com
Jim Nasby <Jim.Nasby@BlueTreble.com> writes:
> On 10/30/14, 4:55 PM, Tom Lane wrote:
>> I think it should be. You should not have to have either prove or
>> IPC::Run (or, IIRC, even Perl) in order to do make check-world.
> Could configure detect if we have IPC::Run? ISTM it'd be nice to make this automatic instead of requiring it to be
specificallyenabled.
The general philosophy we have about features enabled by configure is
exactly opposite to that: we do not for example look for Perl and then
build or not build plperl dependent on that. If you want plperl, you
tell us so, and then we either build it or fail because we can't.
You could argue that test coverage is different from features of the
completed package, but I don't really buy that.
regards, tom lane
On 10/30/14, 5:32 PM, Tom Lane wrote: > Jim Nasby <Jim.Nasby@BlueTreble.com> writes: >> On 10/30/14, 4:55 PM, Tom Lane wrote: >>> I think it should be. You should not have to have either prove or >>> IPC::Run (or, IIRC, even Perl) in order to do make check-world. > >> Could configure detect if we have IPC::Run? ISTM it'd be nice to make this automatic instead of requiring it to be specificallyenabled. > > The general philosophy we have about features enabled by configure is > exactly opposite to that: we do not for example look for Perl and then > build or not build plperl dependent on that. If you want plperl, you > tell us so, and then we either build it or fail because we can't. > > You could argue that test coverage is different from features of the > completed package, but I don't really buy that. If our policy is that tests are there primarily for developers then I agree with you. If not, then would we be OK with make check being a no-op unless you'd configured with --enable-make-check? Making this something you have to enable will seriously limit the number of people running the TAP tests, simply becausefew will know to enable them. -- Jim Nasby, Data Architect, Blue Treble Consulting Data in Trouble? Get it in Treble! http://BlueTreble.com
Jim Nasby <Jim.Nasby@BlueTreble.com> writes:
> If our policy is that tests are there primarily for developers then I agree with you.
> If not, then would we be OK with make check being a no-op unless you'd configured with --enable-make-check?
> Making this something you have to enable will seriously limit the number of people running the TAP tests, simply
becausefew will know to enable them.
Well, TBH I have no problem with that at the moment, because as Robert has
pointed out the current TAP tests are of close to zero value. The odds
that they'll find anything in the hands of Joe Random User are far lower
than the odds that they'll break Joe's build.
At some point down the road that value judgement might (hopefully will)
reverse, and then we could deal with it by making --enable-tap-tests the
default. But even then there would be a place for --disable-tap-tests.
The current situation, where the only way to disable the TAP tests is to
not do make check-world, is utterly unacceptable given their low present
usefulness and lack of proven portability.
I opined before that we should rip those tests out of the 9.4 branch
altogether. I'm willing to leave them there if we have an --enable
switch controlling them, though.
regards, tom lane
On 2014-10-30 19:30:04 -0400, Tom Lane wrote: > Jim Nasby <Jim.Nasby@BlueTreble.com> writes: > > If our policy is that tests are there primarily for developers then I agree with you. > > > If not, then would we be OK with make check being a no-op unless you'd configured with --enable-make-check? > > > Making this something you have to enable will seriously limit the number of people running the TAP tests, simply becausefew will know to enable them. > > Well, TBH I have no problem with that at the moment, because as Robert has > pointed out the current TAP tests are of close to zero value. The odds > that they'll find anything in the hands of Joe Random User are far lower > than the odds that they'll break Joe's build. I already have a couple more ready once this has stabilized... I personally don't agree that they have no value at this point. At least the pg_basebackup tests test paths that are not executed *at all* otherwise and which are not trivial. To my knowledge it's the only thing in our tests that exercises walsender and wal_level != minimal. > At some point down the road that value judgement might (hopefully will) > reverse, and then we could deal with it by making --enable-tap-tests the > default. But even then there would be a place for > --disable-tap-tests. Which would be what exactly? > The current situation, where the only way to disable the TAP tests is to > not do make check-world, is utterly unacceptable given their low present > usefulness and lack of proven portability. Agreed. > I opined before that we should rip those tests out of the 9.4 branch > altogether. I'm willing to leave them there if we have an --enable > switch controlling them, though. Hm. I'm not convinced that that's the best way, but I'm not going fight hard. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
Andres Freund <andres@2ndquadrant.com> writes:
> On 2014-10-30 19:30:04 -0400, Tom Lane wrote:
>> At some point down the road that value judgement might (hopefully will)
>> reverse, and then we could deal with it by making --enable-tap-tests the
>> default. But even then there would be a place for
>> --disable-tap-tests.
> Which would be what exactly?
Well, for example, you don't have and don't want to install IPC::Run.
Or you've tripped over some failure in the TAP tests (which might or
might not be an actual bug) but would like to complete your build anyway.
regards, tom lane
On 2014-10-30 19:53:54 -0400, Tom Lane wrote: > Andres Freund <andres@2ndquadrant.com> writes: > > On 2014-10-30 19:30:04 -0400, Tom Lane wrote: > >> At some point down the road that value judgement might (hopefully will) > >> reverse, and then we could deal with it by making --enable-tap-tests the > >> default. But even then there would be a place for > >> --disable-tap-tests. > > > Which would be what exactly? > > Well, for example, you don't have and don't want to install IPC::Run. Well, that's what the hypothetical configure test is for. I see little reason in this specific case to do anything more complicated than check for prove and IPC::Run in configure and use them if necessary. > Or you've tripped over some failure in the TAP tests (which might or > might not be an actual bug) but would like to complete your build anyway. Well. That's just the same with plain regression tests. But I guess adding --disable is essentially free, so whatever. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
Andres Freund <andres@2ndquadrant.com> writes:
> On 2014-10-30 19:53:54 -0400, Tom Lane wrote:
>> Well, for example, you don't have and don't want to install IPC::Run.
> Well, that's what the hypothetical configure test is for. I see little
> reason in this specific case to do anything more complicated than check
> for prove and IPC::Run in configure and use them if necessary.
As I said upthread, that approach seems to me to be contrary to the
project policy about how configure should behave. If you have selected
(or, someday, defaulted to) --enable-tap-tests, configure should *fail*
if you don't have the tools to run the tests. Not silently disable tests
that we have decided are valuable. How exactly would that be different
from silently omitting readline support if we don't find that library?
regards, tom lane
On Thu, Oct 30, 2014 at 04:54:25PM +0100, Andres Freund wrote: > On 2014-10-30 01:57:15 -0400, Noah Misch wrote: > > On Wed, Oct 29, 2014 at 08:14:07PM -0400, Peter Eisentraut wrote: > > > On 10/28/14 9:09 PM, Peter Eisentraut wrote: > > > > I have looked into IPC::Cmd, but the documentation keeps telling me that > > > > to do anything interesting I have to have IPC::Run anyway. I'll give it > > > > a try, though. > > > > > > I tried this, but I'm not optimistic about it. While parts of IPC::Cmd > > > are actually a bit nicer than IPC::Run, other parts are weird. For > > > example, with most packages and functions in Perl that run a command, > > > you can pass the command as a string or as a list (or array reference). > > > The latter is preferred because it avoids issues with quoting, word > > > splitting, spaces, etc. In IPC::Run, I can use the "run" function in > > > the latter way, but I cannot use the "run_forked" function like that, > > > and I need that one to get the exit code of a command. It's possible to > > > work around that, but I'm getting the feeling that this is not very well > > > designed. > > > > Ick; I concur with your judgment on those aspects of the IPC::Cmd design. > > Thanks for investigating. So, surviving options include: > > > > 1. Require IPC::Run. > > 2. Write our own run() that reports the raw exit code. > > 3. Distill the raw exit code from the IPC::Cmd::run() error string. > > 4. Pass IPC::Run::run_forked() a subroutine that execs an argument list. > > > > Any others worth noting? > > 5. Include a copy of IPC::Run in the repository till it's common enough. True. I eliminated that one because the license of IPC::Run is, like Perl itself, GPL+Artistic. Right now, we presume that the entire PostgreSQL tarball is distributable under the PostgreSQL License. The benefit of bundling IPC::Run is not strong enough to justify muddying that.
On 2014-10-30 20:13:53 -0400, Tom Lane wrote: > Andres Freund <andres@2ndquadrant.com> writes: > > On 2014-10-30 19:53:54 -0400, Tom Lane wrote: > >> Well, for example, you don't have and don't want to install IPC::Run. > > > Well, that's what the hypothetical configure test is for. I see little > > reason in this specific case to do anything more complicated than check > > for prove and IPC::Run in configure and use them if necessary. > > As I said upthread, that approach seems to me to be contrary to the > project policy about how configure should behave. I don't think that holds much water. There's a fair amount of things that configure detects automatically. I don't think the comparison to plperl or such is meaningful - that's a runtime/install time difference. These tests are not. We e.g. detect compiler support for certain features that result in possible speedups and/or better warnings. we detect wether bison is available... > If you have selected > (or, someday, defaulted to) --enable-tap-tests, configure should *fail* > if you don't have the tools to run the tests. Not silently disable tests > that we have decided are valuable. How exactly would that be different > from silently omitting readline support if we don't find that library? Because it doesn't result in a user visible regression? Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
Andres Freund <andres@2ndquadrant.com> writes: > On 2014-10-30 20:13:53 -0400, Tom Lane wrote: >> As I said upthread, that approach seems to me to be contrary to the >> project policy about how configure should behave. > I don't think that holds much water. There's a fair amount of things > that configure detects automatically. I don't think the comparison to > plperl or such is meaningful - that's a runtime/install time > difference. These tests are not. Meh. Right now, it's easy to dismiss these tests as unimportant, figuring that they play little part in whether the completed build is reliable. But that may not always be true. If they do become a significant part of our test arsenal, silently omitting them will not be cool for configure to do. We think that it's okay to autoconfigure things like which semaphore technology to use in part because we believe we can test the results. I think if someone asks for "make check-world", it should be pretty deterministic what that means. Historical note: I was not originally very much on board with the strict enable-what-you-want policy for configure behavior, but I got religion after working at Red Hat for awhile. Nondeterministic package build behaviors *suck*. Here's one example: https://bugzilla.redhat.com/show_bug.cgi?id=427063 >> If you have selected >> (or, someday, defaulted to) --enable-tap-tests, configure should *fail* >> if you don't have the tools to run the tests. Not silently disable tests >> that we have decided are valuable. How exactly would that be different >> from silently omitting readline support if we don't find that library? > Because it doesn't result in a user visible regression? Uncaught bugs become user-visible regressions. regards, tom lane
On 2014-10-30 21:03:43 -0400, Tom Lane wrote: > Andres Freund <andres@2ndquadrant.com> writes: > > On 2014-10-30 20:13:53 -0400, Tom Lane wrote: > >> As I said upthread, that approach seems to me to be contrary to the > >> project policy about how configure should behave. > > > I don't think that holds much water. There's a fair amount of things > > that configure detects automatically. I don't think the comparison to > > plperl or such is meaningful - that's a runtime/install time > > difference. These tests are not. > > Meh. Right now, it's easy to dismiss these tests as unimportant, > figuring that they play little part in whether the completed build > is reliable. But that may not always be true. If they do become > a significant part of our test arsenal, silently omitting them will > not be cool for configure to do. Well, I'm all for erroring out if somebody passed --enable-foo-tests and the prerequisites aren't there. What I *am* against is requiring an explicit flag to enable them because then they'll just not be run in enough environments. And that's what's much more likely to cause unnoticed bugs. > Historical note: I was not originally very much on board with the strict > enable-what-you-want policy for configure behavior, but I got religion > after working at Red Hat for awhile. Nondeterministic package build > behaviors *suck*. Here's one example: > https://bugzilla.redhat.com/show_bug.cgi?id=427063 Sure, but that's about a difference that's meaningful once the package/software is installed. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
Andres Freund <andres@2ndquadrant.com> writes:
> On 2014-10-30 21:03:43 -0400, Tom Lane wrote:
>> Meh. Right now, it's easy to dismiss these tests as unimportant,
>> figuring that they play little part in whether the completed build
>> is reliable. But that may not always be true. If they do become
>> a significant part of our test arsenal, silently omitting them will
>> not be cool for configure to do.
> Well, I'm all for erroring out if somebody passed --enable-foo-tests and
> the prerequisites aren't there. What I *am* against is requiring an
> explicit flag to enable them because then they'll just not be run in
> enough environments. And that's what's much more likely to cause
> unnoticed bugs.
Once they're at the point where they're actually likely to catch stuff
of interest, I'll be all for enabling them by default. What I don't
want is for them to run or not run based on environmental happenstance.
It's way too easy for that to result in them not getting run even though
the user thought they were getting run; and if that results in a bug
not getting caught, it's just as bad as a missing run-time feature.
regards, tom lane
On 2014-10-30 21:24:04 -0400, Tom Lane wrote: > Andres Freund <andres@2ndquadrant.com> writes: > > On 2014-10-30 21:03:43 -0400, Tom Lane wrote: > >> Meh. Right now, it's easy to dismiss these tests as unimportant, > >> figuring that they play little part in whether the completed build > >> is reliable. But that may not always be true. If they do become > >> a significant part of our test arsenal, silently omitting them will > >> not be cool for configure to do. > > > Well, I'm all for erroring out if somebody passed --enable-foo-tests and > > the prerequisites aren't there. What I *am* against is requiring an > > explicit flag to enable them because then they'll just not be run in > > enough environments. And that's what's much more likely to cause > > unnoticed bugs. > > Once they're at the point where they're actually likely to catch stuff > of interest, I'll be all for enabling them by default. Great. We already are at that point due to the pg_basebackup tests. If we slightly extend it to also start up the newly made base backups we will have the first minimal automated test of recovery... Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
On 10/30/2014 09:37 PM, Andres Freund wrote: > On 2014-10-30 21:24:04 -0400, Tom Lane wrote: >> Andres Freund <andres@2ndquadrant.com> writes: >>> On 2014-10-30 21:03:43 -0400, Tom Lane wrote: >>>> Meh. Right now, it's easy to dismiss these tests as unimportant, >>>> figuring that they play little part in whether the completed build >>>> is reliable. But that may not always be true. If they do become >>>> a significant part of our test arsenal, silently omitting them will >>>> not be cool for configure to do. >>> Well, I'm all for erroring out if somebody passed --enable-foo-tests and >>> the prerequisites aren't there. What I *am* against is requiring an >>> explicit flag to enable them because then they'll just not be run in >>> enough environments. And that's what's much more likely to cause >>> unnoticed bugs. >> Once they're at the point where they're actually likely to catch stuff >> of interest, I'll be all for enabling them by default. > Great. We already are at that point due to the pg_basebackup > tests. > If we slightly extend it to also start up the newly made base backups we > will have the first minimal automated test of recovery... > There are other issues. I am not going to enable this in the buildfarm until the check test can work from a single install. It's insane for the bin tests to take an order of magnitude longer than the main regression suite. cheers andrew
Andrew Dunstan <andrew@dunslane.net> writes:
> There are other issues. I am not going to enable this in the buildfarm
> until the check test can work from a single install. It's insane for the
> bin tests to take an order of magnitude longer than the main regression
> suite.
I think the installs as such aren't the only reason for the sucky
performance. We need to also reduce the number of initdb cycles incurred
by the TAP tests. It's useless for example that the pg_controldata test
creates its very own $PGDATA rather than sharing one with other tests.
This line of thought implies that the tests will become less independent
of each other, which will probably result in them being a bit harder to
maintain. Still, we are paying an awful lot of cycles for not much, as
things stand at the moment.
A couple other random ideas for shaving cycles:
* Use initdb -N (no fsync) where we do need to initdb.
* We probably don't need a full install tree for these types of tests;
it's tempting for instance to omit installing the include/ tree. That
wouldn't save a large number of megabytes but it is a sizable number of
files, so it might cut the install/rm -rf time noticeably.
* In the same line, suppressing install of the timezone database file
tree would possibly save a useful number of cycles. We do need to have
that data for functionality, but buildfarm owners could be encouraged
to use --with-system-tzdata to shave install cycles.
regards, tom lane
Peter Eisentraut <peter_e@gmx.net> writes:
> On 10/7/14 1:57 PM, Tom Lane wrote:
>> Peter had a patch to eliminate the overhead of multiple subinstalls;
>> not sure where that stands, but presumably it would address your issue.
> It will need some cleverness to sort out the parallel make issues that
> were brought up in the review thread.
I took a quick look. I concur with Fabien that the dependency on
MAKELEVEL seems pretty horrid: in particular, will that not break the
ability to initiate "make check" from somewhere below the top directory?
I wonder whether it could be solved by having code in the toplevel
Makefile that actually makes the test install tree, and not as a .PHONY
target but along the lines of
tmp-install-stamp:rm -rf tmp_install # in case we failed during a previous attempt$(MKDIR_P) tmp_install/log$(MAKE)
...etc etc ...touch tmp-install-stamp
and then also at top level, put tmp-install-stamp as a prerequisite
for check-world, and then in every subdirectory that has a check
target, add something like
$(abs_top_builddir)/tmp-install-stamp:$(MAKE) -C $(abs_top_builddir) tmp-install-stamp
check: $(abs_top_builddir)/tmp-install-stamp
The way this solves the parallel make problem is that no matter
where you invoke "make check", the first thing it would have to
do is create the tmp_install directory if it's not done already,
before it can launch any parallel operations. Or at least I hope
it'd work like that; I've not actually tried it.
Possibly some of these rules could be kept in Makefile.global
so as to avoid having to touch so many individual Makefiles.
regards, tom lane
I wrote:
> I took a quick look. I concur with Fabien that the dependency on
> MAKELEVEL seems pretty horrid: in particular, will that not break the
> ability to initiate "make check" from somewhere below the top directory?
Another use-case that seems to be broken both by Peter's patch and my
proposed variant is
configure ... make ... cd src/bin/something hack hack hack make make check ooops ... hack some more make make check
Neither proposed approach would result in reinstalling the updated binary
you just built into the common "temp install" tree, so the second "make
check" step would just retest the old binary.
The simplest solution that comes to mind is to define a "temp-install"
target that is the same as "make install" except it installs the stuff
built in the current directory into the "temp install" tree instead of the
configured installation target tree. You'd have to remember to do "make
temp-install" before a "make check"; but it would work, and it's not so
different from "make install" then "make installcheck", which is what you
usually do for this use-case if you've got the tree configured to install
into a throwaway installation location.
regards, tom lane
On Thu, Oct 30, 2014 at 10:49:33PM -0400, Tom Lane wrote: > Andrew Dunstan <andrew@dunslane.net> writes: > > There are other issues. I am not going to enable this in the buildfarm > > until the check test can work from a single install. It's insane for the > > bin tests to take an order of magnitude longer than the main regression > > suite. > > I think the installs as such aren't the only reason for the sucky > performance. We need to also reduce the number of initdb cycles incurred > by the TAP tests. It's useless for example that the pg_controldata test > creates its very own $PGDATA rather than sharing one with other tests. > > This line of thought implies that the tests will become less independent > of each other, which will probably result in them being a bit harder to > maintain. Still, we are paying an awful lot of cycles for not much, as > things stand at the moment. One could memoize initdb within the suite. Call it once per distinct command line, caching the resulting data directory. Copy the cached data directory for each test desiring one.
Noah Misch <noah@leadboat.com> writes:
> On Thu, Oct 30, 2014 at 10:49:33PM -0400, Tom Lane wrote:
>> I think the installs as such aren't the only reason for the sucky
>> performance. We need to also reduce the number of initdb cycles incurred
>> by the TAP tests. It's useless for example that the pg_controldata test
>> creates its very own $PGDATA rather than sharing one with other tests.
> One could memoize initdb within the suite. Call it once per distinct command
> line, caching the resulting data directory. Copy the cached data directory
> for each test desiring one.
At least on older/slower machines like prairiedog, even having to copy
$PGDATA for each test is unappealing. Some numbers for reference:
make install 22 sec
initdb 76 sec
initdb -N 33 sec
cp -r $PGDATA /tmp 17 sec
regards, tom lane
On 10/30/2014 09:17 PM, Andres Freund wrote: > On 2014-10-30 21:03:43 -0400, Tom Lane wrote: >> Andres Freund <andres@2ndquadrant.com> writes: >>> On 2014-10-30 20:13:53 -0400, Tom Lane wrote: >>>> As I said upthread, that approach seems to me to be contrary to the >>>> project policy about how configure should behave. >>> I don't think that holds much water. There's a fair amount of things >>> that configure detects automatically. I don't think the comparison to >>> plperl or such is meaningful - that's a runtime/install time >>> difference. These tests are not. >> Meh. Right now, it's easy to dismiss these tests as unimportant, >> figuring that they play little part in whether the completed build >> is reliable. But that may not always be true. If they do become >> a significant part of our test arsenal, silently omitting them will >> not be cool for configure to do. > Well, I'm all for erroring out if somebody passed --enable-foo-tests and > the prerequisites aren't there. What I *am* against is requiring an > explicit flag to enable them because then they'll just not be run in > enough environments. And that's what's much more likely to cause > unnoticed bugs. When this is properly sorted out I will enable this in the buildfarm default configuration. So I don't think that's going to be an issue in the long term. cheers andrew
On Thu, Oct 30, 2014 at 01:57:15AM -0400, Noah Misch wrote: > On Wed, Oct 29, 2014 at 08:14:07PM -0400, Peter Eisentraut wrote: > > On 10/28/14 9:09 PM, Peter Eisentraut wrote: > > > I have looked into IPC::Cmd, but the documentation keeps telling me that > > > to do anything interesting I have to have IPC::Run anyway. I'll give it > > > a try, though. > > > > I tried this, but I'm not optimistic about it. While parts of IPC::Cmd > > are actually a bit nicer than IPC::Run, other parts are weird. For > > example, with most packages and functions in Perl that run a command, > > you can pass the command as a string or as a list (or array reference). > > The latter is preferred because it avoids issues with quoting, word > > splitting, spaces, etc. In IPC::Run, I can use the "run" function in > > the latter way, but I cannot use the "run_forked" function like that, > > and I need that one to get the exit code of a command. It's possible to > > work around that, but I'm getting the feeling that this is not very well > > designed. > > Ick; I concur with your judgment on those aspects of the IPC::Cmd design. > Thanks for investigating. So, surviving options include: > > 1. Require IPC::Run. > 2. Write our own run() that reports the raw exit code. > 3. Distill the raw exit code from the IPC::Cmd::run() error string. > 4. Pass IPC::Run::run_forked() a subroutine that execs an argument list. FWIW, (3) looks most promising to me. That is to say, implement a reverse of IPC::Cmd::_pp_child_error(). Ugly to be sure, but the wart can be small and self-contained.
On 11/2/14 2:00 PM, Noah Misch wrote: >> Ick; I concur with your judgment on those aspects of the IPC::Cmd design. >> Thanks for investigating. So, surviving options include: >> >> 1. Require IPC::Run. >> 2. Write our own run() that reports the raw exit code. >> 3. Distill the raw exit code from the IPC::Cmd::run() error string. >> 4. Pass IPC::Run::run_forked() a subroutine that execs an argument list. > > FWIW, (3) looks most promising to me. That is to say, implement a reverse of > IPC::Cmd::_pp_child_error(). Ugly to be sure, but the wart can be small and > self-contained. I thank you for this research, but I suggest that we ship 9.4 as is, that is with requiring IPC::Run and --enable-* option. All the possible alternatives will clearly need more rounds of portability testing. We can then evaluate these changes for 9.5 in peace.
Peter Eisentraut <peter_e@gmx.net> writes:
> I thank you for this research, but I suggest that we ship 9.4 as is,
> that is with requiring IPC::Run and --enable-* option. All the possible
> alternatives will clearly need more rounds of portability testing. We
> can then evaluate these changes for 9.5 in peace.
I concur --- it's time to get 9.4 out, and the current state of affairs
is Good Enough imo.
regards, tom lane
On 10/29/14 8:42 AM, Robert Haas wrote: > I'm sympathetic to that line of reasoning, but I really think that if > you want to keep this infrastructure, it needs to be made portable. Let me clarify that this was my intention. I have looked at many test frameworks, many of which are much nicer than what we have, but the portability and dependency implications for this project would have been between shocking and outrageous. I settled for what I felt was the absolute minimum: Perl + IPC::Run. It was only later on that I learned that 1) subtests don't work in Perl 5.10, and 2) subtests are broken in Perl 5.12. So we removed the use of subtests and now we are back at the baseline I started with. The irony in this whole story is that if we had thrown this onto the build farm right away, we might have found and fixed these problems within a week instead of five months later.
On 11/03/2014 04:44 PM, Peter Eisentraut wrote: > On 10/29/14 8:42 AM, Robert Haas wrote: >> I'm sympathetic to that line of reasoning, but I really think that if >> you want to keep this infrastructure, it needs to be made portable. > Let me clarify that this was my intention. I have looked at many test > frameworks, many of which are much nicer than what we have, but the > portability and dependency implications for this project would have been > between shocking and outrageous. I settled for what I felt was the > absolute minimum: Perl + IPC::Run. It was only later on that I learned > that 1) subtests don't work in Perl 5.10, and 2) subtests are broken in > Perl 5.12. So we removed the use of subtests and now we are back at the > baseline I started with. > > The irony in this whole story is that if we had thrown this onto the > build farm right away, we might have found and fixed these problems > within a week instead of five months later. > > An email note to me would probably have done the trick. I've missed one or two things lately, since my mind has been to some extent elsewhere, and don't mind a little memory jog. cheers andrew
On Mon, Nov 03, 2014 at 04:40:39PM -0500, Tom Lane wrote: > Peter Eisentraut <peter_e@gmx.net> writes: > > I thank you for this research, but I suggest that we ship 9.4 as is, > > that is with requiring IPC::Run and --enable-* option. All the possible > > alternatives will clearly need more rounds of portability testing. We > > can then evaluate these changes for 9.5 in peace. > > I concur --- it's time to get 9.4 out, and the current state of affairs > is Good Enough imo. Agreed; any release-blocking aspects of this test suite are behind us.
On Mon, Nov 3, 2014 at 4:44 PM, Peter Eisentraut <peter_e@gmx.net> wrote: > On 10/29/14 8:42 AM, Robert Haas wrote: >> I'm sympathetic to that line of reasoning, but I really think that if >> you want to keep this infrastructure, it needs to be made portable. > > Let me clarify that this was my intention. I have looked at many test > frameworks, many of which are much nicer than what we have, but the > portability and dependency implications for this project would have been > between shocking and outrageous. I settled for what I felt was the > absolute minimum: Perl + IPC::Run. It was only later on that I learned > that 1) subtests don't work in Perl 5.10, and 2) subtests are broken in > Perl 5.12. So we removed the use of subtests and now we are back at the > baseline I started with. Thanks. At last check, which I think was approximately last week, the tests were running and passing on my machine. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Re: Peter Eisentraut 2014-11-03 <5457F54E.4020805@gmx.net> > On 11/2/14 2:00 PM, Noah Misch wrote: > >> Ick; I concur with your judgment on those aspects of the IPC::Cmd design. > >> Thanks for investigating. So, surviving options include: > >> > >> 1. Require IPC::Run. > >> 2. Write our own run() that reports the raw exit code. > >> 3. Distill the raw exit code from the IPC::Cmd::run() error string. > >> 4. Pass IPC::Run::run_forked() a subroutine that execs an argument list. > > > > FWIW, (3) looks most promising to me. That is to say, implement a reverse of > > IPC::Cmd::_pp_child_error(). Ugly to be sure, but the wart can be small and > > self-contained. > > I thank you for this research, but I suggest that we ship 9.4 as is, > that is with requiring IPC::Run and --enable-* option. All the possible > alternatives will clearly need more rounds of portability testing. We > can then evaluate these changes for 9.5 in peace. Hrm. I spent some effort into getting the TAP tests to run on 9.4beta for Debian, and I've only now learned that 9.4.0 doesn't run them unless I say --enable-tap-tests. A short note to -packagers would have been nice, for a change so late in the release cycle... Christoph -- cb@df7cb.de | http://www.df7cb.de/