[tarantool-patches] Re: [PATCH 1/3] Add test on error during reconfigure

  • From: Vladislav Shpilevoy <v.shpilevoy@xxxxxxxxxxxxx>
  • To: tarantool-patches@xxxxxxxxxxxxx, AKhatskevich <avkhatskevich@xxxxxxxxxxxxx>
  • Date: Thu, 19 Jul 2018 18:14:47 +0300

Hi! Thanks for the patch! See 3 comments below.

On 18/07/2018 20:47, AKhatskevich wrote:

In case reconfigure process fails, the node should continue
work properly.
---
  test/lua_libs/util.lua        | 16 ++++++++++++++++
  test/router/router.result     | 33 +++++++++++++++++++++++++++++++++
  test/router/router.test.lua   | 10 ++++++++++
  test/storage/storage.result   | 39 +++++++++++++++++++++++++++++++++++++++
  test/storage/storage.test.lua | 12 ++++++++++++
  vshard/router/init.lua        |  7 +++++++
  vshard/storage/init.lua       |  9 +++++++++
  7 files changed, 126 insertions(+)

diff --git a/test/lua_libs/util.lua b/test/lua_libs/util.lua
index f2d3b48..aeb2342 100644
--- a/test/lua_libs/util.lua
+++ b/test/lua_libs/util.lua
@@ -69,9 +69,25 @@ local function wait_master(test_run, replicaset, master)
      log.info('Slaves are connected to a master "%s"', master)
  end
+-- Check that data has at least all fields as an ethalon.

1. Typo here and in other places: ethalon -> etalon.

2. Please, describe the function in doxygen style since
it is not a trival one-liner.

+local function has_same_fields(ethalon, data)
+    assert(type(ethalon) == 'table' and type(data) == 'table')
+    local diff = {}
+    for k, v in pairs(ethalon) do
+        if v ~= data[k] then
+            table.insert(diff, k)
+        end
+    end
+    if #diff > 0 then
+        return false, diff
+    end
+    return true
+end
+
  return {
      check_error = check_error,
      shuffle_masters = shuffle_masters,
      collect_timeouts = collect_timeouts,
      wait_master = wait_master,
+    has_same_fields = has_same_fields,
  }
diff --git a/test/router/router.result b/test/router/router.result
index 15f4fd0..7ec3e15 100644
--- a/test/router/router.result
+++ b/test/router/router.result
@@ -1156,6 +1156,39 @@ util.check_error(vshard.router.cfg, non_dynamic_cfg)
  ---
  - Non-dynamic option shard_index cannot be reconfigured
  ...
+-- Error during reconfigure process.
+vshard.router.route(1):callro('echo', {'some_data'})
+---
+- some_data
+- null
+- null
+...
+vshard.router.internal.errinj.ERRINJ_CFG = true
+---
+...
+old_internal = table.copy(vshard.router.internal)
+---
+...
+_, err = pcall(vshard.router.cfg, cfg)
+---
+...
+err:match('Error injection:.*')

3. P l e a s e. Again. Do not use pcall + match. Use
util.check_error.

Other related posts: