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 7EF382ACBA for ; Wed, 24 Apr 2019 05:37:54 -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 dAHivrW9CmPY for ; Wed, 24 Apr 2019 05:37:54 -0400 (EDT) Received: from smtpng3.m.smailru.net (smtpng3.m.smailru.net [94.100.177.149]) (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 7EB7F2AB9B for ; Wed, 24 Apr 2019 05:37:53 -0400 (EDT) Subject: [tarantool-patches] [PATCH v2 1/1] tap: fix is_deeply box.NULL corner cases References: From: Kirill Shcherbatov Message-ID: <140e8187-8f84-c89d-10d8-57891e8f9353@tarantool.org> Date: Wed, 24 Apr 2019 12:37:50 +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: tarantool-patches@freelists.org, v.shpilevoy@tarantool.org After a discussion that took place on github after I submitted this patch, it was decided to introduce the ability to change the behavior when comparing box.NULL and nil in is, isnt, is_deeply tests using tap object flag 'strict'. ======================================================================= 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 by default in such case. You may also set tap.strict = true to change this behaviour. @TarantoolBot document Title: tap test new flag 'strict' In some scenarios it is convenient to distinguish box.NULL and nil in tap:is(), tap:isnt(), tap:is_deeply() tests. For example, {result='Success'} and {result='Success', error=None} are different HTTP responses. You may set t.strict = true to behave such way. Example: t = require('tap').test('123') t.strict = true t:is_deeply({a = box.NULL}, {}) -- false Closes #4125 --- Branch: http://github.com/tarantool/tarantool/tree/kshch/gh-4125-inconsistent-isdeeply-results Issue: https://github.com/tarantool/tarantool/issues/4125 src/lua/tap.lua | 22 +++++++++--- test/app-tap/tap.result | 75 ++++++++++++++++++++++++++++++++++++--- test/app-tap/tap.test.lua | 39 ++++++++++++++++++-- 3 files changed, 124 insertions(+), 12 deletions(-) diff --git a/src/lua/tap.lua b/src/lua/tap.lua index 546d15392..866786e6f 100644 --- a/src/lua/tap.lua +++ b/src/lua/tap.lua @@ -91,13 +91,18 @@ local function cmpdeeply(got, expected, extra) if ffi.istype('bool', got) then got = (got == 1) end if ffi.istype('bool', expected) then expected = (expected == 1) end - if got == nil and expected == nil then return true end + if extra.strict == false then + -- Convert all box.NULLs to LUA nil representation. + if got == box.NULL then got = nil end + if expected == box.NULL then expected = nil end + end if type(got) ~= type(expected) then extra.got = type(got) extra.expected = type(expected) return false end + if got == nil and expected == nil then return true end if type(got) ~= 'table' then extra.got = got @@ -117,8 +122,9 @@ 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 + (extra.strict == true or v ~= box.NULL) then extra.expected = 'key ' .. tostring(i) extra.got = 'nil' return false @@ -148,14 +154,18 @@ local function is(test, got, expected, message, extra) extra = extra or {} extra.got = got extra.expected = expected - return ok(test, got == expected, message, extra) + local rc = (test.strict == false or type(got) == type(expected)) and + got == expected + return ok(test, rc, message, extra) end local function isnt(test, got, unexpected, message, extra) extra = extra or {} extra.got = got extra.unexpected = unexpected - return ok(test, got ~= unexpected, message, extra) + local rc = (test.strict == true and type(got) ~= type(unexpected)) or + got ~= unexpected + return ok(test, rc, message, extra) end @@ -163,6 +173,7 @@ local function is_deeply(test, got, expected, message, extra) extra = extra or {} extra.got = got extra.expected = expected + extra.strict = test.strict return ok(test, cmpdeeply(got, expected, extra), message, extra) end @@ -225,6 +236,7 @@ local function test(parent, name, fun, ...) failed = 0; planned = 0; trace = parent == nil and true or parent.trace; + strict = false; }, test_mt) if fun ~= nil then test:diag('%s', test.name) diff --git a/test/app-tap/tap.result b/test/app-tap/tap.result index 3e7882331..13fa4fbcd 100644 --- a/test/app-tap/tap.result +++ b/test/app-tap/tap.result @@ -1,5 +1,5 @@ TAP version 13 -1..32 +1..40 ok - true ok - extra information is not printed on success not ok - extra printed using yaml only on failure @@ -69,6 +69,24 @@ not ok - cdata type expected: ctype got: ctype ... +not ok - box.NULL != nil + --- + got: null + ... +not ok - nil != box.NULL + --- + expected: null + ... +ok - box.NULL == box.NULL +ok - nil == nil +ok - box.NULL != nil +ok - nil != box.NULL +not ok - box.NULL == box.NULL + --- + unexpected: null + got: null + ... +not ok - nil == nil # subtest 1 1..2 ok - true @@ -119,7 +137,7 @@ not ok - failed subtests failed: 1 ... # is_deeply - 1..6 + 1..18 ok - 1 and 1 ok - abc and abc ok - empty tables @@ -127,20 +145,67 @@ not ok - failed subtests not ok - {1} and {2} --- path: //1 + strict: false expected: 2 got: 1 ... not ok - {1,2,{3,4}} and {1,2,{3,5}} --- path: //3/2 + strict: false expected: 5 got: 4 ... + ok - {} and {a = nil} strict = false + ok - {a = nil} and {} strict = false + ok - {a = nil} and {b = nil} strict = false + ok - {a = nil} and {b = nil, c = nil} strict = false + ok - nil and box.NULL strict = false + ok - box.NULL and nil strict = false + not ok - {} and {a = nil} strict = true + --- + strict: true + expected: key a + got: nil + ... + not ok - {a = nil} and {} strict = true + --- + path: //a + strict: true + expected: nil + got: cdata + ... + not ok - {a = nil} and {b = nil} strict = true + --- + path: //a + strict: true + expected: nil + got: cdata + ... + not ok - {a = nil} and {b = nil, c = nil} strict = true + --- + path: //a + strict: true + expected: nil + got: cdata + ... + not ok - nil and box.NULL strict = true + --- + got: nil + expected: cdata + strict: true + ... + not ok - box.NULL and nil strict = true + --- + got: cdata + expected: nil + strict: true + ... # is_deeply: end not ok - failed subtests --- - planned: 6 - failed: 2 + planned: 18 + failed: 8 ... # like 1..2 @@ -148,4 +213,4 @@ not ok - failed subtests ok - unlike(abcde, acd) # like: end ok - like -# failed subtest: 15 +# failed subtest: 19 diff --git a/test/app-tap/tap.test.lua b/test/app-tap/tap.test.lua index 0e1de7f1c..8a70e1c5d 100755 --- a/test/app-tap/tap.test.lua +++ b/test/app-tap/tap.test.lua @@ -20,7 +20,7 @@ test.trace = false -- ok, fail and skip predicates -- -test:plan(32) -- plan to run 3 test +test:plan(40) -- plan to run 40 test test:ok(true, 'true') -- basic function local extra = { state = 'some userful information to debug on failure', details = 'a table argument formatted using yaml.encode()' } @@ -60,6 +60,19 @@ test:iscdata(10, 'int', 'cdata type') test:iscdata(ffi.new('int', 10), 'int', 'cdata type') test:iscdata(ffi.new('unsigned int', 10), 'int', 'cdata type') +-- +-- gh-4125: Strict nulls comparisons. +-- +test.strict = true +test:is(box.NULL, nil, "box.NULL != nil") +test:is(nil, box.NULL, "nil != box.NULL") +test:is(box.NULL, box.NULL, "box.NULL == box.NULL") +test:is(nil, nil, "nil == nil") +test:isnt(box.NULL, nil, "box.NULL != nil") +test:isnt(nil, box.NULL, "nil != box.NULL") +test:isnt(box.NULL, box.NULL, "box.NULL == box.NULL") +test:isnt(nil, nil, "nil == nil") +test.strict = false -- -- Any test also can create unlimited number of sub tests. -- Subtest with callbacks (preferred). @@ -118,7 +131,7 @@ end) test:test('is_deeply', function(t) - t:plan(6) + t:plan(18) t:is_deeply(1, 1, '1 and 1') t:is_deeply('abc', 'abc', 'abc and abc') @@ -127,6 +140,28 @@ 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} strict = false') + t:is_deeply({a = box.NULL}, {}, '{a = nil} and {} strict = false') + t:is_deeply({a = box.NULL}, {b = box.NULL}, + '{a = nil} and {b = nil} strict = false') + t:is_deeply({a = box.NULL}, {b = box.NULL, c = box.NULL}, + '{a = nil} and {b = nil, c = nil} strict = false') + t:is_deeply(nil, box.NULL, 'nil and box.NULL strict = false') + t:is_deeply(box.NULL, nil, 'box.NULL and nil strict = false') + + t.strict = true + t:is_deeply({}, {a = box.NULL}, '{} and {a = nil} strict = true') + t:is_deeply({a = box.NULL}, {}, '{a = nil} and {} strict = true') + t:is_deeply({a = box.NULL}, {b = box.NULL}, + '{a = nil} and {b = nil} strict = true') + t:is_deeply({a = box.NULL}, {b = box.NULL, c = box.NULL}, + '{a = nil} and {b = nil, c = nil} strict = true') + t:is_deeply(nil, box.NULL, 'nil and box.NULL strict = true') + t:is_deeply(box.NULL, nil, 'box.NULL and nil strict = true') + t.strict = false end) -- 2.21.0