[tarantool-patches] Re: [PATCH v1 1/1] tap: fix is_deeply box.NULL corner cases
Vladislav Shpilevoy
v.shpilevoy at tarantool.org
Tue Apr 23 17:14:26 MSK 2019
LGTM.
On 23/04/2019 17:12, Kirill Shcherbatov wrote:
> explicit is better than implicit, yep
>
> - if visited_keys[i] ~= true and v ~= nil then
> + if visited_keys[i] ~= true and v ~= box.NULL then
>
> ===========================================
>
> The tap:is_deeply call used to return inconsistent result
> processing an empty tuple and tuple that has no values:
> is_deeply({a = box.NULL}, {}) == true
> is_deeply({}, {a = box.NULL}) == false
> Fixed to return true in such case.
>
> We shouldn't return false in such case because of marshaling
> features covered by tests:
> is_deeply(decode(encode(t)), t) must be true for all t,
> including t = nil; decode(encode(nil)) == box.NULL.
>
> Closes #4125
> ---
> src/lua/tap.lua | 4 ++--
> test/app-tap/tap.result | 8 ++++++--
> test/app-tap/tap.test.lua | 10 +++++++++-
> 3 files changed, 17 insertions(+), 5 deletions(-)
>
> diff --git a/src/lua/tap.lua b/src/lua/tap.lua
> index 546d15392..e48adbbab 100644
> --- a/src/lua/tap.lua
> +++ b/src/lua/tap.lua
> @@ -117,8 +117,8 @@ local function cmpdeeply(got, expected, extra)
> end
>
> -- check if expected contains more keys then got
> - for i, _ in pairs(expected) do
> - if visited_keys[i] ~= true then
> + for i, v in pairs(expected) do
> + if visited_keys[i] ~= true and v ~= box.NULL then
> extra.expected = 'key ' .. tostring(i)
> extra.got = 'nil'
> return false
> diff --git a/test/app-tap/tap.result b/test/app-tap/tap.result
> index 3e7882331..dced9a5a2 100644
> --- a/test/app-tap/tap.result
> +++ b/test/app-tap/tap.result
> @@ -119,7 +119,7 @@ not ok - failed subtests
> failed: 1
> ...
> # is_deeply
> - 1..6
> + 1..10
> ok - 1 and 1
> ok - abc and abc
> ok - empty tables
> @@ -136,10 +136,14 @@ not ok - failed subtests
> expected: 5
> got: 4
> ...
> + ok - {} and {a = nil}
> + ok - {a = nil} and {}
> + ok - {a = nil} and {b = nil}
> + ok - {a = nil} and {b = nil, c = nil}
> # is_deeply: end
> not ok - failed subtests
> ---
> - planned: 6
> + planned: 10
> failed: 2
> ...
> # like
> diff --git a/test/app-tap/tap.test.lua b/test/app-tap/tap.test.lua
> index 0e1de7f1c..c0593db46 100755
> --- a/test/app-tap/tap.test.lua
> +++ b/test/app-tap/tap.test.lua
> @@ -118,7 +118,7 @@ end)
>
>
> test:test('is_deeply', function(t)
> - t:plan(6)
> + t:plan(10)
>
> t:is_deeply(1, 1, '1 and 1')
> t:is_deeply('abc', 'abc', 'abc and abc')
> @@ -127,6 +127,14 @@ test:test('is_deeply', function(t)
> t:is_deeply({1}, {2}, '{1} and {2}')
> t:is_deeply({1, 2, { 3, 4 }}, {1, 2, { 3, 5 }}, '{1,2,{3,4}} and {1,2,{3,5}}')
>
> + --
> + -- gh-4125: is_deeply inconsistently works with box.NULL.
> + --
> + t:is_deeply({}, {a = box.NULL}, '{} and {a = nil}')
> + t:is_deeply({a = box.NULL}, {}, '{a = nil} and {}')
> + t:is_deeply({a = box.NULL}, {b = box.NULL}, '{a = nil} and {b = nil}')
> + t:is_deeply({a = box.NULL}, {b = box.NULL, c = box.NULL},
> + '{a = nil} and {b = nil, c = nil}')
> end)
>
>
> --
> 2.21.0
>
More information about the Tarantool-patches
mailing list