Tarantool development patches archive
 help / color / mirror / Atom feed
From: Maxim Kokryashkin via Tarantool-patches <tarantool-patches@dev.tarantool.org>
To: Sergey Kaplun <skaplun@tarantool.org>
Cc: tarantool-patches@dev.tarantool.org
Subject: Re: [Tarantool-patches] [PATCH v2 luajit 5/7] test: shrink LUA_CPATH and {DY}LD_LIBRARY_PATH
Date: Fri, 11 Oct 2024 21:37:05 +0300	[thread overview]
Message-ID: <2mst2bpxwtdb72ivhgezbreecwzrvfa6rs77dz5q5rjtfvqiy2@ap3jrgnzhojw> (raw)
In-Reply-To: <15a3b416520eac8f799a59fa51c6dcd9515ec90b.1727172936.git.skaplun@tarantool.org>

Hi, Sergey!
Thanks for the patch!
LGTM, except for a single nit below.

On Tue, Sep 24, 2024 at 01:29:50PM GMT, Sergey Kaplun wrote:
> This patch sets the unique value of each of these variables for each of
> the tests and only where they are needed. Also, it drops the comment
> about SIP [1] tricks since it is obsolete after
> 29897567ee5ed57e961c730432c056a3dbaa8f09 ("test: stop using
> utils.selfrun in tests").
>
> [1]: https://support.apple.com/en-us/HT204899
>
> Resolves tarantool/tarantool#9898
> ---
>  test/tarantool-tests/CMakeLists.txt           | 85 ++++++++-----------
>  test/tarantool-tests/ffi-ccall/CMakeLists.txt |  2 +-
>  .../fix-bit-shift-generation/CMakeLists.txt   |  3 +-
>  .../gh-4427-ffi-sandwich/CMakeLists.txt       |  3 +-
>  .../CMakeLists.txt                            |  3 +-
>  .../gh-6189-cur_L/CMakeLists.txt              |  3 +-
>  .../lj-1004-oom-error-frame/CMakeLists.txt    |  3 +-
>  .../CMakeLists.txt                            |  3 +-
>  .../lj-1166-error-stitch/CMakeLists.txt       |  6 +-
>  .../lj-416-xor-before-jcc/CMakeLists.txt      |  3 +-
>  .../CMakeLists.txt                            |  3 +-
>  .../lj-549-bytecode-loader/CMakeLists.txt     |  3 +-
>  .../CMakeLists.txt                            |  3 +-
>  .../lj-601-fix-gc-finderrfunc/CMakeLists.txt  |  3 +-
>  .../lj-727-lightuserdata-itern/CMakeLists.txt |  3 +-
>  .../CMakeLists.txt                            |  3 +-
>  .../lj-flush-on-trace/CMakeLists.txt          |  3 +-
>  .../both/CMakeLists.txt                       |  4 +-
>  .../gnuhash/CMakeLists.txt                    |  4 +-
>  .../hash/CMakeLists.txt                       |  4 +-
>  .../stripped/CMakeLists.txt                   |  4 +-
>  21 files changed, 82 insertions(+), 69 deletions(-)
>
> diff --git a/test/tarantool-tests/CMakeLists.txt b/test/tarantool-tests/CMakeLists.txt
> index 5e3493b6..d0964aed 100644
> --- a/test/tarantool-tests/CMakeLists.txt
> +++ b/test/tarantool-tests/CMakeLists.txt
> @@ -5,17 +5,16 @@ add_custom_target(tarantool-tests-libs
>    DEPENDS libluajit
>  )
>
> -macro(BuildTestCLib lib sources)
> -  AddTestLib(${lib} ${sources})
> +macro(BuildTestCLib lib source for_tests)

This whole macro signature now reads something like `lib source
for_tests` which looks kind of misleading, especially in the code below.

IMO, it would be better to call the last variable like `dependent_tests`
for better readability. Feel free to ignore, though.

 --
> 2.46.0
>

  parent reply	other threads:[~2024-10-11 18:37 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-09-24 10:29 [Tarantool-patches] [PATCH v2 luajit 0/7] Shrink test env and fix flaky tests Sergey Kaplun via Tarantool-patches
2024-09-24 10:29 ` [Tarantool-patches] [PATCH v2 luajit 1/7] test: move profilers tests to subdirectory Sergey Kaplun via Tarantool-patches
2024-09-24 11:07   ` Sergey Bronnikov via Tarantool-patches
2024-10-11 18:28   ` Maxim Kokryashkin via Tarantool-patches
2024-09-24 10:29 ` [Tarantool-patches] [PATCH v2 luajit 2/7] test: rename <arm64-ccall-fp-convention.test.lua> Sergey Kaplun via Tarantool-patches
2024-09-24 11:07   ` Sergey Bronnikov via Tarantool-patches
2024-10-11 18:29   ` Maxim Kokryashkin via Tarantool-patches
2024-09-24 10:29 ` [Tarantool-patches] [PATCH v2 luajit 3/7] cmake: introduce AppendTestEnvVar macro Sergey Kaplun via Tarantool-patches
2024-09-24 11:07   ` Sergey Bronnikov via Tarantool-patches
2024-10-11 18:30   ` Maxim Kokryashkin via Tarantool-patches
2024-09-24 10:29 ` [Tarantool-patches] [PATCH v2 luajit 4/7] test: shrink LUA_PATH environment variable Sergey Kaplun via Tarantool-patches
2024-09-24 11:15   ` Sergey Bronnikov via Tarantool-patches
2024-09-24 11:33     ` Sergey Kaplun via Tarantool-patches
     [not found]       ` <ZvO5g6ryIcyhM1vg@root>
2024-09-25  7:22         ` Sergey Bronnikov via Tarantool-patches
2024-10-11 18:31   ` Maxim Kokryashkin via Tarantool-patches
2024-09-24 10:29 ` [Tarantool-patches] [PATCH v2 luajit 5/7] test: shrink LUA_CPATH and {DY}LD_LIBRARY_PATH Sergey Kaplun via Tarantool-patches
2024-09-24 11:16   ` Sergey Bronnikov via Tarantool-patches
2024-09-24 11:28     ` Sergey Kaplun via Tarantool-patches
2024-09-24 14:18       ` Sergey Bronnikov via Tarantool-patches
2024-10-11 18:37   ` Maxim Kokryashkin via Tarantool-patches [this message]
2024-10-12  7:00     ` Sergey Kaplun via Tarantool-patches
2024-09-24 10:29 ` [Tarantool-patches] [PATCH v2 luajit 6/7] test: skip flaky tests with enabled table bump Sergey Kaplun via Tarantool-patches
2024-09-24 11:26   ` Sergey Bronnikov via Tarantool-patches
2024-09-24 11:45     ` Sergey Kaplun via Tarantool-patches
2024-09-24 14:17       ` Sergey Bronnikov via Tarantool-patches
2024-10-11 18:38   ` Maxim Kokryashkin via Tarantool-patches
2024-09-24 10:29 ` [Tarantool-patches] [PATCH v2 luajit 7/7] test: set LD_PRELOAD only when necessary Sergey Kaplun via Tarantool-patches
2024-09-24 14:13   ` Sergey Bronnikov via Tarantool-patches
2024-09-24 15:23     ` Sergey Kaplun via Tarantool-patches
2024-09-24 17:41       ` Sergey Bronnikov via Tarantool-patches
2024-10-11 18:46   ` Maxim Kokryashkin via Tarantool-patches
2024-10-18 15:18 ` [Tarantool-patches] [PATCH v2 luajit 0/7] Shrink test env and fix flaky tests Sergey Kaplun via Tarantool-patches

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2mst2bpxwtdb72ivhgezbreecwzrvfa6rs77dz5q5rjtfvqiy2@ap3jrgnzhojw \
    --to=tarantool-patches@dev.tarantool.org \
    --cc=m.kokryashkin@tarantool.org \
    --cc=skaplun@tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH v2 luajit 5/7] test: shrink LUA_CPATH and {DY}LD_LIBRARY_PATH' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

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