Portability issues in TAP tests

Started by Tom Laneover 11 years ago13 messages
#1Tom Lane
tgl@sss.pgh.pa.us

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

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

#2Noah Misch
noah@leadboat.com
In reply to: Tom Lane (#1)
Re: Portability issues in TAP tests

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

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

#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: Noah Misch (#2)
Re: Portability issues in TAP tests

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

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

#4Christoph Berg
cb@df7cb.de
In reply to: Noah Misch (#2)
Re: Portability issues in TAP tests

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.

Christoph
--
cb@df7cb.de | http://www.df7cb.de/

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

#5Robert Haas
robertmhaas@gmail.com
In reply to: Christoph Berg (#4)
Re: Portability issues in TAP tests

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 option prints 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

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

#6Thomas Fanghaenel
tfanghaenel@salesforce.com
In reply to: Robert Haas (#5)
Re: Portability issues in TAP tests

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.

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

#7Robert Haas
robertmhaas@gmail.com
In reply to: Thomas Fanghaenel (#6)
Re: Portability issues in TAP tests

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

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

#8Tom Lane
tgl@sss.pgh.pa.us
In reply to: Robert Haas (#7)
Re: Portability issues in TAP tests

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

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

#9Andrew Dunstan
andrew@dunslane.net
In reply to: Tom Lane (#8)
Re: Portability issues in TAP tests

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

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

#10Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andrew Dunstan (#9)
Re: Portability issues in TAP tests

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

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

#11Robert Haas
robertmhaas@gmail.com
In reply to: Tom Lane (#10)
Re: Portability issues in TAP tests

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

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

#12Peter Eisentraut
peter_e@gmx.net
In reply to: Tom Lane (#1)
Re: Portability issues in TAP tests

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.

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

#13Peter Eisentraut
peter_e@gmx.net
In reply to: Christoph Berg (#4)
Re: Portability issues in TAP tests

On 7/21/14 10:06 AM, Christoph Berg wrote:

6. The tests fail if your $LANG isn't en_something:

This was fixed.

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