From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id BEDF62B053 for ; Tue, 23 Apr 2019 10:14:29 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id GKouwjP5v9lM for ; Tue, 23 Apr 2019 10:14:29 -0400 (EDT) Received: from smtpng2.m.smailru.net (smtpng2.m.smailru.net [94.100.179.3]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 7E9A4282B1 for ; Tue, 23 Apr 2019 10:14:29 -0400 (EDT) Subject: [tarantool-patches] Re: [PATCH v1 1/1] tap: fix is_deeply box.NULL corner cases References: From: Vladislav Shpilevoy Message-ID: Date: Tue, 23 Apr 2019 17:14:26 +0300 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-Help: List-Unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-Subscribe: List-Owner: List-post: List-Archive: To: Kirill Shcherbatov , tarantool-patches@freelists.org, Kirill Yukhin 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 >