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 2E1B7A77BC8; Thu, 11 Apr 2024 15:39:52 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 2E1B7A77BC8 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1712839192; bh=S3ycMQa2zpO7ypxbCGrC/b18jurreSP1vgTyzR4ggc0=; 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=cm2967LPGLHS27mwZ1ON3IJ6dgCgqLzg4Siq5o+eSAUJQ8dtb3TjI4XVDlMRwXw/r Sgj9oqHGHsM7eR0Q2hK6YuOT6nYI/lOK2/2GbVP5n5THon6qbWYMzRTH7IIFXPpG8v 8hwD6cR4jDqIrzx06NwliyFfEMwKxxyd95UAEB3c= Received: from smtp58.i.mail.ru (smtp58.i.mail.ru [95.163.41.96]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id E3763A77BC0 for ; Thu, 11 Apr 2024 15:39:50 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org E3763A77BC0 Received: by smtp58.i.mail.ru with esmtpa (envelope-from ) id 1rutiW-00000004okm-1FZ5; Thu, 11 Apr 2024 15:39:48 +0300 Message-ID: <619ad78c-b36c-48a5-ab67-d6b6b8065bbc@tarantool.org> Date: Thu, 11 Apr 2024 15:39:47 +0300 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird To: Sergey Kaplun References: <6492e6d3-f18b-411f-b1ab-ba4a16d1b0a0@tarantool.org> Content-Language: en-US In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Mailru-Src: smtp X-4EC0790: 10 X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD9D327C87852EB66D3B5F9CEDACDA8A37625F0445A84289E9B182A05F538085040443D1EDC0C1C4B27D4FF92D56319F1970731FE1845063130E5FE9E70C7813D4809E014349702AB77 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE76C0A440987CA342DC2099A533E45F2D0395957E7521B51C2CFCAF695D4D8E9FCEA1F7E6F0F101C6778DA827A17800CE796563325B6E011C88F08D7030A58E5AD1A62830130A00468AEEEE3FBA3A834EE7353EFBB553375663894BE6B133EBBEC7BBF288D8AC9E40E802DF1C3938220EB0DF1DFA3E7BFED61389733CBF5DBD5E913377AFFFEAFD269176DF2183F8FC7C0A29E2F051442AF778941B15DA834481FCF19DD082D7633A0EF3E4896CB9E6436389733CBF5DBD5E9D5E8D9A59859A8B6957A4DEDD2346B42CC7F00164DA146DA6F5DAA56C3B73B237318B6A418E8EAB8D32BA5DBAC0009BE9E8FC8737B5C2249A62421A90F508AD476E601842F6C81A12EF20D2F80756B5FB606B96278B59C4276E601842F6C81A127C277FBC8AE2E8B2E138E2D2EE104743AA81AA40904B5D99C9F4D5AE37F343AD1F44FA8B9022EA23BBE47FD9DD3FB595F5C1EE8F4F765FC72CEEB2601E22B093A03B725D353964B0B7D0EA88DDEDAC722CA9DD8327EE4930A3850AC1BE2E735BFC944FC657CDB72C4224003CC83647689D4C264860C145E X-C1DE0DAB: 0D63561A33F958A5637BBBCC2472C04E5002B1117B3ED69618323BD2F1137B08FB820E9FE7BD014C823CB91A9FED034534781492E4B8EEADAE4FDBF11360AC9BBDAD6C7F3747799A X-C8649E89: 1C3962B70DF3F0ADE00A9FD3E00BEEDF3FED46C3ACD6F73ED3581295AF09D3DF87807E0823442EA2ED31085941D9CD0AF7F820E7B07EA4CFF75D62A4C3771D1C6ADB4C74DC110221D3013BE66F4F8792A48DED9F12218E81F49F51687F14C9B843A9A4803F78FBB9B16AE0359725A1D36781A35CB76AF6B13A699FCF5B1E3419C226CC413062362A913E6812662D5F2AB9AF64DB4688768036DF5FE9C0001AF333F2C28C22F508233FCF178C6DD14203 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2bioja81YqLNwFpnMO62I2vnTUA== X-Mailru-Sender: 520A125C2F17F0B1A9638AD358559B59379B8C4808433325D4FF92D56319F1970731FE1845063130B7CBEF92542CD7C8795FA72BAB74744FC77752E0C033A69EA16A481184E8BB1C9B38E6EA4F046BE03A5DB60FBEB33A8A0DA7A0AF5A3A8387 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH luajit 0/2] Mach-O generation fixes 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 Bronnikov via Tarantool-patches Reply-To: Sergey Bronnikov Cc: Sergey Bronnikov , tarantool-patches@dev.tarantool.org Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" Hi, On 11.04.2024 11:27, Sergey Kaplun wrote: > Hi, > > On 11.04.24, Sergey Bronnikov wrote: >> Hi, >> >> On 08.04.2024 10:47, Sergey Kaplun wrote: >>> Hi, Sergey! >>> Thanks for the patch set! >>> >>> I'll proceed with the review according version on the branch: >>> >>> =================================================================== >>>> Subject: [PATCH 1/3] test: introduce a helper read_file >>> This patch LGTM, after fixing several nits below. >>> >>>> The test `lj-366-strtab-correct-size.test.lua` has a test helper >>>> `read_file` that reads a file's content and returns it. >>>> This helper will be useful for a test upcoming in the next commit, >>>> so it is moved to test tools. >>> So, missed the ticket mentioning: >>> | Needed for tarantool/tarantool#9595 >> It is actually not needed for 9595. > OK, not insisting. > >>>> --- >>>> .../lj-366-strtab-correct-size.test.lua | 10 +--------- >>>> test/tarantool-tests/utils/tools.lua | 8 ++++++++ >>>> 2 files changed, 9 insertions(+), 9 deletions(-) >>>> >>>> diff --git a/test/tarantool-tests/lj-366-strtab-correct-size.test.lua b/test/tarantool-tests/lj-366-strtab-correct-size.test.lua >>>> index 8a97a441..0bb92da6 100644 >>>> --- a/test/tarantool-tests/lj-366-strtab-correct-size.test.lua >>>> +++ b/test/tarantool-tests/lj-366-strtab-correct-size.test.lua >>>> @@ -138,14 +138,6 @@ local function create_obj_file(name) >>>> return elf_filename >>>> end >>>> >>>> --- Reads a file located in a specified path and returns its content. >>>> -local function read_file(path) >>>> - local file = assert(io.open(path), 'cannot open an object file') >>>> - local content = file:read('*a') >>>> - file:close() >>>> - return content >>>> -end >>>> - >>>> -- Parses a buffer in an ELF format and returns an offset and a size of strtab >>>> -- and symtab sections. >>>> local function read_elf(elf_content) >>>> @@ -172,7 +164,7 @@ end >>>> test:plan(3) >>>> >>>> local elf_filename = create_obj_file(MODULE_NAME) >>>> -local elf_content = read_file(elf_filename) >>>> +local elf_content = require('utils').tools.read_file(elf_filename) >>> Minor: I suggest avoiding the change at this line. >>> According to codestyle in other tests, we require helpers separately, >>> even if they are used only once. >> Where can I read a codestyle guide which you want to follow? > This is just a general tradition for all test files. Usually, it is a > good thing to do it this way because we don't want to use a lookup for > the module every time the function is called. This is not related to > your case directly, but if somebody moves this code to the function, he > should also change this part anyway. Plus, as a bonus, we will get an > earlier failure if the `read_file` module doesn't exist. > So, the answer is "we don't have nor a codestyle guide nor a static analysis that will force a certain style". That's why we always get stuck with things like this in code review. I've updated as you suggested. --- a/test/tarantool-tests/lj-366-strtab-correct-size.test.lua +++ b/test/tarantool-tests/lj-366-strtab-correct-size.test.lua @@ -9,6 +9,7 @@ local test = tap.test('lj-366-strtab-correct-size'):skipcond({  })  local ffi = require 'ffi' +local utils = require('utils')  -- Command below exports bytecode as an object file in ELF format:  -- $ luajit -b -n 'lango_team' -e 'print()' xxx.obj @@ -130,7 +131,7 @@ local is64 = is64_arch[jit.arch]  local function create_obj_file(name)    local elf_filename = os.tmpname() .. '.obj'    local lua_path = os.getenv('LUA_PATH') -  local lua_bin = require('utils').exec.luacmd(arg):match('%S+') +  local lua_bin = utils.exec.luacmd(arg):match('%S+')    local cmd_fmt = 'LUA_PATH="%s" %s -b -n "%s" -e "print()" %s'    local cmd = (cmd_fmt):format(lua_path, lua_bin, name, elf_filename)    local ret = os.execute(cmd) >>>> assert(#elf_content ~= 0, 'cannot read an object file') >>>> >>>> local strtab, symtab = read_elf(elf_content) >>>> diff --git a/test/tarantool-tests/utils/tools.lua b/test/tarantool-tests/utils/tools.lua >>>> index f35c6922..26b8c08d 100644 >>>> --- a/test/tarantool-tests/utils/tools.lua >>>> +++ b/test/tarantool-tests/utils/tools.lua >>>> @@ -12,4 +12,12 @@ function M.profilename(name) >>>> return (arg[0]:gsub('^(.+)/([^/]+)%.test%.lua$', replacepattern)) >>>> end >>>> >>>> +-- Reads a file located in a specified path and returns its content. >>> Typo: s/in/at/ >> Everything fine from Quillbot point of view. > But I have it, see [1]. > Sometimes it freezes, so you should reload the current page. Seems so. Fixed. > >> >>>> +function M.read_file(path) >>>> + local file = assert(io.open(path), 'cannot open an object file') >>>> + local content = file:read('*a') >>>> + file:close() >>>> + return content >>>> +end >>>> + >>>> return M >>>> -- >>>> 2.44.0 >>> =================================================================== >>> > [1]: https://ibb.co/XzCPC13 >