Re: [PATCH v2 1/2] tests: replace reading of magic number by helper function

[Nir Soffer]

On Sat, Jun 15, 2019 at 12:55 AM Vojtech Juranek <vjuranek(a)redhat.com&gt;
wrote:

>  Introduce helper function for reading sanlock magic numbers. This
>  helper function seeks to specified offset in given file and reads
>  unsigned int from this position.
>  ---
>   tests/python_test.py | 25 +++++++++----------------
>   tests/util.py        | 11 +++++++++++
>   2 files changed, 20 insertions(+), 16 deletions(-)
>
>  diff --git a/tests/python_test.py b/tests/python_test.py
>  index 3242f57..623a13c 100644
>  --- a/tests/python_test.py
>  +++ b/tests/python_test.py
>  @@ -86,12 +86,9 @@ def test_write_lockspace(tmpdir, sanlock_daemon,
>  filename, encoding, size, offse
>           b"ls_name", 1, path, offset=offset, wait=False)
>       assert acquired is False
>
>  -    with io.open(path, "rb") as f:
>  -        f.seek(offset)
>  -        magic, = struct.unpack("< I", f.read(4))
>  -        assert magic == constants.DELTA_DISK_MAGIC
>  -
>  -        # TODO: check more stuff here...
>  +    magic = util.read_uint(path, offset)
>  +    assert magic == constants.DELTA_DISK_MAGIC
>  +    # TODO: check more stuff here...
>
>       util.check_guard(path, size)
>
>  @@ -117,9 +114,8 @@ def test_write_lockspace_4k(user_4k_path,
>  sanlock_daemon, align):
>       assert acquired is False
>
>       # Verify that lockspace was written.
>  -    with io.open(user_4k_path, "rb") as f:
>  -        magic, = struct.unpack("< I", f.read(4))
>  -        assert magic == constants.DELTA_DISK_MAGIC
>  +    magic = util.read_uint(user_4k_path)
>  +    assert magic == constants.DELTA_DISK_MAGIC
>
>       # Check that sanlock did not write beyond the lockspace area.
>       util.check_guard(user_4k_path, align)
>  @@ -179,10 +175,8 @@ def test_write_resource(tmpdir, sanlock_daemon,
>  filename, encoding, size, offset
>       owners = sanlock.read_resource_owners(b"ls_name", b"res_name",
> disks)
>       assert owners == []
>
>  -    with io.open(path, "rb") as f:
>  -        f.seek(offset)
>  -        magic, = struct.unpack("< I", f.read(4))
>  -        assert magic == constants.PAXOS_DISK_MAGIC
>  +    magic = util.read_uint(path, offset)
>  +    assert magic == constants.PAXOS_DISK_MAGIC
>
>           # TODO: check more stuff here...
>
>  @@ -216,9 +210,8 @@ def test_write_resource_4k(sanlock_daemon,
>  user_4k_path, align):
>       assert owners == []
>
>       # Verify that resource was written.
>  -    with io.open(user_4k_path, "rb") as f:
>  -        magic, = struct.unpack("< I", f.read(4))
>  -        assert magic == constants.PAXOS_DISK_MAGIC
>  +    magic = util.read_uint(user_4k_path)
>  +    assert magic == constants.PAXOS_DISK_MAGIC
>
>       # Check that sanlock did not write beyond the lockspace area.
>       util.check_guard(user_4k_path, align)
>  diff --git a/tests/util.py b/tests/util.py
>  index 08db236..32b34ec 100644
>  --- a/tests/util.py
>  +++ b/tests/util.py
>  @@ -177,3 +177,14 @@ def sanlock_is_running():
>           if e.errno != errno.ENOENT:
>               raise
>       return False
>  +
>  +
>  +def read_uint(path, offset=0):
>  +    """
>  +    Read and return unsigned int from specified file at given offset.
>  +    Typical usage is to read sanlock magic number.
>  +    """
>  +    with io.open(path, "rb") as f:
>  +        f.seek(offset)
>  +        uint, = struct.unpack("< I", f.read(4))
>  +        return uint
>  --
>  2.20.1
>
>  
Looks good, but maybe call it read_magic(path, offset)?