From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id 6461228C61 for ; Thu, 9 Aug 2018 03:06:22 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id b0xGy8_qXLDD for ; Thu, 9 Aug 2018 03:06:22 -0400 (EDT) Received: from smtp15.mail.ru (smtp15.mail.ru [94.100.176.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id CC1B628C29 for ; Thu, 9 Aug 2018 03:06:21 -0400 (EDT) Date: Thu, 9 Aug 2018 10:06:20 +0300 From: Alexander Turenko Subject: [tarantool-patches] Re: [PATCH] lua: add string.fromhex method Message-ID: <20180809070619.i2dgrh5hnbutgmi2@tkn_work_nb> References: <20180808122103.40092-1-n.tatunov@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20180808122103.40092-1-n.tatunov@tarantool.org> Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-help: List-unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-subscribe: List-owner: List-post: List-archive: To: Nikita Tatunov Cc: tarantool-patches@freelists.org Hi, Nikita! The implementation LGTM. See minor comments about the test below. Please, proceed the next review round with Vlad. WBR, Alexander Turenko. On Wed, Aug 08, 2018 at 03:21:03PM +0300, N.Tatunov wrote: > Add string.fromhex method. Add test for string.fromhex(). > > Closes #2562 > --- > > Issue: https://github.com/tarantool/tarantool/issues/2562 > Branch: https://github.com/tarantool/tarantool/tree/N_Tatunov/gh-2562-fromhex-method > > <...> > > diff --git a/test/app-tap/string.test.lua b/test/app-tap/string.test.lua > index 1d10dcfc9..f88296fff 100755 > --- a/test/app-tap/string.test.lua > +++ b/test/app-tap/string.test.lua > @@ -3,7 +3,7 @@ > local tap = require('tap') > local test = tap.test("string extensions") > > -test:plan(6) > +test:plan(7) > > test:test("split", function(test) > test:plan(10) > @@ -114,6 +114,25 @@ test:test("hex", function(test) > test:is(string.hex(""), "", "hex empty string") > end) > > +test:test("fromhex", function(test) > + test:plan(11) > + test:is(string.fromhex("48656c6c6f"), "Hello", "from hex to bin") > + test:is(string.fromhex("4c696e7578"), "Linux", "from hex to bin") > + test:is(string.fromhex("6C6F72656D"), "lorem", "from hex to bin") > + test:is(string.fromhex("697073756D"), "ipsum", "from hex to bin") > + test:is(string.fromhex("6c6f72656d"), "lorem", "from hex to bin") > + test:is(string.fromhex("697073756d"), "ipsum", "from hex to bin") > + test:is(string.fromhex("6A6B6C6D6E6F"), "jklmno", "from hex to bin") > + test:is(string.fromhex("6a6b6c6d6e6f"), "jklmno", "from hex to bin") > + local _, err = pcall(string.fromhex, 'aaa') Use double quotes when a file primarily uses this quotes type. > + test:ok(err and err:match("(even amount of chars expected," .. > + " got odd amount)"), err) 1. Indent is strange (don't get what is the rule). 2. Don't use `err` for diagnostics (the message could can be unusable in case of an error). > + local _, err = pcall(string.fromhex, 'qq') Single qutoes -> doule quotes. > + test:ok(err and err:match("(hex string expected, got non hex chars)"), err) > + local _, err = pcall(string.fromhex, 795) > + test:ok(err and err:match("(string expected, got " .. type(795) .. ")")) type(795) -> number > +end) > + > test:test("strip", function(test) > test:plan(6) > local str = " hello hello " > -- > 2.15.2 (Apple Git-101.1) >