From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <alexander.turenko@tarantool.org>
Date: Wed, 3 Apr 2019 16:26:55 +0300
From: Alexander Turenko <alexander.turenko@tarantool.org>
Subject: Re: [tarantool-patches] Re: [PATCH v2 2/2] lua: add key_def lua
 module
Message-ID: <20190403132655.5icn3op4uknqrpn4@tkn_work_nb>
References: <cover.1553696707.git.kshcherbatov@tarantool.org>
 <d1c3d7d8e7934279da2b8437b4e5105e27167cfd.1553696707.git.kshcherbatov@tarantool.org>
 <20190328020146.lluz4mg5tacpghwv@tkn_work_nb>
 <35ed4661-9789-7cf1-6627-2ced2a821939@tarantool.org>
 <6d915212-e80f-4a6d-d884-b838bf25f8a7@tarantool.org>
 <20190328112158.kpxsk6b55noicbes@tkn_work_nb>
 <20190403111003.x7vq7olda55tthgi@esperanza>
 <20190403114644.25bf3mull76xk7xp@tkn_work_nb>
 <20190403120121.gborrlvg36ghfuhx@esperanza>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <20190403120121.gborrlvg36ghfuhx@esperanza>
To: Vladimir Davydov <vdavydov.dev@gmail.com>
Cc: Kirill Shcherbatov <kshcherbatov@tarantool.org>, tarantool-patches@freelists.org
List-ID: <tarantool-patches.dev.tarantool.org>

On Wed, Apr 03, 2019 at 03:01:21PM +0300, Vladimir Davydov wrote:
> On Wed, Apr 03, 2019 at 02:46:44PM +0300, Alexander Turenko wrote:
> > > > +-- Case: extract_key().
> > > > +test:test('extract_key()', function(test)
> > > > +    test:plan(9)
> > > > +
> > > > +    test:is_deeply(key_def_a:extract_key(tuple_a):totable(), {1}, 'case 1')
> > > > +    test:is_deeply(key_def_b:extract_key(tuple_a):totable(), {1, 22}, 'case 2')
> > > > +
> > > > +    -- JSON path.
> > > > +    local res = key_def_lib.new({
> > > > +        {type = 'string', fieldno = 1, path = 'a.b'},
> > > > +    }):extract_key(box.tuple.new({{a = {b = 'foo'}}})):totable()
> > > > +    test:is_deeply(res, {'foo'}, 'JSON path (tuple argument)')
> > > > +
> > > > +    local res = key_def_lib.new({
> > > > +        {type = 'string', fieldno = 1, path = 'a.b'},
> > > > +    }):extract_key({{a = {b = 'foo'}}}):totable()
> > > > +    test:is_deeply(res, {'foo'}, 'JSON path (table argument)')
> > > 
> > > I like key_def_new_cases - they are very easy to read or extend.
> > > I don't quite like the tests below, because they refer to objects
> > > created a few screens above (tuple_a, key_def_a, etc). Could you
> > > please rewrite them in a similar to key_def_new_cases fashion,
> > > without referring to any predefined variables?
> > 
> > It is easy to separate test cases from a testing code in case of one
> > function like key_def.new(), but it is not so easy when we need to test
> > several functions with different behaviour. So I vote up for inlining
> > related data (tuple_a and so on) to test cases, but doubt these cases
> > could be written in such declarative manner as I did for key_def.new().
> 
> I didn't mean to mix all function test cases in one table. I meant
> using a separate table for each function. Something like this:
> 
> tuple_compare_test_cases = {
> 	{
> 		'Tuple compare with collation',
> 		parts = {{
> 			fieldno = 1,
> 			type = 'string',
> 			collation = 'unicode_ci',
> 		}},
> 		tuple1 = {'test1', 1, 2},
> 		tuple2 = {'test2', 3},
> 		exp_err = nil,
> 		exp_ret = 1,
> 	}
> 	...
> }
> 
> tuple_extract_key_cases = {
> 	...
> }
> 
> Do you think it would be an overkill?

It seems this way will look more structured, you are right. I agree, but
don't insist, because the cases are simple (a function call +
is/is_deeply).