Re: Introduce pg_receivewal gzip compression tests
Michael Paquier <michael@paquier.xyz>
From: Michael Paquier <michael@paquier.xyz>
To: gkokolatos@pm.me
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2021-07-13T01:53:12Z
Lists: pgsql-hackers
Attachments
- v5-0001-Add-some-tests-for-pg_receivewal-compress.patch (text/x-diff) patch v5-0001
On Mon, Jul 12, 2021 at 04:46:29PM +0000, gkokolatos@pm.me wrote:
> On Monday, July 12th, 2021 at 17:07, <gkokolatos@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
Commits
-
Re-enable TAP tests of pg_receivewal for ZLIB on Windows
- 91d395f47aa9 15.0 landed
-
Fix some issues with WAL segment opening for pg_receivewal --compress
- 11dbad74c104 10.18 landed
- 795a9166e2e1 11.13 landed
- b9a0de15eb29 12.8 landed
- fb2b86015a9f 13.4 landed
- 3a0d2d0cbaf3 14.0 landed
- 7fbe0c8c4d4f 15.0 landed
-
Disable tests involving ZLIB on Windows for pg_receivewal
- 6cea447e6a10 15.0 landed
-
Fix portability issue with gzip in TAP test of pg_receivewal
- 0da3c1bc3f72 15.0 landed
-
Add TAP tests for ZLIB compression for pg_receivewal
- ffc9ddaea33f 15.0 landed