2014-02-09 4:16 GMT+01:00 Peter Eisentraut <pete...@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. > > ok regards Pavel
