Re: [HACKERS] additional contrib test suites

On 9/8/17 1:32 PM, Peter Eisentraut wrote:
> 
> Yes, some of the error messages had changed.  Fixed patches attached.

Patches apply and all tests pass.  A few comments:

* [PATCH v2 1/7] adminpack: Add test suite

There are no regular tests for pg_logdir_ls().  It looks like TAP tests
would be required but I'm not sure it's worth it.  The fact that the
"default" log name format is hard-coded in is, um, interesting.

Maybe add:

+ SELECT pg_logdir_ls();
+ ERROR:  could not read directory "log": No such file or directory

to get a little more coverage?  It would be good to at least have a note
on why it is not tested.

* [PATCH v2 4/7] chkpass: Add test suite

Well, this is kind of scary:

+CREATE EXTENSION chkpass;
+WARNING:  type input function chkpass_in should not be volatile

I guess the only side effect is that this column cannot be indexed?  The
docs say that, so OK, but is there anything else a user should be
worried about?

The rest looks good.  I'll mark this "Ready for Committer" since I'm the
only reviewer.  I don't think anything you might add based on my
comments above requires a re-review.

As for testing on more platforms, send it to the build farm?

-- 
-David
david@pgmasters.net


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