Welcome! Log In Create A New Profile

Advanced

Re: [PATCH] Tests: fixed try_run() to remove temporary directory on failure

Maxim Dounin
September 06, 2022 09:48PM
Hello!

On Tue, Sep 06, 2022 at 05:52:51PM +0400, Sergey Kandaurov wrote:

> # HG changeset patch
> # User Sergey Kandaurov <pluknet@nginx.com>
> # Date 1662472340 -14400
> # Tue Sep 06 17:52:20 2022 +0400
> # Node ID 7eb87eaf06f4b9161cfe298f195947dbddedd01d
> # Parent 78fe648d54a79822a72f3a5f8cc79651b3b1f5a7
> Tests: fixed try_run() to remove temporary directory on failure.
>
> Keeping the file handle pointing to the "stderr" file prevented removal
> of the temporary directory on win32 if execution aborted in run().
> The fix is to move safe file operations surrounding run() out of the
> eval block such that the state of file handles is consistent.
> Fixes a1874249496d.

I believe it might need some additional emphasis that this is
about Windows only. For example:

Tests: fixed try_run() to remove temporary directory on win32.

>
> diff -r 78fe648d54a7 -r 7eb87eaf06f4 lib/Test/Nginx.pm
> --- a/lib/Test/Nginx.pm Tue Aug 30 17:24:16 2022 +0400
> +++ b/lib/Test/Nginx.pm Tue Sep 06 17:52:20 2022 +0400
> @@ -290,8 +290,9 @@ sub has_daemon($) {
> sub try_run($$) {
> my ($self, $message) = @_;
>
> + open OLDERR, ">&", \*STDERR;
> +
> eval {
> - open OLDERR, ">&", \*STDERR;
> open NEWERR, ">", $self->{_testdir} . '/stderr'
> or die "Can't open stderr: $!";
> close STDERR;
> @@ -299,10 +300,10 @@ sub try_run($$) {
> close NEWERR;
>
> $self->run();
> + };
>
> - close STDERR;
> - open STDERR, ">&", \*OLDERR;
> - };
> + close STDERR;
> + open STDERR, ">&", \*OLDERR;
>
> return $self unless $@;

Wouldn't it be better to move all stderr handling out of the eval?

diff --git a/lib/Test/Nginx.pm b/lib/Test/Nginx.pm
--- a/lib/Test/Nginx.pm
+++ b/lib/Test/Nginx.pm
@@ -291,14 +291,13 @@ sub try_run($$) {
my ($self, $message) = @_;

open OLDERR, ">&", \*STDERR;
+ open NEWERR, ">", $self->{_testdir} . '/stderr'
+ or die "Can't open stderr: $!";
+ close STDERR;
+ open STDERR, ">&", \*NEWERR;
+ close NEWERR;

eval {
- open NEWERR, ">", $self->{_testdir} . '/stderr'
- or die "Can't open stderr: $!";
- close STDERR;
- open STDERR, ">&", \*NEWERR;
- close NEWERR;
-
$self->run();
};


Alternatively, it might be the right time to backout a1874249496d
completely and rely on "-e ..." instead, given that it was
introduced in 1.19.5 and already present in all supported
versions.

--
Maxim Dounin
http://mdounin.ru/
_______________________________________________
nginx-devel mailing list -- nginx-devel@nginx.org
To unsubscribe send an email to nginx-devel-leave@nginx.org
Subject Author Views Posted

[PATCH] Tests: fixed try_run() to remove temporary directory on failure

Sergey Kandaurov 300 September 06, 2022 09:54AM

Re: [PATCH] Tests: fixed try_run() to remove temporary directory on failure

Maxim Dounin 53 September 06, 2022 09:48PM

Re: [PATCH] Tests: fixed try_run() to remove temporary directory on failure

Sergey Kandaurov 47 September 16, 2022 01:04PM

Re: [PATCH] Tests: fixed try_run() to remove temporary directory on failure

Maxim Dounin 60 September 17, 2022 01:52PM



Sorry, you do not have permission to post/reply in this forum.

Online Users

Guests: 105
Record Number of Users: 6 on February 13, 2018
Record Number of Guests: 421 on December 02, 2018
Powered by nginx      Powered by FreeBSD      PHP Powered      Powered by MariaDB      ipv6 ready