From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id B692B6EC60; Thu, 1 Apr 2021 11:12:43 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org B692B6EC60 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1617264763; bh=n6mLDA0e9PduTZo6wQt63fB3GMoRyQkAPO1Yvhf25sc=; h=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=vvOROxz1A+DTpM/YmXQWmFoFpQUqFDoNUCPFwbsddsOydn9Wsf3WKAPFzDtxvDCJl XXOWT8RLKuRQgTKZDQHyINxwoooyucm8vJi2Aa6rF9sVG/CtDlBieIMV7i3l3Xcoox M99Y+8VfgjS0JZRYz2j+tTq/fLlRX/jeWGXoRBto= Received: from smtp16.mail.ru (smtp16.mail.ru [94.100.176.153]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id ABF716EC60 for ; Thu, 1 Apr 2021 11:12:41 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org ABF716EC60 Received: by smtp16.mail.ru with esmtpa (envelope-from ) id 1lRsRU-0002He-LQ; Thu, 01 Apr 2021 11:12:41 +0300 Date: Thu, 1 Apr 2021 11:11:46 +0300 To: Igor Munkin Message-ID: References: <99ce4b411ab34067105f6f7c58b7a736c32a05f7.1616743343.git.skaplun@tarantool.org> <20210330221352.GR29703@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210330221352.GR29703@tarantool.org> X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD9ED7173E37F4E32941B7C4A78AC10F96A7797F60C25BD4B06182A05F538085040BF46D8D492F93E6406CE3894B938DC1D399CB1938446A098F1774B5A374E669E X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE70B8ADF238913687CB287FD4696A6DC2FA8DF7F3B2552694A4E2F5AFA99E116B42401471946AA11AF23F8577A6DFFEA7C4B44AB1D52BB6B9B8F08D7030A58E5ADC58D69EE07B14084C6CDE5D1141D2B1C959206031EC1B528A697D9CD59A6815AB8F599EAAE0F95099FA2833FD35BB23D9E625A9149C048EE33AC447995A7AD18C26CFBAC0749D213D2E47CDBA5A96583BD4B6F7A4D31EC0BC014FD901B82EE079FA2833FD35BB23D27C277FBC8AE2E8B974A882099E279BDA471835C12D1D977C4224003CC8364762BB6847A3DEAEFB0F43C7A68FF6260569E8FC8737B5C2249B372FE9A2E580EFC725E5C173C3A84C3C74813BC7F81EC8435872C767BF85DA2F004C90652538430E4A6367B16DE6309 X-C1DE0DAB: 0D63561A33F958A560A69D6E14EF7C37EEB624374BE24DA41BD8810769D1198CD59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA7502E6951B79FF9A3F410CA545F18667F91A7EA1CDA0B5A7A0 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D344A2E3AE0A8792E697B6FC3D0AEF2BF69151B8703E65516088543A6DA2F5840A038052F23CC3A53391D7E09C32AA3244C10CCCA46005C305DB222C3FAA5C640503FD9C8CA1B0515E0FACE5A9C96DEB163 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojetunDCtJ20KvIRyGA8+3vQ== X-Mailru-Sender: 3B9A0136629DC91206CBC582EFEF4CB4F9033E1C0F63A0C5C3023B7E8B73D80361B7DBDA62385F35F2400F607609286E924004A7DEC283833C7120B22964430C52B393F8C72A41A89437F6177E88F7363CDA0F3B3F5B9367 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH v2 luajit 01/30] test: add PUC-Rio Lua 5.1 test suite X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Sergey Kaplun via Tarantool-patches Reply-To: Sergey Kaplun Cc: tarantool-patches@dev.tarantool.org Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" Igor, Thanks for the review! On 31.03.21, Igor Munkin wrote: > Sergey, > > Thanks for the patch! LGTM, except the nits below. > > On 26.03.21, Sergey Kaplun wrote: > > This patch adds PUC-Rio Lua 5.1 test suite as a part of the LuaJIT test > > suite. Source code taken verbatim (except trailing whitespaces) from > > Typo: s/code taken/code is taken/. > Typo: I believe it's always singular: whitespace. Fixed. > > > https://www.lua.org/tests/lua5.1-tests.tar.gz. > > > > Some tests may fail after this commit. They will be disabled > > or adapted in the next patches. > > > > Part of tarantool/tarantool#5845 > > Part of tarantool/tarantool#4473 > > --- > > .luacheckrc | 5 +- > > test/CMakeLists.txt | 2 + > > test/PUC-Lua-5.1-tests/CMakeLists.txt | 45 + > > test/PUC-Lua-5.1-tests/README | 41 + > > test/PUC-Lua-5.1-tests/all.lua | 137 +++ > > test/PUC-Lua-5.1-tests/api.lua | 711 ++++++++++++ > > test/PUC-Lua-5.1-tests/attrib.lua | 339 ++++++ > > test/PUC-Lua-5.1-tests/big.lua | 381 +++++++ > > test/PUC-Lua-5.1-tests/calls.lua | 294 +++++ > > test/PUC-Lua-5.1-tests/checktable.lua | 77 ++ > > test/PUC-Lua-5.1-tests/closure.lua | 422 +++++++ > > test/PUC-Lua-5.1-tests/code.lua | 143 +++ > > test/PUC-Lua-5.1-tests/constructs.lua | 240 ++++ > > test/PUC-Lua-5.1-tests/db.lua | 499 +++++++++ > > test/PUC-Lua-5.1-tests/errors.lua | 250 +++++ > > test/PUC-Lua-5.1-tests/etc/ltests.c | 1147 ++++++++++++++++++++ > > test/PUC-Lua-5.1-tests/etc/ltests.h | 92 ++ > > test/PUC-Lua-5.1-tests/events.lua | 360 ++++++ > > test/PUC-Lua-5.1-tests/files.lua | 324 ++++++ > > test/PUC-Lua-5.1-tests/gc.lua | 312 ++++++ > > test/PUC-Lua-5.1-tests/libs/CMakeLists.txt | 18 + > > test/PUC-Lua-5.1-tests/libs/lib1.c | 40 + > > test/PUC-Lua-5.1-tests/libs/lib11.c | 18 + > > test/PUC-Lua-5.1-tests/libs/lib2.c | 28 + > > test/PUC-Lua-5.1-tests/libs/lib21.c | 18 + > > test/PUC-Lua-5.1-tests/literals.lua | 176 +++ > > test/PUC-Lua-5.1-tests/locals.lua | 127 +++ > > test/PUC-Lua-5.1-tests/main.lua | 159 +++ > > test/PUC-Lua-5.1-tests/math.lua | 208 ++++ > > test/PUC-Lua-5.1-tests/nextvar.lua | 396 +++++++ > > test/PUC-Lua-5.1-tests/pm.lua | 273 +++++ > > test/PUC-Lua-5.1-tests/sort.lua | 74 ++ > > test/PUC-Lua-5.1-tests/strings.lua | 176 +++ > > test/PUC-Lua-5.1-tests/vararg.lua | 126 +++ > > test/PUC-Lua-5.1-tests/verybig.lua | 100 ++ > > 35 files changed, 7756 insertions(+), 2 deletions(-) > > create mode 100644 test/PUC-Lua-5.1-tests/CMakeLists.txt > > create mode 100644 test/PUC-Lua-5.1-tests/README > > create mode 100755 test/PUC-Lua-5.1-tests/all.lua > > Minor: I doubt all.lua need to be an executable. Feel free to ignore. The suite is taken with no change; this file was executable, and I see no reason to change it. Also, it highlights this file as the suite runner. Ignoring for now. > > > create mode 100644 test/PUC-Lua-5.1-tests/api.lua > > create mode 100644 test/PUC-Lua-5.1-tests/attrib.lua > > create mode 100644 test/PUC-Lua-5.1-tests/big.lua > > create mode 100644 test/PUC-Lua-5.1-tests/calls.lua > > create mode 100644 test/PUC-Lua-5.1-tests/checktable.lua > > create mode 100644 test/PUC-Lua-5.1-tests/closure.lua > > create mode 100644 test/PUC-Lua-5.1-tests/code.lua > > create mode 100644 test/PUC-Lua-5.1-tests/constructs.lua > > create mode 100644 test/PUC-Lua-5.1-tests/db.lua > > create mode 100644 test/PUC-Lua-5.1-tests/errors.lua > > create mode 100644 test/PUC-Lua-5.1-tests/etc/ltests.c > > create mode 100644 test/PUC-Lua-5.1-tests/etc/ltests.h > > create mode 100644 test/PUC-Lua-5.1-tests/events.lua > > create mode 100644 test/PUC-Lua-5.1-tests/files.lua > > create mode 100644 test/PUC-Lua-5.1-tests/gc.lua > > create mode 100644 test/PUC-Lua-5.1-tests/libs/CMakeLists.txt > > create mode 100644 test/PUC-Lua-5.1-tests/libs/lib1.c > > create mode 100644 test/PUC-Lua-5.1-tests/libs/lib11.c > > create mode 100644 test/PUC-Lua-5.1-tests/libs/lib2.c > > create mode 100644 test/PUC-Lua-5.1-tests/libs/lib21.c > > create mode 100644 test/PUC-Lua-5.1-tests/literals.lua > > create mode 100644 test/PUC-Lua-5.1-tests/locals.lua > > create mode 100644 test/PUC-Lua-5.1-tests/main.lua > > create mode 100644 test/PUC-Lua-5.1-tests/math.lua > > create mode 100644 test/PUC-Lua-5.1-tests/nextvar.lua > > create mode 100644 test/PUC-Lua-5.1-tests/pm.lua > > create mode 100644 test/PUC-Lua-5.1-tests/sort.lua > > create mode 100644 test/PUC-Lua-5.1-tests/strings.lua > > create mode 100644 test/PUC-Lua-5.1-tests/vararg.lua > > create mode 100644 test/PUC-Lua-5.1-tests/verybig.lua > > > > > > > diff --git a/test/PUC-Lua-5.1-tests/CMakeLists.txt b/test/PUC-Lua-5.1-tests/CMakeLists.txt > > new file mode 100644 > > index 0000000..773db0d > > --- /dev/null > > +++ b/test/PUC-Lua-5.1-tests/CMakeLists.txt > > @@ -0,0 +1,45 @@ > > +# Test suite that has been added from PUC-Rio Lua 5.1 test archive > > +# in scope of https://github.com/tarantool/tarantool/issues/5845. > > + > > +# See the rationale in the root CMakeLists.txt. > > +cmake_minimum_required(VERSION 3.1 FATAL_ERROR) > > + > > +# XXX: There are two ways to set up the proper environment > > +# described in the suite's README: > > +# * set LUA_PATH to "?;./?.lua" > > +# * or, better yet, set LUA_PATH to "./?.lua;;" and LUA_INIT to > > +# "package.path = '?;'..package.path" > > +# Unfortunately, Tarantool doesn't support LUA_INIT and most > > +# likely it never will. For more info, see > > +# https://github.com/tarantool/tarantool/issues/5744 > > +# Hence, there is no way other than set LUA_PATH environment > > +# variable as proposed in the first case. > > +set(LUA_PATH "?\;${CMAKE_CURRENT_SOURCE_DIR}/?.lua") > > + > > +# Set PUC-Lua-5.1-tests-prepare target that creates > > Minor: IMHO, "set" fits worse here than "create" or "introduce". Feel > free to ignore. Changed to "Establish" ("Introduce" fits more for the new functionality, not single target, in my opinion). I wanted to use "Create" first, but it invokes a tautology with "creates" later in the sentence. May be "Set up" will be better here. > > > +# subdirectory. > > +add_subdirectory(libs) > > + > > +# TODO: PUC-Rio Lua 5.1 test suite also has special header > > +# and translation unit to check some > > +# internal behaviour of the Lua implementation (see etc/ > > +# directory). It modifies realloc function to check memory > > +# consistency and also contains tests for yield in hooks > > +# and for the Lua C API. > > +# But, unfortunately, depends on specific PUC-Rio > > +# Lua 5.1 internal headers and should be adapted for LuaJIT. > > + > > +add_custom_target(PUC-Lua-5.1-tests > > + DEPENDS ${LUAJIT_TEST_BINARY} PUC-Lua-5.1-tests-prepare > > +) > > + > > +add_custom_command(TARGET PUC-Lua-5.1-tests > > + COMMENT "Running PUC-Rio Lua 5.1 tests" > > + COMMAND > > + env > > + LUA_PATH="${LUA_PATH}\;\;" > > + ${LUAJIT_TEST_COMMAND} ${CMAKE_CURRENT_SOURCE_DIR}/all.lua > > + WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR} > > +) > > + > > +# vim: expandtab tabstop=2 shiftwidth=2 > > > > > diff --git a/test/PUC-Lua-5.1-tests/libs/CMakeLists.txt b/test/PUC-Lua-5.1-tests/libs/CMakeLists.txt > > new file mode 100644 > > index 0000000..f24e7f3 > > --- /dev/null > > +++ b/test/PUC-Lua-5.1-tests/libs/CMakeLists.txt > > @@ -0,0 +1,18 @@ > > +# Test suite that has been added from PUC-Rio Lua 5.1 test archive > > +# in scope of https://github.com/tarantool/tarantool/issues/5845. > > + > > +# See the rationale in the root CMakeLists.txt. > > +cmake_minimum_required(VERSION 3.1 FATAL_ERROR) > > + > > Minor: What about TODO for building libs/*.c sources? Anyway, you did it > in the following patch, so feel free to ignore. Yep, ignoring, because the are added in the next patch for consistency. > > > +# The original tarball contains subdirectory "libs" with an empty > > +# subdirectory "libs/P1", to be used by tests. > > +# Instead of tracking empty directory with some anchor-file for > > +# git, create this directory via CMake. > > +add_custom_target(PUC-Lua-5.1-tests-prepare) > > +add_custom_command(TARGET PUC-Lua-5.1-tests-prepare > > + COMMENT "Create directory for PUC-Rio Lua 5.1 tests" > > + COMMAND ${CMAKE_COMMAND} -E make_directory P1 > > + WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR} > > +) > > + > > +# vim: expandtab tabstop=2 shiftwidth=2 > > > > > -- > > 2.31.0 > > > > -- > Best regards, > IM -- Best regards, Sergey Kaplun