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 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?
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.
