[tarantool-patches] Re: [PATCH v2 2/2] lua: add key_def lua module
Vladimir Davydov
vdavydov.dev at gmail.com
Wed Apr 3 15:01:21 MSK 2019
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?
More information about the Tarantool-patches
mailing list