Fix Lua API tests #153

ligurio · 2025-08-06T16:36:34Z

No description provided.

Buristan

Hi, Sergey!
Thanks for the patchset!
I'll proceed with the review per-patch below.

[PATCH 1/7] tests/lapi: fix table_remove_test

LGTM, with a minor comment below.

Fix out-of-bound indices passed to table.remove().

Please specify the Lua version since which the behaviour has changed. For 5.1 just no value returned.

Buristan

[PATCH 2/7] tests/lapi: fix string_byte_test

Thanks for the patch!
Please consider my comments below.

string.char() returns the internal numeric codes of the

Typo: s/char/byte/

tests/lapi/string_byte_test.lua

Buristan

[PATCH 3/7] tests/lapi: fix string_rep_test

LGTM.

Buristan

[PATCH 4/7] tests/lapi: fix formatting
LGTM.

Buristan

[PATCH 5/7] tests/lapi: cache locales in random_locale()

LGTM.

Buristan

[PATCH 6/7] tests/lapi: add table.clear test

Please consider my coments below.

The patcha adds a test for table.clear() function that is

Typo: s/patcha/patch/

Buristan · 2025-08-22T10:49:00Z

tests/lapi/table_clear_test.lua

+    local fdp = luzer.FuzzedDataProvider(buf)
+    local count = fdp:consume_integer(0, test_lib.MAX_INT64)
+    local tbl = fdp:consume_strings(test_lib.MAX_STR_LEN, count)
+    table_clear(tbl)


It would be nice to check that there are no keys in this table after clearing.

Updated:

--- a/tests/lapi/table_clear_test.lua +++ b/tests/lapi/table_clear_test.lua @@ -32,6 +32,12 @@ local function TestOneInput(buf) local count = fdp:consume_integer(0, test_lib.MAX_INT64) local tbl = fdp:consume_strings(test_lib.MAX_STR_LEN, count) table_clear(tbl) + -- Make sure the table is empty. + local n_items = 0 + for _, _ in pairs(tbl) do + n_items = n_items + 1 + end + assert(n_items == 0) end local args = {

Minor: s/_, _/_/

Buristan

[PATCH 7/7] tests/lapi: update assertions in os tests

IINM, these values are not LuaJIT-specific, so there is no need to conditionally exclude them. The Lua may be built with the LUA_USE_POSIX as well to obtain the same behaviour. Looks like this patch is unnecessary.

ligurio · 2025-08-27T13:31:26Z

Added missed comments to tests:

--- a/tests/lapi/os_date_test.lua
+++ b/tests/lapi/os_date_test.lua
@@ -33,6 +33,7 @@ local function TestOneInput(buf)
     local ok, res = xpcall(os.date, err_handler, format, time)
     if not ok then return end
     local type_check = type(res) == "string" or type(res) == "table"
+    -- Undocumented.
     if test_lib.lua_version() == "LuaJIT" then
         assert(type_check or type(res) == "number" or res == nil)
     else
diff --git a/tests/lapi/os_time_test.lua b/tests/lapi/os_time_test.lua
index f8feebd..72a1483 100644
--- a/tests/lapi/os_time_test.lua
+++ b/tests/lapi/os_time_test.lua
@@ -33,6 +33,7 @@ local function TestOneInput(buf)
     local ok, res = xpcall(os.time, err_handler, time)
     if not ok then return end
     local type_check = type(res) == "number" or type(res) == "table"
+    -- Undocumented.
     if test_lib.lua_version() == "LuaJIT" then
         assert(type_check or res == nil)
     else

ligurio · 2025-08-27T13:51:13Z

Fix out-of-bound indices passed to table.remove().

Please specify the Lua version since which the behaviour has changed. For 5.1 just no value returned.

Since Lua 5.1 for non-empty tables:

$ lua5.2 -e "print(table.remove({1}, 0))"
lua5.2: (command line):1: bad argument #1 to 'remove' (position out of bounds)
stack traceback:
        [C]: in function 'remove'
        (command line):1: in main chunk
        [C]: in ?
$ lua5.3 -e "print(table.remove({1}, 0))"
lua5.3: (command line):1: bad argument #1 to 'remove' (position out of bounds)
stack traceback:
        [C]: in function 'table.remove'
        (command line):1: in main chunk
        [C]: in ?
$ lua5.4 -e "print(table.remove({1}, 0))"
lua5.4: (command line):1: bad argument #2 to 'remove' (position out of bounds)
stack traceback:
        [C]: in function 'table.remove'
        (command line):1: in main chunk
        [C]: in ?
$ lua5.4 -e "print(table.remove({}, 0))"
nil

Updated the test:

--- a/tests/lapi/table_remove_test.lua
+++ b/tests/lapi/table_remove_test.lua
@@ -20,7 +20,13 @@ local function TestOneInput(buf, _size)
     local tbl = fdp:consume_strings(test_lib.MAX_STR_LEN, count)
 
     local indices_count = fdp:consume_integer(0, #tbl)
-    local indices = fdp:consume_integers(1, count, indices_count)
+    local min_index = 0
+    -- PUC Rio Lua 5.2+ raises an error "position out of bounds"
+    -- when `pos` is equal to 0 and table is not empty.
+    if test_lib.lua_current_version_ge_than(5, 2) then
+        min_index = 1
+    end
+    local indices = fdp:consume_integers(min_index, count, indices_count)
     for _, idx in ipairs(indices) do
         local old_v = tbl[idx]
         assert(table.remove(tbl, idx) == old_v)

Fix out-of-bound indices passed to `table.remove()`.

ligurio · 2025-08-27T13:53:35Z

[PATCH 7/7] tests/lapi: update assertions in os tests

IINM, these values are not LuaJIT-specific, so there is no need to conditionally exclude them. The Lua may be built with the LUA_USE_POSIX as well to obtain the same behaviour. Looks like this patch is unnecessary.

The flag is enabled:

lua-c-api-tests/cmake/BuildLua.cmake

Lines 67 to 69 in 42ef050

    
           # `io.popen()` is not supported by default, it is enabled 
        
           # by `LUA_USE_POSIX` flag. Required by a function `random_locale()`. 
        
           set(CFLAGS "${CFLAGS} -DLUA_USE_POSIX")

Buristan · 2025-08-27T14:05:50Z

IINM, these values are not LuaJIT-specific, so there is no need to conditionally exclude them. The Lua may be built with the LUA_USE_POSIX as well to obtain the same behaviour. Looks like this patch is unnecessary.

The flag is enabled:

Yes, but it makes a difference only since 5.2 IINM. For 5.4-5.3 the same time input should raise an error, for example. For 5.1-5.2 nil should be pushed as well for the non-representable input as for the LuaJIT.

The patch fixes the following issues: - `string.byte()` returns the internal numeric codes of the characters, not a single number. - `string.char()` returns `nil` when values `i` or `j` are outside the acceptable range (less than zero and greater than the length of the string). It is not documented. Follows up #124

The patch fixes description (synopsis is at the end of the description) and fixes lower boundary for `n`, with `n` equal to zero assertion calculates negative number in right part of expression. Follows up #124

The patch changes function `random_locale()`, so it builds a table with locales only once on the first execution and then returns a cached table.

The patch adds a test for `table.clear()` function that is present in LuaJIT. Follows up the commit ee032e7 ("tests/lapi: add table tests"). Follows up #142

The patch updates assertions for values returned by `os.date()` and `os.time()`. Follows up #141

Buristan

Sergey,
Thanks for the fixes!
LGTM, after fixing a minor comment above.

Also, don't forget to fix the last (TMP) commit.

ligurio force-pushed the ligurio/gh-xxxx-fix-lapi-tests-2 branch from 9c77028 to 68047ba Compare August 7, 2025 08:18

ligurio requested a review from Buristan August 7, 2025 10:01

ligurio assigned ligurio and Buristan and unassigned ligurio Aug 7, 2025

ligurio force-pushed the ligurio/gh-xxxx-fix-lapi-tests-2 branch from 5ac345f to 42fa96a Compare August 8, 2025 13:06

Buristan approved these changes Aug 22, 2025

View reviewed changes

Buristan requested changes Aug 22, 2025

View reviewed changes

tests/lapi/string_byte_test.lua Outdated Show resolved Hide resolved

tests/lapi/string_byte_test.lua Outdated Show resolved Hide resolved

tests/lapi/string_byte_test.lua Outdated Show resolved Hide resolved

Buristan approved these changes Aug 22, 2025

View reviewed changes

Buristan requested changes Aug 22, 2025

View reviewed changes

Buristan assigned ligurio and unassigned Buristan Aug 22, 2025

tests/lapi: fix table_remove_test

509ef25

Fix out-of-bound indices passed to `table.remove()`.

ligurio force-pushed the ligurio/gh-xxxx-fix-lapi-tests-2 branch from 42fa96a to 2e2509b Compare August 27, 2025 13:56

ligurio force-pushed the ligurio/gh-xxxx-fix-lapi-tests-2 branch from 2e2509b to 699b395 Compare August 27, 2025 14:20

ligurio added 7 commits August 28, 2025 18:06

tests/lapi: fix string_rep_test

d76e27b

The patch fixes description (synopsis is at the end of the description) and fixes lower boundary for `n`, with `n` equal to zero assertion calculates negative number in right part of expression. Follows up #124

tests/lapi: fix formatting

0f2e3a4

tests/lapi: cache locales in random_locale()

e4b84d8

The patch changes function `random_locale()`, so it builds a table with locales only once on the first execution and then returns a cached table.

tests/lapi: add table.clear test

603ec82

The patch adds a test for `table.clear()` function that is present in LuaJIT. Follows up the commit ee032e7 ("tests/lapi: add table tests"). Follows up #142

tests/lapi: update assertions in os tests

d355a38

The patch updates assertions for values returned by `os.date()` and `os.time()`. Follows up #141

Fix PUC Rio Lua compilation [TMP]

23b7403

ligurio force-pushed the ligurio/gh-xxxx-fix-lapi-tests-2 branch from 699b395 to 23b7403 Compare August 28, 2025 15:06

ligurio requested a review from Buristan August 28, 2025 15:15

ligurio assigned Buristan and unassigned ligurio Aug 28, 2025

Buristan approved these changes Aug 28, 2025

View reviewed changes

Buristan assigned ligurio and unassigned Buristan Aug 28, 2025

Fix Lua API tests #153

Are you sure you want to change the base?

Fix Lua API tests #153

Uh oh!

Conversation

ligurio commented Aug 6, 2025

Uh oh!

Buristan left a comment

Choose a reason for hiding this comment

Uh oh!

Buristan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Buristan left a comment

Choose a reason for hiding this comment

Uh oh!

Buristan left a comment

Choose a reason for hiding this comment

Uh oh!

Buristan left a comment

Choose a reason for hiding this comment

Uh oh!

Buristan left a comment

Choose a reason for hiding this comment

Uh oh!

Buristan Aug 22, 2025

Choose a reason for hiding this comment

Uh oh!

ligurio Aug 27, 2025

Choose a reason for hiding this comment

Uh oh!

Buristan Aug 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Buristan left a comment

Choose a reason for hiding this comment

Uh oh!

ligurio commented Aug 27, 2025

Uh oh!

ligurio commented Aug 27, 2025

Uh oh!

ligurio commented Aug 27, 2025

Uh oh!

Buristan commented Aug 27, 2025

Uh oh!

Buristan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Buristan Aug 28, 2025 •

edited

Loading