Re: Re: In-core regression tests for replication, cascading, archiving, PITR, etc.

Hello, I'm studying this.

Two hunks in 0003 needed a fix but the other part applied cleanly
on master.

At Fri, 22 Jan 2016 15:17:51 +0900, Michael Paquier <michael.paquier@gmail.com> wrote in <CAB7nPqTTAtVCEXAoyMtF4Xu9g=mXY4cjnP=+hy7jgYfnFzM=JA@mail.gmail.com>
> On Mon, Dec 21, 2015 at 4:45 PM, Michael Paquier
> <michael.paquier@gmail.com> wrote:
> > As this thread is stalling a bit, please find attached a series of
> > patch gathering all the pending issues for this thread:
> > - 0001, fix config_default.pl for MSVC builds to take into account TAP tests
> > - 0002, append a node name in get_new_node (per Noah's request)
> > - 0003, the actual recovery test suite
> > Hopefully this facilitates future reviews.
> 
> Patch 2 has been pushed as c8642d9 (thanks Alvaro). The remaining two
> patches still apply and pass cleanly.

The TAP test framework doesn't remove existing temporary
directories when a test script suite (or a prove?) starts, and it
in turn removes all temprorary directories it has made even if
ended with fairures. It would be sometimes inconvenient to find
the cause of the failures and inconsistent with the behavior of
the ordinary(?)  make check, as far as my understanding goes.

tmp_check is left remained but it would be ok to preserve logs,
which is located in tmp_check differently than the ordinary
regressions.

One annoyance is the name of data directories is totally
meaningless. We cannot investigate them even if it is left
behind.

Addition to them, maybe it is useful that a test script can get
stderr content from PostgresNode->psql(). Setting
client_min_messages lower can give a plenty of useful information
about how server is working.


So, I'd like to propose four (or five) changes to this harness.

 - prove_check to remove all in tmp_check

 - TestLib to preserve temporary directories/files if the current
   test fails.

 - PostgresNode::get_new_node to create data directory with
   meaningful basenames.

 - PostgresNode::psql to return a list of ($stdout, $stderr) if
   requested. (The previous behavior is not changed)

 - (recovery/t/00x_* gives test number to node name)

As a POC, the attached diff will appliy on the 0001 and (fixed)
0003 patches.

It might be good to give test number to the name of temp dirs by
any automated way, but it is not included in it.


Opinions? Thoughts?

regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center