Re: Improving log capture of TAP tests with IPC::Run

From: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
To: Heikki Linnakangas <hlinnaka(at)iki(dot)fi>
Cc: PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Improving log capture of TAP tests with IPC::Run
Date: 2015-07-09 01:50:39
Message-ID: CAB7nPqQ+fMJ_4WzKZW8pc+EvnxooMKJJsrQ3Gxa8YKywKZ9Dww@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Jul 9, 2015 at 12:49 AM, Heikki Linnakangas wrote:
>>> Looking at the manual page of Test::More, it looks like you could change
>>> where the perl script's STDOUT and STDERR point to, because Test::More
>>> takes
>>> a copy of them (when? at program startup I guess..). That would be much
>>> more
>>> convenient than decorating every run call with ">> logfile".
>>
>>
>> Hm. There are two types of logs we want to capture:
>> 1) stdout and stderr from the subprocesses kicked by IPC::Run::run
>> 2) Status messages written in the log file by the process running the
>> tests.
>> Perhaps we could redirect the output of stdout and stderr but I think
>> that this is going to need an fd open from the beginning of the test
>> until the end, with something like that:
>> open my $test_logfile_fd, '>>', $test_logfile;
>> *STDOUT = $test_logfile_fd;
>> *STDERR = $test_logfile_fd;
>>
>> While that would work on OSX and Linux for sure, I suspect that this
>> will not on Windows where two concurrent processes cannot write to the
>> same file.
>
>
> Hmm, as long as you make sure all the processes use the same filehandle,
> rather than open the log file separately, I think it should work. But it's
> Windows, so who knows..

And actually your patch works even on Windows. Tests are running and
log can be captured in the same shape as Linux and OSX!

> I came up with the attached, which does that. It also plays some tricks with
> perl "tie", to copy the "ok - ..." lines that go to the console, to the log.
>
> I tried to test that on my Windows system, but I don't have IPC::Run
> installed. How did you get that on Windows? Can you test this?

I simply downloaded them from CPAN and put them in PERL5LIB. And it
worked. For Windows, you will also need some adjustments to make the
tests able to run (see the other thread related to support TAP in MSVC
http://www.postgresql.org/message-id/CAB7nPqTQwphkDfZP07w7yBnbFNDhW_JBAMyCFAkarE2VWg8irQ@mail.gmail.com)
like using SSPI for connection, adjusting listen_addresses. The patch
attached, which is a merge of your patch and my MSVC stuff, is able to
do that. This is just intended for testing, so feel free to use it if
you want to check by yourself how logs are captured. I'll repost a new
version of it on the other thread depending on the outcome here.

- system_or_bail(
-"pg_basebackup -D $test_standby_datadir -p $port_master -x >>$log_path 2>&1");
+ system_or_bail('pg_basebackup', '-D', $test_standby_datadir,
+ '-p', $port_master, '-x';
A parenthesis is missing here.

- my $result = run(
- [ 'pg_rewind',
- "--source-server",
- "port=$port_standby dbname=postgres",
- "--target-pgdata=$test_master_datadir" ],
- '>>',
- $log_path,
- '2>&1');
- ok($result, 'pg_rewind remote');
+ command_ok(['pg_rewind',
+ "--source-server",
+ "port=$port_standby dbname=postgres",
+ "--target-pgdata=$test_master_datadir"],
+ 'pg_rewind remote');
As long as we are on it, I think that we should add --debug here to
make the logs more useful.

Except that this patch looks good to me. Thanks for the black magic on
stdout/stderr handling.
Regards,
--
Michael

Attachment Content-Type Size
20150709_tap_msvc_logs.patch text/x-diff 28.5 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message David Rowley 2015-07-09 02:41:27 Re: Memory Accounting v11
Previous Message Jim Nasby 2015-07-09 00:09:56 Re: Freeze avoidance of very large table.