On Mon, Jul 12, 2021 at 04:46:29PM +0000, gkokola...@pm.me wrote: > On Monday, July 12th, 2021 at 17:07, <gkokola...@pm.me> wrote: >> ‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
Are you using outlook? The format of your messages gets blurry on the
PG website, so does it for me.
>> I am admittedly not so well versed on Windows systems. Thank you for
>> informing me.
>> Please find attached v3 of the patch where $ENV{GZIP_PROGRAM} is used
>> instead. To the best of my knowledge one should avoid using $ENV{GZIP}
>> because that would translate to the obsolete, yet used environment
>> variable GZIP which holds a set of default options for gzip. In essence
>> it would be equivalent to executing:
>> GZIP=gzip gzip --test <files>
>> which can result to errors similar to:
>> gzip: gzip: non-option in GZIP environment variable
-# make this available to TAP test scripts
+# make these available to TAP test scripts
export TAR
+export GZIP_PROGRAM=$(GZIP)
Wow. So this comes from the fact that the command gzip can feed on
the environment variable from the same name. I was not aware of
that, and a comment would be in place here. That means complicating a
bit the test flow for people on Windows, but I am fine to live with
that as long as this does not fail hard. One extra thing we could do
is drop this part of the test, but I agree that this is useful to have
around as a validity check.
> After a bit more thinking, I went ahead and added on top of v3 a test
> verifying that the gzip program can actually be called.
+ system_log($gzip, '--version') != 0);
Checking after that does not hurt, indeed. I am wondering if we
should do that for TAR.
Another thing I find unnecessary is the number of the tests. This
does two rounds of pg_receivewal just to test the long and short
options of -Z/-compress, which brings only coverage to make sure that
both option names are handled. That's a high cost for a low amound of
extra coverage, so let's cut the runtime in half and just use the
round with --compress.
There was also a bit of confusion with ZLIB and gzip in the variable
names and the comments, the latter being only the command while the
compression happens with zlib. With a round of indentation on top of
all that, I ge tthe attached.
What do you think?
--
Michael
From 44d971f8d4187fa34dc7426b629eafeeb0af53c2 Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@paquier.xyz>
Date: Tue, 13 Jul 2021 10:51:20 +0900
Subject: [PATCH v5] Add some tests for pg_receivewal --compress
---
src/bin/pg_basebackup/Makefile | 6 ++-
src/bin/pg_basebackup/t/020_pg_receivewal.pl | 46 +++++++++++++++++++-
2 files changed, 50 insertions(+), 2 deletions(-)
diff --git a/src/bin/pg_basebackup/Makefile b/src/bin/pg_basebackup/Makefile
index 66e0070f1a..459d514183 100644
--- a/src/bin/pg_basebackup/Makefile
+++ b/src/bin/pg_basebackup/Makefile
@@ -18,8 +18,12 @@ subdir = src/bin/pg_basebackup
top_builddir = ../../..
include $(top_builddir)/src/Makefile.global
-# make this available to TAP test scripts
+# make these available to TAP test scripts
export TAR
+# Note that GZIP cannot be used directly as this environment variable is
+# used by the command "gzip" to pass down options, so stick with a different
+# name.
+export GZIP_PROGRAM=$(GZIP)
override CPPFLAGS := -I$(libpq_srcdir) $(CPPFLAGS)
LDFLAGS_INTERNAL += -L$(top_builddir)/src/fe_utils -lpgfeutils $(libpq_pgport)
diff --git a/src/bin/pg_basebackup/t/020_pg_receivewal.pl b/src/bin/pg_basebackup/t/020_pg_receivewal.pl
index a547c97ef1..9fa1a3378b 100644
--- a/src/bin/pg_basebackup/t/020_pg_receivewal.pl
+++ b/src/bin/pg_basebackup/t/020_pg_receivewal.pl
@@ -5,7 +5,7 @@ use strict;
use warnings;
use TestLib;
use PostgresNode;
-use Test::More tests => 19;
+use Test::More tests => 23;
program_help_ok('pg_receivewal');
program_version_ok('pg_receivewal');
@@ -66,6 +66,50 @@ $primary->command_ok(
],
'streaming some WAL with --synchronous');
+# Check ZLIB compression if available.
+SKIP:
+{
+ skip "postgres was not build with ZLIB support", 4
+ if (!check_pg_config("#define HAVE_LIBZ 1"));
+
+ # Generate more WAL.
+ $primary->psql('postgres', 'SELECT pg_switch_wal();');
+ $nextlsn =
+ $primary->safe_psql('postgres', 'SELECT pg_current_wal_insert_lsn();');
+ chomp($nextlsn);
+ $primary->psql('postgres',
+ 'INSERT INTO test_table VALUES (generate_series(100,200));');
+ $primary->psql('postgres', 'SELECT pg_switch_wal();');
+
+ $primary->command_ok(
+ [
+ 'pg_receivewal', '-D', $stream_dir, '--verbose',
+ '--endpos', $nextlsn, '--compress', '1'
+ ],
+ "streaming some WAL using ZLIB compression");
+
+ # Verify that the stored files are generated with the expected names.
+ my @zlib_wals = glob "$stream_dir/*.gz";
+ is(scalar(@zlib_wals), 1,
+ "one WAL segment compressed with ZLIB was created");
+ my @zlib_partial_wals = glob "$stream_dir/*.gz.partial";
+ is(scalar(@zlib_partial_wals),
+ 1, "one partial WAL segment compressed with ZLIB was created");
+
+ # There are two files compressed with ZLIB, check the integrity of
+ # all of them.
+ my $gzip = $ENV{GZIP_PROGRAM};
+ skip "program gzip is not found in your system", 1
+ if ( !defined $gzip
+ || $gzip eq ''
+ || system_log($gzip, '--version') != 0);
+
+ push(@zlib_wals, @zlib_partial_wals);
+ my $gzip_is_valid = system_log($gzip, '--test', @zlib_wals);
+ is($gzip_is_valid, 0,
+ "gzip verified the integrity of compressed WAL segments");
+}
+
# Permissions on WAL files should be default
SKIP:
{
--
2.32.0
signature.asc
Description: PGP signature
