Review: tests for client programs
Hello
This patch creates a infrastructure for client side testing environment
1. Surely we want this patch, this infrastructure - lot of important
functionality is not covered by tests - pg_dump, pg_basebackup, ... Without
infrastructure is terrible work to create some tests.
2. This patch does exactly what was proposed
3. New functionality has zero impact on our server side or client side
software (performance, quality, readability, ..)
4. I was able to use this patch cleanly and it works
5. I didn't test it on windows
6. I found only few issues:
a) Configure doesn't check a required IRC::Run module
b) Prepared tests fails when PostgreSQL server was up - should be checked
and should to raise a valuable error message
c) I am not sure if infrastructure is enough - for test pg_dump I miss a
comparation of produced file with some other expected file. This objection
is not blocker - someone can enhance it (when will write tests of pg_dump
for example).
Regards
Pavel Stehule
2014-01-29 Pavel Stehule <pavel.stehule@gmail.com>:
Show quoted text
Hello
I am looking on this patch.
It is great idea, and I am sure, so we want this patch - it was requested
and proposed more time.Some tips:
a) possibility to test only selected tests
b) possibility to verify generated file against expected file
c) detection some warnings (expected/unexpected)p.s. some tests fails when other Postgres is up. It should be checked on
start.and raise warning or stop testing.Regards
Pavel
ok 4 - pg_ctl initdb
waiting for server to start....LOG: could not bind IPv6 socket: Address
already in use
HINT: Is another postmaster already running on port 5432? If not, wait a
few seconds and retry.
LOG: could not bind IPv4 socket: Address already in use
HINT: Is another postmaster already running on port 5432? If not, wait a
few seconds and retry.
WARNING: could not create listen socket for "localhost"
FATAL: could not create any TCP/IP sockets
.... stopped waiting
pg_ctl: could not start server
Examine the log output.
not ok 5 - pg_ctl start -w# Failed test 'pg_ctl start -w'
# at
/home/pavel/src/postgresql/src/bin/pg_ctl/../../../src/test/perl/TestLib.pm
line 89.
waiting for server to start....LOG: could not bind IPv6 socket: Address
already in use
HINT: Is another postmaster already running on port 5432? If not, wait a
few seconds and retry.
LOG: could not bind IPv4 socket: Address already in use
HINT: Is another postmaster already running on port 5432? If not, wait a
few seconds and retry.
WARNING: could not create listen socket for "localhost"
FATAL: could not create any TCP/IP sockets
.... stopped waiting
pg_ctl: could not start server
Examine the log output.
not ok 6 - second pg_ctl start succeeds# Failed test 'second pg_ctl start succeeds'
# at
/home/pavel/src/postgresql/src/bin/pg_ctl/../../../src/test/perl/TestLib.pm
line 89.
pg_ctl: PID file
"/home/pavel/src/postgresql/src/bin/pg_ctl/testf2KW/data/postmaster.pid"
does not exist
Is server running?
not ok 7 - pg_ctl stop -w# Failed test 'pg_ctl stop -w'
# at
/home/pavel/src/postgresql/src/bin/pg_ctl/../../../src/test/perl/TestLib.pm
line 89.
ok 8 - second pg_ctl stop fails
pg_ctl: PID file
"/home/pavel/src/postgresql/src/bin/pg_ctl/testf2KW/data/postmaster.pid"
does not exist
Is server running?
starting server anyway
pg_ctl: could not read file
"/home/pavel/src/postgresql/src/bin/pg_ctl/testf2KW/data/postmaster.opts"
not ok 9 - pg_ctl restart with server not running# Failed test 'pg_ctl restart with server not running'
# at
/home/pavel/src/postgresql/src/bin/pg_ctl/../../../src/test/perl/TestLib.pm
line 89.
pg_ctl: PID file
"/home/pavel/src/postgresql/src/bin/pg_ctl/testf2KW/data/postmaster.pid"
does not exist
Is server running?
starting server anyway
pg_ctl: could not read file
"/home/pavel/src/postgresql/src/bin/pg_ctl/testf2KW/data/postmaster.opts"
not ok 10 - pg_ctl restart with server running# Failed test 'pg_ctl restart with server running'
# at
/home/pavel/src/postgresql/src/bin/pg_ctl/../../../src/test/perl/TestLib.pm
line 89.
pg_ctl: PID file
"/home/pavel/src/postgresql/src/bin/pg_ctl/testf2KW/data/postmaster.pid"
does not exist
Is server running?
Bailout called. Further testing stopped: system pg_ctl stop -D
/home/pavel/src/postgresql/src/bin/pg_ctl/testf2KW/data -m fast failed: 256
Bail out! system pg_ctl stop -D
/home/pavel/src/postgresql/src/bin/pg_ctl/testf2KW/data -m fast failed: 256
FAILED--Further testing stopped: system pg_ctl stop -D
/home/pavel/src/postgresql/src/bin/pg_ctl/testf2KW/data -m fast failed: 256
make[1]: *** [check] Error 255
make[1]: Leaving directory `/home/pavel/src/postgresql/src/bin/pg_ctl'
make: *** [check-pg_ctl-recurse] Error 2
make: Leaving directory `/home/pavel/src/postgresql/src/bin'
On 01/31/2014 02:50 AM, Pavel Stehule wrote:
5. I didn't test it on windows
I guess that's my cue. I'll be home later today, and will take a look at
it on my Windows test setup.
--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
I posted an updated patch in the original thread. Please see the commit
fest web site for the URL.
On Thu, 2014-01-30 at 19:50 +0100, Pavel Stehule wrote:
6. I found only few issues:
a) Configure doesn't check a required IRC::Run module
Clearly, we will need to figure out something about how to require this
module, and possibly others in the future, as we expand the tests.
Having configure check for it is not necessarily the best solution --
What is configure supposed to do if it can't find it?
We could perhaps use Test::More skip_all to just skip these tests
depending on what modules are available. And add appropriate
documentation.
b) Prepared tests fails when PostgreSQL server was up - should be
checked and should to raise a valuable error message
The original patch used a hard-coded port number, which was a mistake.
I have changed this now to use a nonstandard port number that is
different from the compiled-in one, similar to how pg_regress used to do
it. It's still not bullet-proof. Do we need to do more?
c) I am not sure if infrastructure is enough - for test pg_dump I miss
a comparation of produced file with some other expected file. This
objection is not blocker - someone can enhance it (when will write
tests of pg_dump for example).
Yes, some more infrastructure will need to be added for pg_dump. But
that's a separate project. I don't see anything where the current setup
would be in the way of that.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
2014-02-09 4:16 GMT+01:00 Peter Eisentraut <peter_e@gmx.net>:
I posted an updated patch in the original thread. Please see the commit
fest web site for the URL.On Thu, 2014-01-30 at 19:50 +0100, Pavel Stehule wrote:
6. I found only few issues:
a) Configure doesn't check a required IRC::Run module
Clearly, we will need to figure out something about how to require this
module, and possibly others in the future, as we expand the tests.
Having configure check for it is not necessarily the best solution --
What is configure supposed to do if it can't find it?
there can be option "--with-client-tests" and this option should to require
IRC::Run
We could perhaps use Test::More skip_all to just skip these tests
depending on what modules are available. And add appropriate
documentation.b) Prepared tests fails when PostgreSQL server was up - should be
checked and should to raise a valuable error messageThe original patch used a hard-coded port number, which was a mistake.
I have changed this now to use a nonstandard port number that is
different from the compiled-in one, similar to how pg_regress used to do
it. It's still not bullet-proof. Do we need to do more?
you can check before starting test if some Postgres on this port is living
or not. We have pg_isready.
It can be enough
c) I am not sure if infrastructure is enough - for test pg_dump I miss
a comparation of produced file with some other expected file. This
objection is not blocker - someone can enhance it (when will write
tests of pg_dump for example).Yes, some more infrastructure will need to be added for pg_dump. But
that's a separate project. I don't see anything where the current setup
would be in the way of that.
ok
regards
Pavel
On Sat, Feb 8, 2014 at 10:16 PM, Peter Eisentraut <peter_e@gmx.net> wrote:
Clearly, we will need to figure out something about how to require this
module, and possibly others in the future, as we expand the tests.
Having configure check for it is not necessarily the best solution --
What is configure supposed to do if it can't find it?We could perhaps use Test::More skip_all to just skip these tests
depending on what modules are available. And add appropriate
documentation.
I would think we would want to keep the number of dependencies
relatively small. If it gets large, that just means that nobody will
be able to run the tests. And -1 for the idea of running only the
tests that we can given what's installed; that'll make it very easy to
not run all the tests, which kind of defeats the purpose of having
them IMHO. We should just "require" whatever we need and let the test
run abort if it's not available.
--
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
Pavel Stehule escribi�:
2014-02-09 4:16 GMT+01:00 Peter Eisentraut <peter_e@gmx.net>:
a) Configure doesn't check a required IRC::Run module
Clearly, we will need to figure out something about how to require this
module, and possibly others in the future, as we expand the tests.
Having configure check for it is not necessarily the best solution --
What is configure supposed to do if it can't find it?there can be option "--with-client-tests" and this option should to require
IRC::Run
A configure option seems a workable idea.
In the future we might want to use the Perl test framework for other
things, so perhaps --enable-perl-testing or something like that. See
for instance
/messages/by-id/64aaac31739cef275839932f16fda226@biglumber.com
--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 2/9/14, 1:01 AM, Pavel Stehule wrote:
b) Prepared tests fails when PostgreSQL server was up - should be
checked and should to raise a valuable error messageThe original patch used a hard-coded port number, which was a mistake.
I have changed this now to use a nonstandard port number that is
different from the compiled-in one, similar to how pg_regress used to do
it. It's still not bullet-proof. Do we need to do more?you can check before starting test if some Postgres on this port is
living or not. We have pg_isready.
I'm having trouble reproducing this scenario. The tests use a
Unix-domain socket in a private directory, so I don't see how that can
conflict. Can you show me exactly how you invoked the tests and which
tests and which tests failed? And what platform are you on?
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 2/17/14, 9:19 AM, Alvaro Herrera wrote:
there can be option "--with-client-tests" and this option should to require
IRC::RunA configure option seems a workable idea.
In the future we might want to use the Perl test framework for other
things, so perhaps --enable-perl-testing or something like that.
I don't think I want to add another configure option. That just reduces
flexibility during development and will make it less likely that people
can or will run the tests.
Skipping tests because of missing dependencies is a standard behavior in
TAP test suites. The alternative is that we make it a hard requirement.
I'd be for that, but maybe next release.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers