Thread: Re: [PATCH] Better cleanup in TLS tests for -13beta2

Re: [PATCH] Better cleanup in TLS tests for -13beta2

From
Michael Paquier
Date:
On Mon, Jun 29, 2020 at 03:51:48PM -0400, Tom Lane wrote:
> Daniel Gustafsson <daniel@yesql.se> writes:
>> That being said, we do retain temporary files on such failures on purpose in
>> our TestLib since 88802e068017bee8cea7a5502a712794e761c7b5 and a few follow-up
>> commits since, should these be handled differently?  They are admittedly less
>> "unknown" as compared to other files as they are copies, but famous last words
>> have been spoken about bugs that can never happen.
>
> Oh, good point.  Objection withdrawn.

I looked at the patch, and can confirm that client_wrongperms_tmp.key
remains around after running 001_ssltests.pl, and client_tmp.key after
running 002_scram.pl.  The way the patch does its cleanup looks fine
to me, so I'll apply and backpatch where necessary, if there are no
objections of course.
--
Michael

Attachment

Re: [PATCH] Better cleanup in TLS tests for -13beta2

From
michael@paquier.xyz
Date:
On Tue, Jun 30, 2020 at 01:13:39PM +0900, Michael Paquier wrote:
> I looked at the patch, and can confirm that client_wrongperms_tmp.key
> remains around after running 001_ssltests.pl, and client_tmp.key after
> running 002_scram.pl.  The way the patch does its cleanup looks fine
> to me, so I'll apply and backpatch where necessary, if there are no
> objections of course.

I found one problem when testing with parallel jobs once we apply this
patch (say PROVE_FLAGS="-j 4"): the tests of 001 and 002 had the idea
to use the same file name client_tmp.key, so it was possible to easily
fail the tests if for example 002 removes the temporary client key
copy that 001 needs, or vice-versa.  001 takes longer than 002, so the
removal would likely be done by the latter, not the former.  And it
was even logically possible to fail in the case where 001 removes the
file and 002 needs it, though very unlikely because 002 needs this
file for a very short amount of time and one test case.  I have fixed
this issue by just making 002 use a different file name, as we do in
001 for the case of the wrong permissions, and applied the patch down
to 13.
--
Michael

Attachment