Обсуждение: Portability issues in TAP tests
My Salesforce colleagues have been complaining that the TAP tests added
in 9.4 don't work terribly well for them. I've been poking at this,
and I believe this is a reasonably complete list of the problems:
1. "make [install]check-world" tries to run the TAP tests even when
"prove" was not found by configure. I regard this as a stop-ship issue
for 9.4; "prove" is not part of a basic Perl installation, and even if
it were, we don't require Perl to build from a tarball.
2. Most of the tests fail in "make check" mode, unless you already did
"make install", because of failures to load libpq.so. The right fix for
this seems to be to modify LD_LIBRARY_PATH and friends to include the temp
install's libdir, comparably to what pg_regress.c does (lines 903ff in
HEAD). Unfortunately we can't just borrow that code since there is no
C wrapper involved in the TAP tests. (Maybe there should be?) This again
seems like a stop-ship issue.
3. Many of the tests depend on Test::More's "subtest" feature, which
unfortunately is of rather recent vintage (2009 or so). It's not present
for example in RHEL6's version of Test::More, and presumably not in any
older distros either. I'm not sure if we should consider it acceptable
to depend on this feature: it doesn't seem exactly critical, and if we
continue to use it, there is approximately 0 chance that we'll ever be
able to enable the TAP tests on many of the existing buildfarm members.
In any case, as long as we depend on it, there had better be some cleaner
response to older versions of Test::More than just blowing up.
4. IPC::Run isn't installed by default on RHEL, and probably not on other
distros either. If there's a reasonably painless way to remove this
dependency, it'd improve the portability of the tests. This is lower
priority than the previous items, for sure.
regards, tom lane
On Thu, Jul 17, 2014 at 03:31:19PM -0400, Tom Lane wrote: > My Salesforce colleagues have been complaining that the TAP tests added > in 9.4 don't work terribly well for them. I've been poking at this, > and I believe this is a reasonably complete list of the problems: > 3. Many of the tests depend on Test::More's "subtest" feature, which > unfortunately is of rather recent vintage (2009 or so). It's not present > for example in RHEL6's version of Test::More, and presumably not in any > older distros either. I'm not sure if we should consider it acceptable > to depend on this feature: it doesn't seem exactly critical, and if we > continue to use it, there is approximately 0 chance that we'll ever be > able to enable the TAP tests on many of the existing buildfarm members. Installing a new version of one Perl module is well within the capabilities of buildfarm owners. Saving them the trouble, which in turn means more of them actually activating the TAP tests, might justify the loss. I'd be sad to see the "subtest" use go away, but I lean toward thinking it's for the best. From a technical standpoint, it would be nicest to bundle copies of Test::More and IPC::Run for the test suites to use. Unfortunately, their licenses are more restrictive than the PostgreSQL license. Complicating the license situation of the PostgreSQL tarball more than cancels out the benefit. A tractable alternative is to give the buildfarm client an option to setup a private installation of the necessary modules for itself. It does feel like cheating, though, to make the buildfarm client paper over a usability challenge in the core distribution. Not sure. > In any case, as long as we depend on it, there had better be some cleaner > response to older versions of Test::More than just blowing up. +1 for, if nothing else, putting a version number in the "use Test::More" statement of TestLib. > 4. IPC::Run isn't installed by default on RHEL, and probably not on other > distros either. If there's a reasonably painless way to remove this > dependency, it'd improve the portability of the tests. This is lower > priority than the previous items, for sure. Perl 5.10 and later incorporate IPC::Cmd, a weaker counterpart to IPC::Run. It looked promising as a replacement when I examined the matter briefly. Why do you estimate the priority of (4) well below the priority of (3)? A minor point to add to your list: 5. The TAP suites don't work when $PWD contains spaces. Thanks, nm -- Noah Misch EnterpriseDB http://www.enterprisedb.com
Noah Misch <noah@leadboat.com> writes:
> On Thu, Jul 17, 2014 at 03:31:19PM -0400, Tom Lane wrote:
>> 4. IPC::Run isn't installed by default on RHEL, and probably not on other
>> distros either. If there's a reasonably painless way to remove this
>> dependency, it'd improve the portability of the tests. This is lower
>> priority than the previous items, for sure.
> Perl 5.10 and later incorporate IPC::Cmd, a weaker counterpart to IPC::Run.
> It looked promising as a replacement when I examined the matter briefly. Why
> do you estimate the priority of (4) well below the priority of (3)?
Well, "yum install perl-IPC-Run" fixes that problem on RHEL6. Installing
a more modern version of Test::More is at least one order of magnitude
harder technically; and it might involve political/managerial hurdles as
well, for people who face policies against installing not-supported-by-vendor
packages.
regards, tom lane
Re: Noah Misch 2014-07-18 <20140718052625.GA2231360@tornado.leadboat.com> > Installing a new version of one Perl module is well within the capabilities of > buildfarm owners. Saving them the trouble, which in turn means more of them > actually activating the TAP tests, might justify the loss. I'd be sad to see > the "subtest" use go away, but I lean toward thinking it's for the best. > > From a technical standpoint, it would be nicest to bundle copies of Test::More > and IPC::Run for the test suites to use. Please not. If no OS packages are available, there's still "cpan IPC::Run" to get it installed to $HOME automatically. > A minor point to add to your list: > > 5. The TAP suites don't work when $PWD contains spaces. Here's yet another thing that I think is pretty major: 6. The tests fail if your $LANG isn't en_something: ok 1 - vacuumdb -z postgres exit code 0 not ok 2 - vacuumdb -z: SQL found in server log # Failed test 'vacuumdb-z: SQL found in server log' # at /home/cbe/projects/postgresql/postgresql/9.4/build/../src/test/perl/TestLib.pmline 221. # 'LOG: Anweisung:VACUUM (ANALYZE); # ' # doesn't match '(?^:statement: VACUUM \(ANALYZE\);)' # Looks like you failed 1test of 2. ... repeated a gazillion times. pg_regress unsets LANG and LC_*, the perl tests should do also. Christoph -- cb@df7cb.de | http://www.df7cb.de/
On Mon, Jul 21, 2014 at 10:06 AM, Christoph Berg <cb@df7cb.de> wrote: > Re: Noah Misch 2014-07-18 <20140718052625.GA2231360@tornado.leadboat.com> >> Installing a new version of one Perl module is well within the capabilities of >> buildfarm owners. Saving them the trouble, which in turn means more of them >> actually activating the TAP tests, might justify the loss. I'd be sad to see >> the "subtest" use go away, but I lean toward thinking it's for the best. >> >> From a technical standpoint, it would be nicest to bundle copies of Test::More >> and IPC::Run for the test suites to use. > > Please not. If no OS packages are available, there's still "cpan IPC::Run" > to get it installed to $HOME automatically. > >> A minor point to add to your list: >> >> 5. The TAP suites don't work when $PWD contains spaces. > > Here's yet another thing that I think is pretty major: > > 6. The tests fail if your $LANG isn't en_something: > > ok 1 - vacuumdb -z postgres exit code 0 > not ok 2 - vacuumdb -z: SQL found in server log > > # Failed test 'vacuumdb -z: SQL found in server log' > # at /home/cbe/projects/postgresql/postgresql/9.4/build/../src/test/perl/TestLib.pm line 221. > # 'LOG: Anweisung: VACUUM (ANALYZE); > # ' > # doesn't match '(?^:statement: VACUUM \(ANALYZE\);)' > # Looks like you failed 1 test of 2. > > ... repeated a gazillion times. pg_regress unsets LANG and LC_*, the > perl tests should do also. While we're complaining... The tests weren't running for me at all on MacOS X, because I was missing some prerequisite. So I installed it, and now they promptly fail: 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 Unlike the regular regression tests, these tests don't seem to provide any guidance on where to see the difference between the expected and actual output, or how to get more details, so that's all I know. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Mon, Jul 21, 2014 at 8:21 AM, Robert Haas <robertmhaas@gmail.com> wrote: > The tests weren't running for me at all on MacOS X, because I was > missing some prerequisite. So I installed it, and now they promptly > fail: > > ok 2 - initdb --version > 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 I've had the same issue. The error doesn't seem to come from a failed test, but from within 'prove' itself. Installing Perl 5.18 via Homebrew, and setting PROVE="<path-to-your-cellar>/perl518/5.18.2/prove" at configure time seems to help avoid this particular problem. So I did not attempt any further diagnosis of the root cause.
On Mon, Jul 21, 2014 at 12:00 PM, Thomas Fanghaenel <tfanghaenel@salesforce.com> wrote: > On Mon, Jul 21, 2014 at 8:21 AM, Robert Haas <robertmhaas@gmail.com> wrote: >> The tests weren't running for me at all on MacOS X, because I was >> missing some prerequisite. So I installed it, and now they promptly >> fail: >> >> ok 2 - initdb --version >> 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 > > I've had the same issue. The error doesn't seem to come from a failed > test, but from within 'prove' itself. Installing Perl 5.18 via > Homebrew, and setting > PROVE="<path-to-your-cellar>/perl518/5.18.2/prove" at configure time > seems to help avoid this particular problem. So I did not attempt any > further diagnosis of the root cause. Mmph. Well, I don't want to install a non-default Perl on my system just to make these tests pass, and I don't think that should be a requirement. (But thanks for the information.) -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Robert Haas <robertmhaas@gmail.com> writes:
> Mmph. Well, I don't want to install a non-default Perl on my system
> just to make these tests pass, and I don't think that should be a
> requirement.
I had the same feeling about the Perl on RHEL6 ;-). The TAP tests
will need to be a great deal more portable than they've proven so far
before they'll really be useful for anything much. I trust we're
committed to making that happen.
regards, tom lane
On 07/21/2014 02:40 PM, Tom Lane wrote: > Robert Haas <robertmhaas@gmail.com> writes: >> Mmph. Well, I don't want to install a non-default Perl on my system >> just to make these tests pass, and I don't think that should be a >> requirement. > I had the same feeling about the Perl on RHEL6 ;-). The TAP tests > will need to be a great deal more portable than they've proven so far > before they'll really be useful for anything much. I trust we're > committed to making that happen. > > I'd be rather inclined to remove them from the check-world and installcheck-world targets until we have dealt with the issues people have identified. cheers andrew
Andrew Dunstan <andrew@dunslane.net> writes:
> On 07/21/2014 02:40 PM, Tom Lane wrote:
>> I had the same feeling about the Perl on RHEL6 ;-). The TAP tests
>> will need to be a great deal more portable than they've proven so far
>> before they'll really be useful for anything much. I trust we're
>> committed to making that happen.
> I'd be rather inclined to remove them from the check-world and
> installcheck-world targets until we have dealt with the issues people
> have identified.
I think it would be reasonable to do that in the 9.4 branch, since
it's a bit hard to argue that the TAP tests are production grade
at the moment. We could leave them there in HEAD.
regards, tom lane
On Mon, Jul 21, 2014 at 11:39 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Andrew Dunstan <andrew@dunslane.net> writes: >> On 07/21/2014 02:40 PM, Tom Lane wrote: >>> I had the same feeling about the Perl on RHEL6 ;-). The TAP tests >>> will need to be a great deal more portable than they've proven so far >>> before they'll really be useful for anything much. I trust we're >>> committed to making that happen. > >> I'd be rather inclined to remove them from the check-world and >> installcheck-world targets until we have dealt with the issues people >> have identified. > > I think it would be reasonable to do that in the 9.4 branch, since > it's a bit hard to argue that the TAP tests are production grade > at the moment. We could leave them there in HEAD. That doesn't do developers who can't run the tests in their environment much good: most people develop against master. Maybe that's the point: to get these fixed. But it's pretty annoying that I can no longer do 'make world'. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On 7/17/14 3:31 PM, Tom Lane wrote: > My Salesforce colleagues have been complaining that the TAP tests added > in 9.4 don't work terribly well for them. I've been poking at this, > and I believe this is a reasonably complete list of the problems: Quick followup: > 1. "make [install]check-world" tries to run the TAP tests even when > "prove" was not found by configure. I regard this as a stop-ship issue > for 9.4; "prove" is not part of a basic Perl installation, and even if > it were, we don't require Perl to build from a tarball. I just sent in a proposed patch for that. > 2. Most of the tests fail in "make check" mode, unless you already did > "make install", because of failures to load libpq.so. The right fix for > this seems to be to modify LD_LIBRARY_PATH and friends This was fixed. > 3. Many of the tests depend on Test::More's "subtest" feature, This was changed so it doesn't fail anymore. Discussion is ongoing about whether to keep using that feature, but it's not a fatal error anymore. > 4. IPC::Run isn't installed by default on RHEL, and probably not on other > distros either. If there's a reasonably painless way to remove this > dependency, it'd improve the portability of the tests. This is lower > priority than the previous items, for sure. Same as above.
On 7/21/14 10:06 AM, Christoph Berg wrote: > 6. The tests fail if your $LANG isn't en_something: This was fixed.