The whole idea of the patch-set introduce module for LuaJIT C tests. It
also, can be used for unit tests.
* The first patch is the prerequisite for the patch-set. It fixes
LD_LIBRARY_PATH definition.
* The 2nd and 3d patches provides an API and helper for writing the tests.
* The last 3 patches rewrite existing tests that should be written in C in
the proper way.

Branch: https://github.com/tarantool/luajit/tree/skaplun/gh-noticket-tarantool-c-tests
PR: https://github.com/tarantool/tarantool/pull/8444
Related Issue:
* https://github.com/tarantool/tarantool/issues/7900
* https://github.com/tarantool/tarantool/issues/781

Thanks Maxim, for the review!

I've fixed some Maxim comments and suggestions for the previous series.
Some ignorable comments about wording are ignored:).

Also, see answers for your questions below:

> >+if(NOT PROVE)
> >+ message(WARNING "`prove' is not found, so tarantool-c-tests target is not generated")
> >+ return()
> >+endif()
> There is the same check in the test/tarantool-tests/CMakeLists.txt. Maybe
> we should move it to the higher-level CMake so the lower-level CMakeLists
> inherit it.

I agree it maybe done, but not within this particular patchsett, so
ignoring for now.

Don’t see any reason to postpone it, tbh. New module requires the same

checks as the already present one. It seems logical to do necessary changes

in this patchset.

> >+#define test_run_group(t_arr, t_state) \
> >+ _test_run_group(__func__, t_arr, lengthof(t_arr), t_state)
> Is there any reason for it to be a macro and not a function wrapper?
> I believe it is better to use the functions when possible, since they are
> easier to support and debug.

Just for the convenience in usage of __func__ macro as a test group name.

Oh, I see, thx.

> >+/* Need for `strchr()` in diagnostic parsing. */
> `strchr()` is not safe, despite the fact it searches till `\0`.
> We should at least replace it with `memchr()`, which has
> the explicit constraint for buffer length.
> >+#include <string.h>

Yes, but:
1) We use it only for our test code, where we set this `\0` directly to
mark EOL.
2) It's simplier than use several marks in buffer.
So, ignoring for now.

> >+# vim: ft=cmake expandtab shiftwidth=2: tabstop=2:
> That change is not necessary.

Yes, but more convenient to use in vim -- since our usual codestyle isn't 4
tabs as its default for CMake. :)
Still we don't use it anywhere (unfortunately), so removed.

> >+ bail_out("failed to translate Lua code snippet");
> Why `bail_out` and not an assertion? Here and below.

Assertion is for some thing we wnat to test and may fail.
Bail out usage is more specific:
| As an emergency measure a test script can decide that further tests are
| useless (e.g. missing dependencies) and testing should stop immediately.
| In that case the test script prints the magic words
See [1]. I think that loading Lua script helper (dependency) is
something like that.

Thanks for the clarification.

> >+#include "lj_arch.h"
> Side note: I don't like the approach with private headers, but
> I couldn't find any better way to check that. Maybe it is a good
> idea to implement a public C API function to get the information
> about OS and ARCH, since it is a really common to check them?

I think, that this is the best option, espessialy if we want to write
some unit test for some specific module (I mean <src/lj_*>).

Changes in v2:
1) use

| int _test_run_group(const char *group_name, const struct test_unit tests[],
| size_t n_tests, void *test_state);

instead of

| int _test_run_group(const char *group_name, const struct test_unit *tests,
| size_t n_tests, void *test_state);

2) `skip()` `skip_all()` and `todo()` helpers now return values to be
return to runner.
i.e. change usage from
| if (cond)
| skip("NIY");
to
| if (cond)
| return skip("NIY");

`bail_out()` helper still just exits with error code, which corresponding
its standard specification.

But now some parts of the code start to look "alya cringe":
| return todo("Need to replace backtrace with libunwind first");
| lua_State *L = test_state;
| utils_get_aux_lfunc(L);
| (void)luaJIT_setmode(L, 0, LUAJIT_MODE_ENGINE | LUAJIT_MODE_OFF);
| (void)luaJIT_setmode(L, 0, LUAJIT_MODE_ENGINE | LUAJIT_MODE_FLUSH);
| check_profile_func(L);
| (void)luaJIT_setmode(L, 0, LUAJIT_MODE_ENGINE | LUAJIT_MODE_ON);
| return TEST_EXIT_SUCCESS;

Well, what is cringe here? There are a few unreachable lines, but now it is

obvious that those are skipped.

(Yes, we want to use unconditional `todo()`).
So I commented the similar code, helper `check_profile_func()`, etc.
with `#if 0`.

Do we really need to that though? Again, it is clearly visible that those are

unreachable. Comment in `todo` is sufficient.

--
Best regards,

Maxim Kokryashkin