On 07/09/2015 04:50 AM, Michael Paquier wrote:
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!

Great!

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_jbamycfakare2vwg8...@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.

Thanks, fixed the parenthesis and committed. The missing --debug is a separate issue.

- Heikki


--
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