Re: [Tarantool-patches] [PATCH vshard 1/4] test: support luatest

[Oleg Babin via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

Thanks for your patch.

Am I right that it's partially imported from tarantool 
https://github.com/tarantool/tarantool/tree/master/test/luatest_helpers ?


LGTM. See 2 minor comments/questions below.



On 09.02.2022 03:32, Vladislav Shpilevoy wrote:
> Test-run's most recent guideline is to write new tests in luatest
> instead of diff console tests when possible. Luatest isn't exactly
> in a perfect condition now, but it has 2 important features which
> would be very useful in vshard:
>
> - Easy cluster build. All can be done programmatically except a
>    few basic things - from config creation to all replicasets
>    start, router start, and buckets bootstrap. No need to hardcode
>    that into files like storage_1_a.lua, router_1.lua, etc.
>
> - Can opt-out certain tests depending on Tarantool version. For
>    instance, soon coming support for netbox's return_raw option
>    will need to run tests only for > 2.10.0-beta2. In diff tests it
>    is also possible but would be notably complicated to achieve.
>
> Needed for #312
> ---
> All luatest_helpers/ except vtest.lua are blindly copied from Tarantool's main
> repo. I didn't check their bugs, code style, etc.
>
>   test-run                            |   2 +-
>   test/instances/router.lua           |  15 ++
>   test/instances/storage.lua          |  21 +++
>   test/luatest_helpers.lua            |  72 ++++++++
>   test/luatest_helpers/asserts.lua    |  43 +++++
>   test/luatest_helpers/cluster.lua    | 132 ++++++++++++++
>   test/luatest_helpers/server.lua     | 266 ++++++++++++++++++++++++++++
>   test/luatest_helpers/vtest.lua      | 135 ++++++++++++++
>   test/router-luatest/router_test.lua |  54 ++++++
>   test/router-luatest/suite.ini       |   5 +
>   10 files changed, 744 insertions(+), 1 deletion(-)
>   create mode 100755 test/instances/router.lua
>   create mode 100755 test/instances/storage.lua
>   create mode 100644 test/luatest_helpers.lua
>   create mode 100644 test/luatest_helpers/asserts.lua
>   create mode 100644 test/luatest_helpers/cluster.lua
>   create mode 100644 test/luatest_helpers/server.lua
>   create mode 100644 test/luatest_helpers/vtest.lua
>   create mode 100644 test/router-luatest/router_test.lua
>   create mode 100644 test/router-luatest/suite.ini
>
> diff --git a/test-run b/test-run
> index c345003..2604c46 160000
> --- a/test-run
> +++ b/test-run
> @@ -1 +1 @@
> -Subproject commit c34500365efe8316e79c7936a2f2d04644602936
> +Subproject commit 2604c46c7b6368dbde59489d5303ce3d1d430331
> diff --git a/test/instances/router.lua b/test/instances/router.lua
> new file mode 100755
> index 0000000..ccec6c1
> --- /dev/null
> +++ b/test/instances/router.lua
> @@ -0,0 +1,15 @@
> +#!/usr/bin/env tarantool
> +local helpers = require('test.luatest_helpers')
> +-- Do not load entire vshard into the global namespace to catch errors when code
> +-- relies on that.
> +_G.vshard = {
> +    router = require('vshard.router'),
> +}
> +-- Somewhy shutdown hangs on new Tarantools even though the nodes do not seem to
> +-- have any long requests running.
> +box.ctl.set_on_shutdown_timeout(0.001)
> +
> +box.cfg(helpers.box_cfg())
> +box.schema.user.grant('guest', 'super', nil, nil, {if_not_exists = true})
> +
> +_G.ready = true
> diff --git a/test/instances/storage.lua b/test/instances/storage.lua
> new file mode 100755
> index 0000000..2d679ba
> --- /dev/null
> +++ b/test/instances/storage.lua
> @@ -0,0 +1,21 @@
> +#!/usr/bin/env tarantool
> +local helpers = require('test.luatest_helpers')
> +-- Do not load entire vshard into the global namespace to catch errors when code
> +-- relies on that.
> +_G.vshard = {
> +    storage = require('vshard.storage'),
> +}
> +-- Somewhy shutdown hangs on new Tarantools even though the nodes do not seem to
> +-- have any long requests running.
> +box.ctl.set_on_shutdown_timeout(0.001)
> +
> +box.cfg(helpers.box_cfg())
> +box.schema.user.grant('guest', 'super', nil, nil, {if_not_exists = true})
> +
> +local function echo(...)
> +    return ...
> +end
> +
> +_G.echo = echo
> +
> +_G.ready = true
> diff --git a/test/luatest_helpers.lua b/test/luatest_helpers.lua
> new file mode 100644
> index 0000000..283906c
> --- /dev/null
> +++ b/test/luatest_helpers.lua
> @@ -0,0 +1,72 @@
> +local fun = require('fun')
> +local json = require('json')
> +local fio = require('fio')
> +local log = require('log')
> +local yaml = require('yaml')
> +local fiber = require('fiber')
> +
> +local luatest_helpers = {
> +    SOCKET_DIR = fio.abspath(os.getenv('VARDIR') or 'test/var')
> +}

Is fio.abspath really needed here? AFAIK the max length of unix socket 
is 108 symbols. Relative paths give a more chances that we don't face 
any issues.

> +
> +luatest_helpers.Server = require('test.luatest_helpers.server')
> +
> +local function default_cfg()
> +    return {
> +        work_dir = os.getenv('TARANTOOL_WORKDIR'),
> +        listen = os.getenv('TARANTOOL_LISTEN'),
> +        log = ('%s/%s.log'):format(os.getenv('TARANTOOL_WORKDIR'), os.getenv('TARANTOOL_ALIAS')),
> +    }
> +end
> +
> +local function env_cfg()
> +    local src = os.getenv('TARANTOOL_BOX_CFG')
> +    if src == nil then
> +        return {}
> +    end
> +    local res = json.decode(src)
> +    assert(type(res) == 'table')
> +    return res
> +end
> +
> +-- Collect box.cfg table from values passed through
> +-- luatest_helpers.Server({<...>}) and from the given argument.
> +--
> +-- Use it from inside an instance script.
> +function luatest_helpers.box_cfg(cfg)
> +    return fun.chain(default_cfg(), env_cfg(), cfg or {}):tomap()
> +end
> +
> +function luatest_helpers.instance_uri(alias, instance_id)
> +    if instance_id == nil then
> +        instance_id = ''
> +    end
> +    instance_id = tostring(instance_id)
> +    return ('%s/%s%s.iproto'):format(luatest_helpers.SOCKET_DIR, alias, instance_id);
> +end
> +
> +function luatest_helpers:get_vclock(server)
> +    return server:eval('return box.info.vclock')
> +end
> +
> +function luatest_helpers:wait_vclock(server, to_vclock)
> +    while true do
> +        local vclock = self:get_vclock(server)
> +        local ok = true
> +        for server_id, to_lsn in pairs(to_vclock) do
> +            local lsn = vclock[server_id]
> +            if lsn == nil or lsn < to_lsn then
> +                ok = false
> +                break
> +            end
> +        end
> +        if ok then
> +            return
> +        end
> +        log.info("wait vclock: %s to %s", yaml.encode(vclock),
> +                 yaml.encode(to_vclock))
> +        fiber.sleep(0.001)
> +    end
> +end
> +
> +return luatest_helpers
> diff --git a/test/luatest_helpers/asserts.lua b/test/luatest_helpers/asserts.lua
> new file mode 100644
> index 0000000..77385d8
> --- /dev/null
> +++ b/test/luatest_helpers/asserts.lua
> @@ -0,0 +1,43 @@
> +local t = require('luatest')
> +
> +local asserts = {}
> +
> +function asserts:new(object)
> +    self:inherit(object)
> +    object:initialize()
> +    return object
> +end
> +
> +function asserts:inherit(object)
> +    object = object or {}
> +    setmetatable(object, self)
> +    self.__index = self
> +    return object
> +end
> +
> +function asserts:assert_server_follow_upstream(server, id)
> +    local status = server:eval(
> +        ('return box.info.replication[%d].upstream.status'):format(id))
> +    t.assert_equals(status, 'follow',
> +        ('%s: this server does not follow others.'):format(server.alias))
> +end
> +
> +
> +function asserts:wait_fullmesh(servers, wait_time)
> +    wait_time = wait_time or 20
> +    t.helpers.retrying({timeout = wait_time}, function()
> +        for _, server in pairs(servers) do
> +            for _, server2 in pairs(servers) do
> +                if server ~= server2 then
> +                    local server_id = server:eval('return box.info.id')
> +                    local server2_id = server2:eval('return box.info.id')
> +                    if server_id ~= server2_id then
> +                            self:assert_server_follow_upstream(server, server2_id)
Indention looks broken here (8 spaces instead of 4).
> +                    end
> +                end
> +            end
> +        end
> +    end)
> +end
> +
> +return asserts
> diff --git a/test/luatest_helpers/cluster.lua b/test/luatest_helpers/cluster.lua
> new file mode 100644
> index 0000000..43e3479