From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <vdavydov.dev@gmail.com>
Date: Wed, 25 Jul 2018 19:40:57 +0300
From: Vladimir Davydov <vdavydov.dev@gmail.com>
Subject: Re: [tarantool-patches] [PATCH v3 3/4] box-tap: syslog destination
 test unix socket
Message-ID: <20180725164057.tfni54gm6wuqetq2@esperanza>
References: <20180724171152.24921-1-arkholga@tarantool.org>
 <20180724171152.24921-4-arkholga@tarantool.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20180724171152.24921-4-arkholga@tarantool.org>
To: Olga Arkhangelskaia <arkholga@tarantool.org>
Cc: tarantool-patches@freelists.org
List-ID: <tarantool-patches.dev.tarantool.org>

We fold tests in the patch that implements a feature or fixes a bug
whenever possible. Please fold patches 3 and 4 in patch 1.

On Tue, Jul 24, 2018 at 08:11:51PM +0300, Olga Arkhangelskaia wrote:
> @@ -472,5 +472,40 @@ code = [[pcall(box.cfg, {log = 'syslog:identity=tarantool'})
>  ]]
>  test:is(run_script(code), 0, "syslog log configuration")
>  
> +--
> +-- Check syslog socket configuration
> +--
> +code = [[

> +local tap = require('tap')

Not needed.

> +local socket = require('socket')
> +local os = require('os')

> +local test = tap.test("syslog_unix")

Not needed.

> +local log = require('log')
> +local fio = require('fio')
> +
> +path = fio.pathjoin(fio.cwd(), 'log_unix_socket_test.sock')
> +unix_socket = socket('AF_UNIX', 'SOCK_DGRAM',0)

Nit: a space missing between , and 0.

> +unix_socket:bind('unix/', path)

> +socket.tcp_connect('unix', path)

Why?

> +
> +opt = string.format("syslog:server=unix:%s,identity=tarantool", path)
> +res = 1
> +if pcall (box.cfg, {log = opt, log_level = 5}) then

You don't need pcall here - box.cfg() must succeed here.

Also, log_level = 5 by default so you don't need to specify it
explicitly.

> +    log.info("Test socket syslog destination")
> +    buf = unix_socket:read("Test socket syslog destination", 30)
> +    if buf ~= nil then
> +        if buf:match('Test socket syslog destination') then res = 0
> +        else res = 1 end
> +    else
> +        res = 1
> +    end

This looks over-complicated. Can we rewrite it like shown below?

buf = ...
if buf and buf:match(...) then
    res = 0
else
    res = 1
end

> +end
> +
> +unix_socket:close()                                                         
> +os.remove(path) 
> +os.exit(res)
> +]]
> +test:is(run_script(code), 0, "unix socket syslog log configuration")
> +
>  test:check()
>  os.exit(0)