On 07/30/2015 04:48 AM, Michael Paquier wrote:
On Thu, Jul 30, 2015 at 5:54 AM, Andreas Karlsson <andr...@proxel.se> wrote:
What I do not like though is how the path src/test/tables_fk/t/ tells us
nothing about what features are of PostgreSQL are tested here. For this I
personally prefer the earlier versions where I think that was clear.

+Simple module used to check data dump ordering of pg_dump with tables
+linked with foreign keys.
The README mentions that this is to test pg_dump, perhaps referring to
the TAP tests makes sense as well?

The comment is good, but what I personally do not like is that the path to the test suite is non-obvious and not self explanatory I would not expect src/test/tables_fk/t/ to test pg_dump and extensions. I find it to confusing to regard the test suite as testing an extension called "tables_fk" since in my mind that is just a necessary tool built to test something else.

This is obviously a subjective thing, and I see that for example Heikki likes it the way it is. I am fine with setting this as ready for committer and and let a committer weigh in on the naming.

Another thought: would it be worthwhile to also add an assertion to check if
the data really was restored properly or would that just be redundant code?

That seems worth doing, hence added something for it. Thanks for the suggestion.

At the same time I have added an entry in .gitignore for the generated
tmp_check/.

Nice changes.

--
Andreas



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

Reply via email to