Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Strange assert_equals behaviour for tables returned from or passed to server:exec #310

Open
sergepetrenko opened this issue Jun 2, 2023 · 1 comment
Labels
bug Something isn't working

Comments

@sergepetrenko
Copy link
Contributor

TL; DR:

  1. t.assert_equals(expect_tbl, return_tbl) on line 19 fails, but
    t.assert_equals(return_tbl, expect_tbl) on line 20 passes!
  2. t.assert_equals(tbl, expect) on line 24 fails, but
    t.assert_equals(expect, tbl) on line 23 passes!

The problems I see:
a) server:exec() encodes table arguments / return values with extra box.NULL when passing them to / from other server
b) assert_equals works when the argument passed over the net is first, but doesn't work when that argument is second.

The details:
Consider the following reproducer:

repro_test.lua
local t = require('luatest')
local server = require('luatest.server')
local g = t.group('repro')

g.test_repro = function(cg)
    local server = server:new{}
    server:start()
    local expect_tbl = server:exec(function()
        return {1, 2, 3, 4, 5, 6, 7, 8, 9}
    end)
    local return_tbl = server:exec(function(expect)
        local tbl = {1, 2, 3, 4, 5, 6, 7, 8, 9}
        t.assert_equals(expect, tbl)
        t.assert_equals(tbl, expect)
        tbl[16] = 'something'
        return tbl
    end, {expect_tbl})
    expect_tbl[16] = 'something'
    t.assert_equals(expect_tbl, return_tbl)
    t.assert_equals(return_tbl, expect_tbl)
    server:exec(function(expect)
        local tbl = {1, 2, 3, 4, 5, 6, 7, 8, 9, [16] = 'something'}
        t.assert_equals(expect, tbl)
        t.assert_equals(tbl, expect)
    end, {expect_tbl})
    server:drop()
end

When I run it with luatest, luatest repro_test.lua -v, it fails like this:

fail 1
Failed tests:
-------------

1) repro.test_repro
/Users/s.petrenko/Source/sptnt/test/box-luatest/r_test.lua:19: expected: 
{
    1,
    2,
    3,
    4,
    5,
    6,
    7,
    8,
    9,
    cdata<void *>: NULL,
    cdata<void *>: NULL,
    cdata<void *>: NULL,
    cdata<void *>: NULL,
    cdata<void *>: NULL,
    cdata<void *>: NULL,
    "something",
}
actual: 
{1, 2, 3, 4, 5, 6, 7, 8, 9, [16] = "something"}
List difference analysis:
* lists A (actual) and B (expected) have the same size
* lists A and B start differing at index 17
* lists A and B are equal again from index 1
* Common parts:
  = A[1], B[1]: 1
  = A[2], B[2]: 2
  = A[3], B[3]: 3
  = A[4], B[4]: 4
  = A[5], B[5]: 5
  = A[6], B[6]: 6
  = A[7], B[7]: 7
  = A[8], B[8]: 8
  = A[9], B[9]: 9
  = A[10], B[10]: nil
  = A[11], B[11]: nil
  = A[12], B[12]: nil
  = A[13], B[13]: nil
  = A[14], B[14]: nil
  = A[15], B[15]: nil
  = A[16], B[16]: "something"
* Common parts at the end of the lists
  = A[1], B[1]: 1
  = A[2], B[2]: 2
  = A[3], B[3]: 3
  = A[4], B[4]: 4
  = A[5], B[5]: 5
  = A[6], B[6]: 6
  = A[7], B[7]: 7
  = A[8], B[8]: 8
  = A[9], B[9]: 9
  = A[10], B[10]: nil
  = A[11], B[11]: nil
  = A[12], B[12]: nil
  = A[13], B[13]: nil
  = A[14], B[14]: nil
  = A[15], B[15]: nil
  = A[16], B[16]: "something"
stack traceback:
	/Users/s.petrenko/Source/sptnt/test/box-luatest/r_test.lua:19: in function 'repro.test_repro'
	...
	[C]: in function 'xpcall'

Now, if I comment the line 19 (t.assert_equals(expect_tbl, return_tbl)), it starts failing in another place.
(Note, that line 20 passes successfully (t.assert_equals(return_tbl, expect_tbl)) and it has the same arguments as line 19 in reverse order.

fail2
Tests with errors:
------------------

1) repro.test_repro
{"status":"fail","class":"LuatestError","message":"\/Users\/s.petrenko\/Source\/sptnt\/test\/box-luatest\/r_test.lua:24: expected: \n{\n    1,\n    2,\n    3,\n    4,\n    5,\n    6,\n    7,\n    8,\n    9,\n    cdata<void *>: NULL,\n    cdata<void *>: NULL,\n    cdata<void *>: NULL,\n    cdata<void *>: NULL,\n    cdata<void *>: NULL,\n    cdata<void *>: NULL,\n    \"something\",\n}\nactual: \n{1, 2, 3, 4, 5, 6, 7, 8, 9, [16] = \"something\"}\nList difference analysis:\n* lists A (actual) and B (expected) 
stack traceback:
	/Users/s.petrenko/Source/sptnt/test/box-luatest/r_test.lua:21: in function 'repro.test_repro'
	...
	[C]: in function 'xpcall'

Ok, now I comment out line 24 (t.assert_equals(_G.ballot, expect)) and the test passes! So:

  1. t.assert_equals(expect_tbl, return_tbl) on line 19 fails, but
    t.assert_equals(return_tbl, expect_tbl) on line 20 passes!
  2. t.assert_equals(tbl, expect) on line 24 fails, but
    t.assert_equals(expect, tbl) on line 23 passes!

The problems I see:
a) server:exec() encodes table arguments / return values with extra box.NULL when passing them to / from other server
b) assert_equals works when the argument passed over the net is first, but doesn't work when that argument is second.

@ylobankov ylobankov added the bug Something isn't working label Jun 2, 2023
@Totktonada
Copy link
Member

I use the following helper for the strict validation (with nil/box.NULL differentiation).

-- Compare types deeply.
--
-- The main motivation is to differentiate nil and box.NULL.
--
-- The following calls do not raise an error:
--
-- * t.assert_equals(nil, box.NULL)
-- * t.assert_equals(box.NULL, nil) too.
-- * t.assert_equals({foo = box.NULL}, {foo = nil})
--
-- However, the following does:
--
-- * t.assert_equals({foo = nil}, {foo = box.NULL})
--
-- See also https://github.com/tarantool/luatest/issues/310
local function assert_type_equals(a, b)
    t.assert_equals(type(a), type(b))
    if type(a) ~= 'table' then 
        return
    end  
    assert(type(b) == 'table')

    for k, v in pairs(a) do
        assert_type_equals(v, b[k])
    end  
    for k, v in pairs(b) do
        assert_type_equals(v, a[k])
    end  
end

My usage:

t.assert_equals(res, exp)
assert_type_equals(res, exp)

I doubt that we can strengthen the assert_equals() function, but it should be OK to add assert_equals_strict() (or whatever we'll name it).

OTOH, we added the strict flag for tap's is_deeply() method four years ago (see tarantool/tarantool@8e33785 plus tarantool/tarantool@676582c) and it was never used, at least in tarantool tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants