Re: [Tarantool-patches] [PATCH] test: make strings compatible with Python 3

[Leonid Vasiliev <lvasiliev@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 <sergeyb@tarantool.org>
>>>
>>> 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)
>>>