From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <vdavydov.dev@gmail.com>
Date: Wed, 12 Dec 2018 16:58:06 +0300
From: Vladimir Davydov <vdavydov.dev@gmail.com>
Subject: Re: [PATCH v3] fio: cleanup error messages
Message-ID: <20181212135806.vmiyuncwxrbeoxdj@esperanza>
References: <20181211224614.6609-1-roman.habibov@tarantool.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20181211224614.6609-1-roman.habibov@tarantool.org>
To: Roman Khabibov <roman.habibov@tarantool.org>
Cc: tarantool-patches@freelists.org
List-ID: <tarantool-patches.dev.tarantool.org>

On Wed, Dec 12, 2018 at 01:46:14AM +0300, Roman Khabibov wrote:
> Otherwise it is hard to debug code, throwing
> exceptions. fio.pathjoin was just one example among
> many.
> 
> Closes #3580
> 
> Branch: https://github.com/tarantool/tarantool/tree/romanhabibov/gh-3580-err-msg-pathjoin
> Issue: https://github.com/tarantool/tarantool/issues/3580

Branch/Issue prefixes are not necessary - it's clear which is which
without them. Also, the links should go after the '---' below (this way
they are ignored by git-am). And you forgot a changelog - whenever you
resubmit a patch after making some changes you should write a brief
change log, e.g.

https://www.freelists.org/post/tarantool-patches/PATCH-v3-replication-fix-exit-with-ER-NO-SUCH-USER-during-bootstrap

> ---
>  src/lua/fio.lua       | 18 ++++++-------
>  test/app/fio.result   | 61 +++++++++++++++++++++++++++++++++++++++++++
>  test/app/fio.test.lua | 19 +++++++++++++-
>  3 files changed, 88 insertions(+), 10 deletions(-)
> 
> diff --git a/src/lua/fio.lua b/src/lua/fio.lua
> index 55faebdcb..f92267023 100644
> --- a/src/lua/fio.lua
> +++ b/src/lua/fio.lua
> @@ -126,7 +126,7 @@ fio_methods.seek = function(self, offset, whence)
>      end
>      if type(whence) == 'string' then
>          if fio.c.seek[whence] == nil then
> -            error(sprintf("Unknown whence: %s", whence))
> +            error(sprintf("Usage: fio.seek() unknown whence: %s", whence))

I meant that "Usage: ..." pattern should only be used for errors that
don't have any additional details. Sorry for misunderstanding. I amended
it by myself.

> +-- gh-3580: Check that error messages are descriptive enough.
> +fh1:seek(nil, 'a')
> +---
> +- error: 'builtin/fio.lua:129: Usage: fio.seek() unknown whence: a'
> +...

Forgot to tell you last time. These error messages would change if we
made any changes to fio.lua, which would be annoying. I filtered the
line numbers out (see test_run:cmd("push filter ...")).

>  fio.unlink(tmp1)
>  fio.unlink(tmp2)
> -fio.rmdir(tmpdir)
> +fio.rmdir(tmpdir)
> \ No newline at end of file

Newline was removed by your patch. Please fix your editor. I amended it
as well and pushed the patch to 2.1.