From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp40.i.mail.ru (smtp40.i.mail.ru [94.100.177.100]) (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 5F42E4765E0 for ; Wed, 23 Dec 2020 14:09:26 +0300 (MSK) References: <09d776ce-85fc-d5a5-f85a-2bf86f2d73fc@tarantool.org> <0c64a8dd571e8f9e491b84cd9bfbba77df4a3e2f.1608624736.git.estetus@gmail.com> <7bb1a2a2-a8cf-f2b9-c81e-48246c45078a@tarantool.org> <5a2fef33-bec9-7280-35a9-d0e4e94fd454@tarantool.org> From: Leonid Vasiliev Message-ID: <7fe8258f-4627-9184-95f0-4f851788cf33@tarantool.org> Date: Wed, 23 Dec 2020 14:09:20 +0300 MIME-Version: 1.0 In-Reply-To: <5a2fef33-bec9-7280-35a9-d0e4e94fd454@tarantool.org> Content-Type: text/plain; charset="utf-8"; format="flowed" Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [Tarantool-patches] [PATCH] test: make strings compatible with Python 3 List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Sergey Bronnikov , tarantool-patches@dev.tarantool.org, v.shpilevoy@tarantool.org Hi! LGTM. On 23.12.2020 13:35, Sergey Bronnikov wrote: > Hi, > > > thanks for review > > > On 23.12.2020 12:59, Leonid Vasiliev wrote: >> Hi! Thank you for the patch. >> Generally I'm comfortable with the patch. But I have a few questions: >> >> On 22.12.2020 11:15, sergeyb@tarantool.org wrote: >>> From: Sergey Bronnikov >>> >>> The largest change in Python 3 is the handling of strings. >>> In Python 2, the str type was used for two different >>> kinds of values – text and bytes, whereas in Python 3, >>> these are separate and incompatible types. >>> Patch converts strings to byte strings where it is required >>> to make tests compatible with Python 3. >>> >>> Part of #5538 >>> --- >>>   test/box-py/bad_trigger.test.py      |  2 +- >>>   test/box-py/iproto.test.py           | 42 +++++++++++++++++----------- >>>   test/replication-py/conflict.test.py |  5 ++-- >>>   3 files changed, 29 insertions(+), 20 deletions(-) >>> >>> diff --git a/test/box-py/bad_trigger.test.py >>> b/test/box-py/bad_trigger.test.py >>> index 9dd6e17c9..03b7efcda 100644 >>> --- a/test/box-py/bad_trigger.test.py >>> +++ b/test/box-py/bad_trigger.test.py >>> @@ -41,7 +41,7 @@ unpacker.feed(packet) >>>   header = unpacker.unpack() >>>   body = unpacker.unpack() >>>   print("error code {}".format((header[IPROTO_CODE] & >>> (REQUEST_TYPE_ERROR - 1)))) >>> -print("error message:  {}".format(body[IPROTO_ERROR])) >>> +print("error message: {}".format(body[IPROTO_ERROR].decode("utf-8"))) >>>   print("eof: {}".format(len(s.recv(1024)) == 0)) >>>   s.close() >>>   diff --git a/test/box-py/iproto.test.py b/test/box-py/iproto.test.py >>> index 6723e03f5..ff0469f10 100644 >>> --- a/test/box-py/iproto.test.py >>> +++ b/test/box-py/iproto.test.py >>> @@ -1,5 +1,6 @@ >>>   from __future__ import print_function >>>   +import binascii >> >> Where is the module used? > > Left after another implementation, removed and force-pushed. > > >> >>>   import os >>>   import sys >>>   import struct >>> @@ -163,7 +164,7 @@ class RawInsert(Request): >>>         def __init__(self, conn, space_no, blob): >>>           super(RawInsert, self).__init__(conn) >>> -        request_body = "\x82" + msgpack.dumps(IPROTO_SPACE_ID) + \ >>> +        request_body = b'\x82' + msgpack.dumps(IPROTO_SPACE_ID) + \ >>>               msgpack.dumps(space_id) + msgpack.dumps(IPROTO_TUPLE) + >>> blob >>>           self._body = request_body >>>   @@ -172,7 +173,7 @@ class RawSelect(Request): >>>         def __init__(self, conn, space_no, blob): >>>           super(RawSelect, self).__init__(conn) >>> -        request_body = "\x83" + msgpack.dumps(IPROTO_SPACE_ID) + \ >>> +        request_body = b'\x83' + msgpack.dumps(IPROTO_SPACE_ID) + \ >>>               msgpack.dumps(space_id) + msgpack.dumps(IPROTO_KEY) + >>> blob + \ >>>               msgpack.dumps(IPROTO_LIMIT) + msgpack.dumps(100); >>>           self._body = request_body >>> @@ -182,13 +183,13 @@ space = c.space("test") >>>   space_id = space.space_no >>>     TESTS = [ >>> -    (1,     "\xa1", "\xd9\x01", "\xda\x00\x01", >>> "\xdb\x00\x00\x00\x01"), >>> -    (31,    "\xbf", "\xd9\x1f", "\xda\x00\x1f", >>> "\xdb\x00\x00\x00\x1f"), >>> -    (32,    "\xd9\x20", "\xda\x00\x20", "\xdb\x00\x00\x00\x20"), >>> -    (255,   "\xd9\xff", "\xda\x00\xff", "\xdb\x00\x00\x00\xff"), >>> -    (256,   "\xda\x01\x00", "\xdb\x00\x00\x01\x00"), >>> -    (65535, "\xda\xff\xff", "\xdb\x00\x00\xff\xff"), >>> -    (65536, "\xdb\x00\x01\x00\x00"), >>> +    (1,     b'\xa1', b'\xd9\x01', b'\xda\x00\x01', >>> b'\xdb\x00\x00\x00\x01'), >>> +    (31,    b'\xbf', b'\xd9\x1f', b'\xda\x00\x1f', >>> b'\xdb\x00\x00\x00\x1f'), >>> +    (32,    b'\xd9\x20', b'\xda\x00\x20', b'\xdb\x00\x00\x00\x20'), >>> +    (255,   b'\xd9\xff', b'\xda\x00\xff', b'\xdb\x00\x00\x00\xff'), >>> +    (256,   b'\xda\x01\x00', b'\xdb\x00\x00\x01\x00'), >>> +    (65535, b'\xda\xff\xff', b'\xdb\x00\x00\xff\xff'), >>> +    (65536, b'\xdb\x00\x01\x00\x00'), >> >> Why do you choose single quotes for byte strings and double quotes for >> strings? Maybe we will use double quotes for all strings? (applies to >> the whole patch) > > I use double quotes for strings and single quotes for bytes literals, > see [1]. > > 1. https://www.python.org/dev/peps/pep-3112/ Ok. > >> >>>   ] >>>     for test in TESTS: >>> @@ -197,16 +198,19 @@ for test in TESTS: >>>       print("STR", size) >>>       print("--") >>>       for fmt in it: >>> -        print("0x" + fmt.encode("hex"), "=>", end=" ") >>> +        if sys.version[0] == '2': >>> +            print("0x" + fmt.encode("hex"), "=>", end=" ") >>> +        else: >>> +            print("0x" + fmt.hex(), "=>", end=" ") >>>           field = "*" * size >>> -        c._send_request(RawInsert(c, space_id, "\x91" + fmt + field)) >>> +        c._send_request(RawInsert(c, space_id, b'\x91' + fmt + >>> field.encode("utf-8"))) >>>           tuple = space.select(field)[0] >>>           print(len(tuple[0])== size and "ok" or "fail", end=" ") >>>           it2 = iter(test) >>>           next(it2) >>>           for fmt2 in it2: >>>               tuple = c._send_request(RawSelect(c, space_id, >>> -                "\x91" + fmt2 + field))[0] >>> +                b'\x91' + fmt2 + field.encode("utf-8")))[0] >>>               print(len(tuple[0]) == size and "ok" or "fail", end=" ") >>>           tuple = space.delete(field)[0] >>>           print(len(tuple[0]) == size and "ok" or "fail", end="") >>> @@ -357,15 +361,18 @@ s = c._socket >>>   header = { "hello": "world"} >>>   body = { "bug": 272 } >>>   resp = test_request(header, body) >>> -print("sync={}, {}".format(resp["header"][IPROTO_SYNC], >>> resp["body"].get(IPROTO_ERROR))) >>> +print("sync={}, {}".format(resp["header"][IPROTO_SYNC], >>> +        resp["body"].get(IPROTO_ERROR).decode("utf-8"))) >>>   header = { IPROTO_CODE : REQUEST_TYPE_SELECT } >>>   header[IPROTO_SYNC] = 1234 >>>   resp = test_request(header, body) >>> -print("sync={}, {}".format(resp["header"][IPROTO_SYNC], >>> resp["body"].get(IPROTO_ERROR))) >>> +print("sync={}, {}".format(resp["header"][IPROTO_SYNC], >>> +        resp["body"].get(IPROTO_ERROR).decode("utf-8"))) >>>   header[IPROTO_SYNC] = 5678 >>>   body = { IPROTO_SPACE_ID: 304, IPROTO_KEY: [], IPROTO_LIMIT: 1 } >>>   resp = test_request(header, body) >>> -print("sync={}, {}".format(resp["header"][IPROTO_SYNC], >>> resp["body"].get(IPROTO_ERROR))) >>> +print("sync={}, {}".format(resp["header"][IPROTO_SYNC], >>> +        resp["body"].get(IPROTO_ERROR).decode("utf-8"))) >>>   c.close() >>>     @@ -424,7 +431,10 @@ header = { IPROTO_CODE: REQUEST_TYPE_CALL, >>> IPROTO_SYNC: 100 } >>>   body = { IPROTO_FUNCTION_NAME: "kek" } >>>   resp = test_request(header, body) >>>   print("Sync: ", resp["header"][IPROTO_SYNC]) >>> -print("Retcode: ", resp["body"][IPROTO_DATA]) >>> +body = resp["body"][IPROTO_DATA] >>> +if sys.version[0] == '3': >>> +    body = [body[0].decode("utf-8")] >>> +print("Retcode: ", body) >>>     c.close() >>>   diff --git a/test/replication-py/conflict.test.py >>> b/test/replication-py/conflict.test.py >>> index 5e19d0c40..925ae046b 100644 >>> --- a/test/replication-py/conflict.test.py >>> +++ b/test/replication-py/conflict.test.py >>> @@ -20,9 +20,8 @@ replica.deploy() >>>   def parallel_run(cmd1, cmd2, compare): >>>       print("parallel send: {}".format(cmd1)) >>>       print("parallel send: {}".format(cmd2)) >>> -    master.admin.socket.sendall("{}\n".format(cmd1)) >>> -    replica.admin.socket.sendall("{}\n".format(cmd2)) >>> - >>> +    master.admin.socket.sendall(cmd1.encode("utf-8") + b'\n') >>> +    replica.admin.socket.sendall(cmd2.encode("utf-8") + b'\n') >>>       master.admin.socket.recv(2048) >>>       replica.admin.socket.recv(2048) >>>