On Fri, Jun 14, 2019 at 6:47 PM Vojtech Juranek <vjuranek@redhat.com> wrote:
Add tests for write_resource() with parameter 'clear' set to True, which
shoudl result into clearing the resource (overwritting PAXOS_DISK_MAGIC
with PAXOS_DISK_CLEAR).
---
 tests/constants.py   |  4 +++
 tests/python_test.py | 71 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 75 insertions(+)

diff --git a/tests/constants.py b/tests/constants.py
index a7eb16b..21089b8 100644
--- a/tests/constants.py
+++ b/tests/constants.py
@@ -17,3 +17,7 @@ RINDEX_DISK_MAGIC = 0x01042018

 RINDEX_ENTRY_SIZE = 64
 RINDEX_ENTRIES_SECTORS = 2000
+
+# src/sanlock_rv.h
+
+SANLK_LEADER_MAGIC = -223
diff --git a/tests/python_test.py b/tests/python_test.py
index 3242f57..185c3e1 100644
--- a/tests/python_test.py
+++ b/tests/python_test.py
@@ -234,6 +234,77 @@ def test_write_resource_4k_invalid_sector_size(sanlock_daemon, user_4k_path):
     assert e.value.errno == errno.EINVAL


+@pytest.mark.parametrize("filename,encoding", FILE_NAMES)
+@pytest.mark.parametrize("size,offset", [
+    # Smallest offset.
+    (MIN_RES_SIZE, 0),
+    # Large offset.
+    (LARGE_FILE_SIZE, LARGE_FILE_SIZE - MIN_RES_SIZE),
+])

I don't think we need to test offset and Unicode file names for verifying clear
behavior.

+def test_clear_resource(tmpdir, sanlock_daemon, filename, encoding, size, offset):
+    path = util.generate_path(tmpdir, filename, encoding)

We can use constant value as we use in the higher level flow tests.

 
+    util.create_file(path, size)
+    disks = [(path, offset)]
+
+    # Test write, clear and read with default alignment and sector size values.
+    sanlock.write_resource(b"ls_name", b"res_name", disks)
+    sanlock.write_resource(b"ls_name", b"res_name", disks, clear=True)
+
+    with pytest.raises(sanlock.SanlockException) as e:
+        sanlock.read_resource(path, offset=offset)
+    assert e.value.errno == constants.SANLK_LEADER_MAGIC

This is enough to verify the behavior of clearing a resource.

 
+
+    # Test write, clear and read with explicit alignment and sector size
+    # values.
+    sanlock.write_resource(
+        b"ls_name", b"res_name", disks, align=ALIGNMENT_1M,
+        sector=SECTOR_SIZE_512, clear=True)
+
+    with pytest.raises(sanlock.SanlockException) as e:
+        sanlock.read_resource(
+            path, offset=offset, align=ALIGNMENT_1M, sector=SECTOR_SIZE_512)
+    assert e.value.errno == constants.SANLK_LEADER_MAGIC

I don't think we need to verify align and sector for explicit argument, this was
already verified in other tests.
 
+
+    with io.open(path, "rb") as f:
+        f.seek(offset)
+        magic, = struct.unpack("< I", f.read(4))
+        assert magic == constants.PAXOS_DISK_CLEAR
+
+    util.check_guard(path, size)

This looks good, but we repeat this too many times. Would be good idea to add  a helper
to read magic from some offset.

What is missing here is the case of clearing a non existing resource - we want to know what
is sanlock behavior in this case. We can add this test here or add a new test for this.

+
+
+@pytest.mark.parametrize("align", sanlock.ALIGN_SIZE)
+def test_clear_resource_4k(sanlock_daemon, user_4k_path, align):
+    disks = [(user_4k_path, 0)]
+
+    # Poison resource area, ensuring that previous tests will not break this
+    # test, and sanlock does not write beyond the lockspace area.
+    with io.open(user_4k_path, "rb+") as f:
+        f.write(align * b"x")
+    util.write_guard(user_4k_path, align)
+
+    sanlock.write_resource(
+        b"ls_name", b"res_name", disks, align=align, sector=SECTOR_SIZE_4K)
+    sanlock.write_resource(
+        b"ls_name", b"res_name",
+        disks,
+        align=align,
+        sector=SECTOR_SIZE_4K,
+        clear=True)
+
+    with pytest.raises(sanlock.SanlockException) as e:
+        sanlock.read_resource(user_4k_path, align=align, sector=SECTOR_SIZE_4K)
+    assert e.value.errno == constants.SANLK_LEADER_MAGIC
+
+    # Verify that resource was cleared.
+    with io.open(user_4k_path, "rb") as f:
+        magic, = struct.unpack("< I", f.read(4))
+        assert magic == constants.PAXOS_DISK_CLEAR
+
+    # Check that sanlock did not write beyond the lockspace area.
+    util.check_guard(user_4k_path, align)
+

I'm not sure we need to test clear flag with 4k as we already tested writing resource
with 4k.
 
+
 def test_read_resource_4k_invalid_sector_size(sanlock_daemon, user_4k_path):
     disks = [(user_4k_path, 0)]

--
2.20.1