On 07/30/2015 04:48 AM, Michael Paquier wrote:
> On Thu, Jul 30, 2015 at 5:54 AM, Andreas Karlsson <andreas@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