[PATCH 0/5] Test improvemnts for python 3

[Nir Soffer]

These patches improve tests infrastructure for testing strings and bytes
support, and add missing tests for argument parsing.

If nobody objects, I will push these patches later this week.

The tests pass on all python versions:
https://travis-ci.org/nirs/sanlock/builds/538240633

Amit Bawer (5):
  tests: Add pytest.fixture of no_sanlock_daemon
  tests: Add test calls for sanlock.get_lockspaces()
  python: Generalize set_value_error as set_error utility function
  tests: Add test for killpath(...)
  tests: Add tests for parsing lockspace and resource names

 python/sanlock.c     |  10 ++--
 tests/conftest.py    |   9 ++++
 tests/python_test.py | 112 ++++++++++++++++++++++++++++++++++++++++++-
 tests/util.py        |  17 +++++++
 4 files changed, 142 insertions(+), 6 deletions(-)

-- 
2.17.2

[PATCH 1/5] tests: Add pytest.fixture of no_sanlock_daemon

[Nir Soffer]

From: Amit Bawer <abawer(a)redhat.com&gt;

New fixture calls util.sanlock_is_running() to check if the
pid showing in sanlock.pid file (if file exists) leads to local
sanlock executable file to conclude if sanlock is currenlty
running in test env. In case sanlock is running, a SunlockIsRunning
exception will be raised from test.
---
 tests/conftest.py |  9 +++++++++
 tests/util.py     | 17 +++++++++++++++++
 2 files changed, 26 insertions(+)

diff --git a/tests/conftest.py b/tests/conftest.py
index 93201bb..70e83d4 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -42,5 +42,14 @@ def user_4k_path(request):
     if not os.path.exists(request.param):
         pytest.skip(
             "user storage available - run 'python tests/strorage.py setup'
"
             "to enable 4k storage tests")
     return request.param
+
+
+(a)pytest.fixture
+def no_sanlock_daemon():
+    if util.sanlock_is_running():
+        raise SanlockIsRunning
+
+class SanlockIsRunning(Exception):
+    pass
diff --git a/tests/util.py b/tests/util.py
index d944611..08db236 100644
--- a/tests/util.py
+++ b/tests/util.py
@@ -158,5 +158,22 @@ def generate_path(dirname, filename, encoding=None):
     """
     path = os.path.join(str(dirname), filename)
     if encoding:
         path = path.encode(encoding)
     return path
+
+
+def sanlock_is_running():
+    """
+    Return boolean value indicating whether sanlock process
+    is currently executing within pytests run dir.
+    """
+    pid_file = os.path.join(os.environ["SANLOCK_RUN_DIR"],
"sanlock.pid")
+    try:
+        with open(pid_file, "r") as f:
+            pid = f.readline().strip()
+            executable = os.readlink("/proc/{}/exe".format(pid))
+            return os.path.samefile(executable, SANLOCK)
+    except OSError as e:
+        if e.errno != errno.ENOENT:
+            raise
+    return False
-- 
2.17.2

[PATCH 2/5] tests: Add test calls for sanlock.get_lockspaces()

[Nir Soffer]

From: Amit Bawer <abawer(a)redhat.com&gt;

We expect to get acquired/added lockspaces details on list.

Applies to both Py2 and Py3.
---
 tests/python_test.py | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/tests/python_test.py b/tests/python_test.py
index cfbdf9c..2d50537 100644
--- a/tests/python_test.py
+++ b/tests/python_test.py
@@ -303,17 +303,29 @@ def test_add_rem_lockspace(tmpdir, sanlock_daemon, size, offset):
     # Once the lockspace is acquired, we exepect to get True.
     acquired = sanlock.inq_lockspace(
         "ls_name", 1, path, offset=offset, wait=False)
     assert acquired is True
 
+    lockspaces = sanlock.get_lockspaces()
+    assert lockspaces == [{
+        'flags': 0,
+        'host_id': 1,
+        'lockspace': b'ls_name',
+        'offset': offset,
+        'path': path
+    }]
+
     sanlock.rem_lockspace("ls_name", 1, path, offset=offset)
 
     # Once the lockspace is released, we exepect to get False.
     acquired = sanlock.inq_lockspace(
         "ls_name", 1, path, offset=offset, wait=False)
     assert acquired is False
 
+    lockspaces = sanlock.get_lockspaces()
+    assert lockspaces == []
+
 
 def test_add_rem_lockspace_async(tmpdir, sanlock_daemon):
     path = str(tmpdir.join("ls_name"))
     util.create_file(path, MiB)
 
-- 
2.17.2

[PATCH 3/5] python: Generalize set_value_error as set_error utility function

[Nir Soffer]

From: Amit Bawer <abawer(a)redhat.com&gt;

Allow to pass it PyExc_XXX object as well for raising different
exception types along with message and tested object for printout.
---
 python/sanlock.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/python/sanlock.c b/python/sanlock.c
index 5d228e8..cec5c9b 100644
--- a/python/sanlock.c
+++ b/python/sanlock.c
@@ -33,11 +33,11 @@
 #define MODULE_NAME "sanlock"
 
 /* Functions prototypes */
 static void __set_exception(int en, char *msg) __sets_exception;
 static int __parse_resource(PyObject *obj, struct sanlk_resource **res_ret)
__neg_sets_exception;
-static void set_value_error(const char* format, PyObject* obj);
+static void set_error(PyObject *exception, const char* format, PyObject* obj);
 
 /* Sanlock module */
 PyDoc_STRVAR(pydoc_sanlock, "\
 Copyright (C) 2010-2011 Red Hat, Inc.  All rights reserved.\n\
 This copyrighted material is made available to anyone wishing to use,\n\
@@ -129,18 +129,18 @@ __parse_resource(PyObject *obj, struct sanlk_resource **res_ret)
         uint64_t offset;
 
         disk = PyList_GetItem(obj, i);
 
         if (!PyTuple_Check(disk)) {
-            set_value_error("Invalid disk %s", disk);
+            set_error(PyExc_ValueError, "Invalid disk %s", disk);
             goto exit_fail;
 
         }
 
         if (!PyArg_ParseTuple(disk, "sK", &path, &offset)) {
             /* Override the error since it confusing in this context. */
-            set_value_error("Invalid disk %s", disk);
+            set_error(PyExc_ValueError, "Invalid disk %s", disk);
             goto exit_fail;
         }
 
         strncpy(res->disks[i].path, path, SANLK_PATH_LEN - 1);
         res->disks[i].offset = offset;
@@ -202,17 +202,17 @@ add_align_flag(long align, uint32_t *flags)
     }
     return 0;
 }
 
 static void
-set_value_error(const char* format, PyObject* obj)
+set_error(PyObject* exception, const char* format, PyObject* obj)
 {
     const char* str_rep = "";
     PyObject* rep = PyObject_Repr(obj);
     if (rep)
         str_rep = pystring_as_cstring(rep);
-    PyErr_Format(PyExc_ValueError, format, str_rep);
+    PyErr_Format(exception, format, str_rep);
     Py_XDECREF(rep);
 }
 
 static PyObject *
 __hosts_to_list(struct sanlk_host *hss, int hss_count)
-- 
2.17.2

[PATCH 4/5] tests: Add test for killpath(...)

[Nir Soffer]

From: Amit Bawer <abawer(a)redhat.com&gt;

Current status:
- Py2: Accept command paths in bytes or ascii strings.
- Py3: Accept command paths in unicode/ascii strings only.
---
 tests/python_test.py | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/tests/python_test.py b/tests/python_test.py
index 2d50537..6eb027c 100644
--- a/tests/python_test.py
+++ b/tests/python_test.py
@@ -485,5 +485,10 @@ def test_write_resource_invalid_disk(tmpdir, sanlock_daemon, disk):
     disks = [disk]
     with pytest.raises(ValueError) as e:
         sanlock.write_resource("ls_name", "res_name", disks)
     assert repr(disk) in str(e.value)
 
+(a)pytest.mark.parametrize(&quot;filename,encoding&quot;, FILE_NAMES)
+def test_killpath(tmpdir, sanlock_daemon, filename, encoding):
+    cmd_path = util.generate_path(tmpdir, filename, encoding)
+    fd = sanlock.register()
+    sanlock.killpath(cmd_path, [cmd_path], fd)
-- 
2.17.2

[PATCH 5/5] tests: Add tests for parsing lockspace and resource names

[Nir Soffer]

From: Amit Bawer <abawer(a)redhat.com&gt;

We add fast tests for sanlock API stubs only to test their
argument parsing. It is expected that if argument parsing
succeed, the API call itself will fail with errno since those
are only stubs so we add helper to assert on the proper errno.
---
 tests/python_test.py | 95 +++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 94 insertions(+), 1 deletion(-)

diff --git a/tests/python_test.py b/tests/python_test.py
index 6eb027c..db77862 100644
--- a/tests/python_test.py
+++ b/tests/python_test.py
@@ -6,12 +6,14 @@ from __future__ import absolute_import
 import errno
 import io
 import struct
 import time
 
-import six
+from contextlib import contextmanager
+
 import pytest
+import six
 
 import sanlock
 
 from . import constants
 from . import util
@@ -47,10 +49,25 @@ FILE_NAMES = [
         marks=pytest.mark.xfail(
             six.PY3,
             reason="currently not supporting bytes paths")),
 ]
 
+LOCKSPACE_OR_RESOURCE_NAMES = [
+    # Bytes are supported with python 2 and 3.
+    pytest.param(
+        b"\xd7\x90",
+        marks=pytest.mark.xfail(six.PY3, reason="bytes support not implemented
yet")),
+    # Python 2 also supports str.
+    pytest.param(
+        "\xd7\x90",
+        marks=pytest.mark.skipif(six.PY3, reason="python 3 supports only
bytes")),
+    # Python 2 also supports unicode with ascii content.
+    pytest.param(
+        u"ascii",
+        marks=pytest.mark.skipif(six.PY3, reason="python 3 supports only
bytes")),
+]
+
 @pytest.mark.parametrize("filename, encoding" , FILE_NAMES)
 @pytest.mark.parametrize("size,offset", [
     # Smallest offset.
     (LOCKSPACE_SIZE, 0),
     # Large offset.
@@ -485,10 +502,86 @@ def test_write_resource_invalid_disk(tmpdir, sanlock_daemon, disk):
     disks = [disk]
     with pytest.raises(ValueError) as e:
         sanlock.write_resource("ls_name", "res_name", disks)
     assert repr(disk) in str(e.value)
 
+
 @pytest.mark.parametrize("filename,encoding", FILE_NAMES)
 def test_killpath(tmpdir, sanlock_daemon, filename, encoding):
     cmd_path = util.generate_path(tmpdir, filename, encoding)
     fd = sanlock.register()
     sanlock.killpath(cmd_path, [cmd_path], fd)
+
+
+@contextmanager
+def raises_sanlock_errno(expected_errno=errno.ECONNREFUSED):
+    with pytest.raises(sanlock.SanlockException) as e:
+        yield
+    assert e.value.errno == expected_errno
+
+
+(a)pytest.mark.parametrize(&quot;name&quot;, LOCKSPACE_OR_RESOURCE_NAMES)
+def test_rem_lockspace_parse_args(no_sanlock_daemon, name):
+    with raises_sanlock_errno():
+        sanlock.rem_lockspace(name, 1, "ls_path", 0)
+
+
+(a)pytest.mark.parametrize(&quot;name&quot;, LOCKSPACE_OR_RESOURCE_NAMES)
+def test_add_lockspace_parse_args(no_sanlock_daemon, name):
+    with raises_sanlock_errno():
+        sanlock.add_lockspace(name, 1, "ls_path", 0)
+
+
+(a)pytest.mark.parametrize(&quot;name&quot;, LOCKSPACE_OR_RESOURCE_NAMES)
+def test_write_lockspace_parse_args(no_sanlock_daemon, name):
+    with raises_sanlock_errno():
+        sanlock.write_lockspace(name, "ls_path")
+
+
+(a)pytest.mark.parametrize(&quot;name&quot;, LOCKSPACE_OR_RESOURCE_NAMES)
+def test_write_resource_parse_args(no_sanlock_daemon, name):
+    with raises_sanlock_errno():
+        sanlock.write_resource(name, "res_name", [("disk_path",0)])
+
+    with raises_sanlock_errno():
+        sanlock.write_resource("ls_name", name, [("disk_path",0)])
+
+
+(a)pytest.mark.parametrize(&quot;name&quot;, LOCKSPACE_OR_RESOURCE_NAMES)
+def test_release_resource_parse_args(no_sanlock_daemon, name):
+    with raises_sanlock_errno():
+        sanlock.release(name, "res_name", [("disk_path",0)])
+
+    with raises_sanlock_errno():
+        sanlock.release("ls_name", name, [("disk_path",0)])
+
+
+(a)pytest.mark.parametrize(&quot;name&quot;, LOCKSPACE_OR_RESOURCE_NAMES)
+def test_read_resource_owners_parse_args(no_sanlock_daemon, name):
+    with raises_sanlock_errno():
+        sanlock.read_resource_owners(name, "res_name",
[("disk_path",0)])
+
+    with raises_sanlock_errno():
+        sanlock.read_resource_owners("ls_name", name,
[("disk_path",0)])
+
+
+(a)pytest.mark.parametrize(&quot;name&quot;, LOCKSPACE_OR_RESOURCE_NAMES)
+def test_get_hosts_parse_args(no_sanlock_daemon, name):
+    with raises_sanlock_errno():
+        sanlock.get_hosts(name, 1)
+
+
+(a)pytest.mark.parametrize(&quot;name&quot;, LOCKSPACE_OR_RESOURCE_NAMES)
+def test_inq_lockspace_parse_args(no_sanlock_daemon, name):
+    with raises_sanlock_errno():
+        sanlock.inq_lockspace(name, 1, "path", wait=False)
+
+
+(a)pytest.mark.parametrize(&quot;name&quot;, LOCKSPACE_OR_RESOURCE_NAMES)
+def test_reg_event_parse_args(no_sanlock_daemon, name):
+    with raises_sanlock_errno():
+        sanlock.reg_event(name)
+
+(a)pytest.mark.parametrize(&quot;name&quot;, LOCKSPACE_OR_RESOURCE_NAMES)
+def test_end_event_parse_args(no_sanlock_daemon, name):
+    with raises_sanlock_errno(errno.EALREADY):
+        sanlock.end_event(-1, name)
-- 
2.17.2