From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp45.i.mail.ru (smtp45.i.mail.ru [94.100.177.105]) (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 E3F67469719 for ; Tue, 10 Mar 2020 14:29:45 +0300 (MSK) References: <20200218180101.37385-1-arkholga@tarantool.org> <20200304135133.hnj3wdsnizsbo76l@tkn_work_nb> From: Olga Arkhangelskaia Message-ID: Date: Tue, 10 Mar 2020 14:29:44 +0300 MIME-Version: 1.0 In-Reply-To: <20200304135133.hnj3wdsnizsbo76l@tkn_work_nb> Content-Type: text/plain; charset="utf-8"; format="flowed" Content-Transfer-Encoding: 8bit Content-Language: en-GB Subject: Re: [Tarantool-patches] [PATCH] test_run: add path option to grep_log List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexander Turenko Cc: tarantool-patches@dev.tarantool.org Hi! Thanks for the review. I have looked thought the patch and fixed it. You can look at diffs below. 04.03.2020 16:51, Alexander Turenko пишет: > The patch looks okay, but I would ask you to do few small modifications > (see below). > > WBR, Alexander Turenko. > > On Tue, Feb 18, 2020 at 09:01:01PM +0300, Olga Arkhangelskaia wrote: >> While using grep_log it is useful to have a possibility to go through crashed > Please, keep a commit message body within 72 symbols. [PATCH] test_run: add filename option to grep_log To search custom logs filename option of grep_log can be used. Usage: grep_log(self, node, waht, bytes, {filename = XXX}) Closes #196 @Changelog Search custom log grep_log(self, node, waht, bytes, {filename = XXX}) > See https://www.tarantool.io/en/doc/1.10/dev_guide/developer_guidelines/#how-to-write-a-commit-message > >> instance log. >> Usage in case of crashed instance: >> test_run:grep_log(nil, str-to-find, nil, {path=path_to_log}) >> >> Closes #196 >> --- >> Issue:https://github.com/tarantool/test-run/issues/196 >> Branch:https://github.com/tarantool/test-run/pull/new/OKriw/gh-196-Extend-grep_log-for-crashed-instances >> >> test_run.lua | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/test_run.lua b/test_run.lua >> index 4d8132d..68dba53 100644 >> --- a/test_run.lua >> +++ b/test_run.lua >> @@ -326,7 +326,9 @@ end >> local function grep_log(self, node, what, bytes, opts) >> local opts = opts or {} >> local noreset = opts.noreset or false >> - local filename = self:eval(node, "box.cfg.log")[1] >> + -- node is used in case of alive instance, otherwise path to log can be >> + -- used for path >> + local filename = opts.path or self:eval(node, "box.cfg.log")[1] > Nit: It is rule of thumb to fit comments within 66 symbols. > > We already use 'filename' name for this, I would use it for the option > too. > > I suggest the following pattern for options: > > | local function f(<...>, opts) > | local opts = opts or {} > | local foo = opts.foo (or if it is not nil) > | local bar = opts.bar > | <...the rest code...> > | end > > This way it is easy to find them all in code at one glance. diff --git a/test_run.lua b/test_run.lua index 4d8132d..9a25246 100644 --- a/test_run.lua +++ b/test_run.lua @@ -326,7 +326,8 @@ end  local function grep_log(self, node, what, bytes, opts)      local opts = opts or {}      local noreset = opts.noreset or false -    local filename = self:eval(node, "box.cfg.log")[1] +    -- if instance has crashed provide filename to use grep_log +    local filename = opts.filename or self:eval(node, "box.cfg.log")[1]      local file = fio.open(filename, {'O_RDONLY', 'O_NONBLOCK'})      local function fail(msg) >> local file = fio.open(filename, {'O_RDONLY', 'O_NONBLOCK'}) >> >> local function fail(msg) >> -- >> 2.20.1 (Apple Git-117) >>