From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <v.shpilevoy@tarantool.org>
Received: from smtp37.i.mail.ru (smtp37.i.mail.ru [94.100.177.97])
 (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
 (No client certificate requested)
 by dev.tarantool.org (Postfix) with ESMTPS id E4C4246971A
 for <tarantool-patches@dev.tarantool.org>;
 Sun,  8 Dec 2019 18:55:40 +0300 (MSK)
References: <20191206154417.5213-1-i.kosarev@tarantool.org>
From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Message-ID: <f61f5aa6-3b0b-9a5b-1700-0e94b6b20c74@tarantool.org>
Date: Sun, 8 Dec 2019 16:55:38 +0100
MIME-Version: 1.0
In-Reply-To: <20191206154417.5213-1-i.kosarev@tarantool.org>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Subject: Re: [Tarantool-patches] [PATCH v2] test: fix flaky socket test
List-Id: Tarantool development patches <tarantool-patches.dev.tarantool.org>
List-Unsubscribe: <https://lists.tarantool.org/mailman/options/tarantool-patches>, 
 <mailto:tarantool-patches-request@dev.tarantool.org?subject=unsubscribe>
List-Archive: <https://lists.tarantool.org/pipermail/tarantool-patches/>
List-Post: <mailto:tarantool-patches@dev.tarantool.org>
List-Help: <mailto:tarantool-patches-request@dev.tarantool.org?subject=help>
List-Subscribe: <https://lists.tarantool.org/mailman/listinfo/tarantool-patches>, 
 <mailto:tarantool-patches-request@dev.tarantool.org?subject=subscribe>
To: Ilya Kosarev <i.kosarev@tarantool.org>, tarantool-patches@dev.tarantool.org

Thanks for the fixes!

See 2 comments below.

> diff --git a/test/app/socket.result b/test/app/socket.result
> index fd299424c96..0095b89b867 100644
> --- a/test/app/socket.result
> +++ b/test/app/socket.result
> @@ -107,7 +110,7 @@ s:nonblock(true)
>  ---
>  - true
>  ...
> -s:readable(.1)
> +s:readable(WAIT_TCP_CONNECT_TIME)

1. How did you choose exactly these places to patch?
I see some other places, which also wait for an event,
but they still use a small timeout (in comparison with
240 secs). For example:

    socket.result line 233, 904, 919:
        s:readable(1)
        ---
        - true
        ...

    socket.result line 836, 848:
        s:readable(10)
        ---
        - true
        ...

    socket.result line 2870:
        s:settimeout(1)
        receive2 = s:receive('*a')

      Here :receive() has timeout 1 sec,
      not infinity.

> @@ -754,7 +757,7 @@ string.match(tostring(sc), ', peer') == nil
>  ---
>  - true
>  ...
> -sc:writable()
> +sc:writable(WAIT_TCP_CONNECT_TIME)

2. Here the timeout was infinity, but you
changed it to your constant. Is it to make
all the places consistent? That is strange,
because there are still other places calling
writable() without arguments or a settimeout()
invoked beforehand.