Tarantool development patches archive
 help / color / mirror / Atom feed
* [Tarantool-patches] [PATCH v2] Move .tarantoolctl to test-run tool submodule
@ 2020-12-01 14:58 Alexander V. Tikhonov
  2020-12-06  1:52 ` Alexander Turenko
  0 siblings, 1 reply; 2+ messages in thread
From: Alexander V. Tikhonov @ 2020-12-01 14:58 UTC (permalink / raw)
  To: Alexander Turenko, Kirill Yukhin; +Cc: tarantool-patches

Moved .tarantoolctl configuration file to test-run tool submodule
repository as:

  <tarantool repository>/test-run/.tarantoolctl

Also set backward compability to use old path location:

  <tarantool repository>/test/.tarantoolctl

as its primary place.

Updated tests with replication_sync_timeout check value. Set it to
hidden value due to it could be set the other than default in options
at test-run run command.

Found that no need to copy tarantoolctl configuration file to binary
path any more, after it was moved to test-run repository, so reverting
changes from:

  aa609de2d1afeb8f99ae1630f9274e8f18a6cfea ('cmake for tests updated: copy ctl config in builddir')

Needed for #5504
---

Github: https://github.com/tarantool/tarantool/tree/avtikhon/move_tarantoolctl_config
Issue: https://github.com/tarantool/tarantool/issues/5504

 test/.tarantoolctl    | 15 ---------------
 test/CMakeLists.txt   |  8 --------
 test/box/admin.result |  2 +-
 test/box/box.lua      |  3 ++-
 test/box/cfg.result   |  4 ++--
 5 files changed, 5 insertions(+), 27 deletions(-)
 delete mode 100644 test/.tarantoolctl

diff --git a/test/.tarantoolctl b/test/.tarantoolctl
deleted file mode 100644
index 5c46f8acf..000000000
--- a/test/.tarantoolctl
+++ /dev/null
@@ -1,15 +0,0 @@
--- Options for test-run tarantoolctl
-
-local workdir = os.getenv('TEST_WORKDIR')
-default_cfg = {
-    pid_file   = workdir,
-    wal_dir    = workdir,
-    memtx_dir  = workdir,
-    vinyl_dir  = workdir,
-    log        = workdir,
-    background = false,
-}
-
-instance_dir = workdir
-
--- vim: set ft=lua :
diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt
index 10882c6a1..814ce10f1 100644
--- a/test/CMakeLists.txt
+++ b/test/CMakeLists.txt
@@ -78,13 +78,5 @@ add_subdirectory(unit)
 add_subdirectory(${PROJECT_SOURCE_DIR}/third_party/luajit/test
                  ${PROJECT_BINARY_DIR}/third_party/luajit/test)
 
-# Move tarantoolctl config
-if (NOT ${PROJECT_BINARY_DIR} STREQUAL ${PROJECT_SOURCE_DIR})
-    configure_file(
-        "${PROJECT_SOURCE_DIR}/test/.tarantoolctl"
-        "${PROJECT_BINARY_DIR}/test/.tarantoolctl"
-        )
-endif()
-
 # Disable connector_c for 1.6
 #add_subdirectory(connector_c)
diff --git a/test/box/admin.result b/test/box/admin.result
index 8c5626c36..e05440f66 100644
--- a/test/box/admin.result
+++ b/test/box/admin.result
@@ -86,7 +86,7 @@ cfg_filter(box.cfg)
   - - replication_sync_lag
     - 10
   - - replication_sync_timeout
-    - 300
+    - <hidden>
   - - replication_synchro_quorum
     - 1
   - - replication_synchro_timeout
diff --git a/test/box/box.lua b/test/box/box.lua
index 6fad07015..44d82ff0a 100644
--- a/test/box/box.lua
+++ b/test/box/box.lua
@@ -14,7 +14,8 @@ require('console').listen(os.getenv('ADMIN'))
 local _hide = {
     pid_file=1, log=1, listen=1, vinyl_dir=1,
     memtx_dir=1, wal_dir=1,
-    memtx_max_tuple_size=1, memtx_min_tuple_size=1
+    memtx_max_tuple_size=1, memtx_min_tuple_size=1,
+    replication_sync_timeout=1
 }
 
 function cfg_filter(data)
diff --git a/test/box/cfg.result b/test/box/cfg.result
index 4ad3c6493..6beae2e11 100644
--- a/test/box/cfg.result
+++ b/test/box/cfg.result
@@ -74,7 +74,7 @@ cfg_filter(box.cfg)
  |   - - replication_sync_lag
  |     - 10
  |   - - replication_sync_timeout
- |     - 300
+ |     - <hidden>
  |   - - replication_synchro_quorum
  |     - 1
  |   - - replication_synchro_timeout
@@ -187,7 +187,7 @@ cfg_filter(box.cfg)
  |   - - replication_sync_lag
  |     - 10
  |   - - replication_sync_timeout
- |     - 300
+ |     - <hidden>
  |   - - replication_synchro_quorum
  |     - 1
  |   - - replication_synchro_timeout
-- 
2.25.1

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [Tarantool-patches] [PATCH v2] Move .tarantoolctl to test-run tool submodule
  2020-12-01 14:58 [Tarantool-patches] [PATCH v2] Move .tarantoolctl to test-run tool submodule Alexander V. Tikhonov
@ 2020-12-06  1:52 ` Alexander Turenko
  0 siblings, 0 replies; 2+ messages in thread
From: Alexander Turenko @ 2020-12-06  1:52 UTC (permalink / raw)
  To: Alexander V. Tikhonov; +Cc: tarantool-patches

On Tue, Dec 01, 2020 at 05:58:36PM +0300, Alexander V. Tikhonov wrote:
> Moved .tarantoolctl configuration file to test-run tool submodule
> repository as:
> 
>   <tarantool repository>/test-run/.tarantoolctl
> 
> Also set backward compability to use old path location:
> 
>   <tarantool repository>/test/.tarantoolctl
> 
> as its primary place.
> 
> Updated tests with replication_sync_timeout check value. Set it to
> hidden value due to it could be set the other than default in options
> at test-run run command.
> 
> Found that no need to copy tarantoolctl configuration file to binary
> path any more, after it was moved to test-run repository, so reverting
> changes from:
> 
>   aa609de2d1afeb8f99ae1630f9274e8f18a6cfea ('cmake for tests updated: copy ctl config in builddir')
> 
> Needed for #5504
> ---
> 
> Github: https://github.com/tarantool/tarantool/tree/avtikhon/move_tarantoolctl_config
> Issue: https://github.com/tarantool/tarantool/issues/5504

The patch is okay. I made the following tweaks:

1. Added 'test' prefix.
2. Carried the long line.
3. Changed the first part of the message to clarify that the file was
   placed into test-run by the previous commit and just removed here.
4. Clarified how removal of the .tarantoolctl affects the behaviour.

The rest of the commit message is also dubious. For example, 'Updated
tests with replication_sync_timeout check value'. With what? What is the
'replication_sync_timeout check value'? I left it as is, but, please,
take care to grammar and how others may understand you.

Your task is to explain non-trivial things that were changed. It is not
something that should be in a reader responsibility: don't understand --
go ahead. No.

See any Vlad's commit message for inspiration. I don't know almost
anything about Raft and its implementation details, but I'm able to
understand an idea of a commit.

Pushed to master, 2.6, 2.5, 1.10.

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2020-12-06  1:52 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-01 14:58 [Tarantool-patches] [PATCH v2] Move .tarantoolctl to test-run tool submodule Alexander V. Tikhonov
2020-12-06  1:52 ` Alexander Turenko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox