Re: [Libguestfs] [PATCH nbdkit v2 10/10] tests: Test the Python plugin thoroughly.

On Fri, Nov 22, 2019 at 9:55 PM Richard W.M. Jones <rjones(a)redhat.com&gt; wrote: > > This tests the Python plugin thoroughly by issuing client commands > through libnbd and checking we get the expected results. > --- > tests/Makefile.am | 13 +-- > tests/test-python-plugin.py | 134 ++++++++++++++++++++++++++++ > tests/test-python.py | 172 ++++++++++++++++++++++++++++++++++++ > tests/test.py | 60 ------------- > 4 files changed, 309 insertions(+), 70 deletions(-) > > diff --git a/tests/Makefile.am b/tests/Makefile.am > index 13cd8f3..88849f5 100644 > --- a/tests/Makefile.am > +++ b/tests/Makefile.am > @@ -115,7 +115,8 @@ EXTRA_DIST = \ > test-pattern-largest-for-qemu.sh \ > test-python-exception.sh \ > test.pl \ > - test.py \ > + test-python.py \ > + test-python-plugin.py \ > test-rate.sh \ > test-rate-dynamic.sh \ > test.rb \ > @@ -787,18 +788,10 @@ endif HAVE_PERL > if HAVE_PYTHON > > TESTS += \ > + test-python.py \ > test-python-exception.sh \ > test-shebang-python.sh \ > $(NULL) > -LIBGUESTFS_TESTS += test-python > - > -test_python_SOURCES = test-lang-plugins.c test.h > -test_python_CFLAGS = \ > - -DLANG='"python"' -DSCRIPT='"$(srcdir)/test.py"' \ > - $(WARNINGS_CFLAGS) \ > - $(LIBGUESTFS_CFLAGS) \ > - $(NULL) > -test_python_LDADD = libtest.la $(LIBGUESTFS_LIBS) > > endif HAVE_PYTHON > > diff --git a/tests/test-python-plugin.py b/tests/test-python-plugin.py > new file mode 100644 > index 0000000..053a380 > --- /dev/null > +++ b/tests/test-python-plugin.py > @@ -0,0 +1,134 @@ > +#!/usr/bin/env python3 > +# nbdkit > +# Copyright (C) 2019 Red Hat Inc. > +# > +# Redistribution and use in source and binary forms, with or without > +# modification, are permitted provided that the following conditions are > +# met: > +# > +# * Redistributions of source code must retain the above copyright > +# notice, this list of conditions and the following disclaimer. > +# > +# * Redistributions in binary form must reproduce the above copyright > +# notice, this list of conditions and the following disclaimer in the > +# documentation and/or other materials provided with the distribution. > +# > +# * Neither the name of Red Hat nor the names of its contributors may be > +# used to endorse or promote products derived from this software without > +# specific prior written permission. > +# > +# THIS SOFTWARE IS PROVIDED BY RED HAT AND CONTRIBUTORS ''AS IS'' AND > +# ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, > +# THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A > +# PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL RED HAT OR > +# CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, > +# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT > +# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF > +# USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND > +# ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, > +# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT > +# OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF > +# SUCH DAMAGE. > + > +# See test-python.py. > + > +import nbdkit > +import sys > +import pickle > +import codecs > + > +def api_version (): > + return 2 > + > +cfg = {} > + > +def config (k, v): > + global cfg > + if k == "cfg": > + cfg = pickle.loads (codecs.decode (v.encode(), "base64")) > + > +def config_complete (): > + print ("set_error = %r" % nbdkit.set_error) > + > +def open (readonly): > + return { > + 'disk': bytearray (cfg.get ('size', 0)) > + } > + > +def get_size (h): > + return len (h['disk']) > + > +def is_rotational(h): > + return cfg.get ('is_rotational', False) > + > +def can_multi_conn (h): > + return cfg.get ('can_multi_conn', False) > + > +def can_write (h): > + return cfg.get ('can_write', True) > + > +def can_flush(h): > + return cfg.get ('can_flush', False) > + > +def can_trim(h): > + return cfg.get ('can_trim', False) > + > +def can_zero(h): > + return cfg.get ('can_zero', False) > + > +def can_fast_zero(h): > + return cfg.get ('can_fast_zero', False) > + > +def can_fua(h): > + fua = cfg.get ('can_fua', "none") > + if fua == "none": > + return nbdkit.FUA_NONE > + elif fua == "emulate": > + return nbdkit.FUA_EMULATE > + elif fua == "native": > + return nbdkit.FUA_NATIVE > + > +def can_cache(h): > + cache = cfg.get ('can_cache', "none") > + if cache == "none": > + return nbdkit.CACHE_NONE > + elif cache == "emulate": > + return nbdkit.CACHE_EMULATE > + elif cache == "native": > + return nbdkit.CACHE_NATIVE > + > +def pread(h, count, offset, flags): > + assert flags == 0 > + return h['disk'][offset:offset+count] Very nice and simple test plugin! But this returns always a bytearray, which is also what nbdkit python plugin expects. But real code using HTTPConnection return bytes: >>> c = http.client.HTTPSConnection("www.python.org") >>> c.request("GET", "/") >>> r = c.getresponse() >>> r.read()[:10] b'<!doctype ' I think the plugin should support both bytearray, memoryview, or bytes. Supporting objects implementing the buffer protocol would be best.

> + > +def pwrite(h, buf, offset, flags): > + expect_fua = cfg.get ('pwrite_expect_fua', False) > + actual_fua = bool (flags & nbdkit.FLAG_FUA) > + assert expect_fua == actual_fua > + end = offset + len(buf) > + h['disk'][offset:end] = buf > + > +def flush(h, flags): > + assert flags == 0 > + > +def trim(h, count, offset, flags): > + expect_fua = cfg.get ('trim_expect_fua', False) > + actual_fua = bool (flags & nbdkit.FLAG_FUA) > + assert expect_fua == actual_fua > + h['disk'][offset:offset+count] = bytearray(count) > + > +def zero(h, count, offset, flags): > + expect_fua = cfg.get ('zero_expect_fua', False) > + actual_fua = bool (flags & nbdkit.FLAG_FUA) > + assert expect_fua == actual_fua > + expect_may_trim = cfg.get ('zero_expect_may_trim', False) > + actual_may_trim = bool (flags & nbdkit.FLAG_MAY_TRIM) > + assert expect_may_trim == actual_may_trim > + expect_fast_zero = cfg.get ('zero_expect_fast_zero', False) > + actual_fast_zero = bool (flags & nbdkit.FLAG_FAST_ZERO) > + assert expect_fast_zero == actual_fast_zero > + h['disk'][offset:offset+count] = bytearray(count) > + > +def cache(h, count, offset, flags): > + assert flags == 0 > + # do nothing > diff --git a/tests/test-python.py b/tests/test-python.py > new file mode 100755 > index 0000000..2f565ba > --- /dev/null > +++ b/tests/test-python.py > @@ -0,0 +1,172 @@ > +#!/usr/bin/env python3 > +# nbdkit > +# Copyright (C) 2019 Red Hat Inc. > +# > +# Redistribution and use in source and binary forms, with or without > +# modification, are permitted provided that the following conditions are > +# met: > +# > +# * Redistributions of source code must retain the above copyright > +# notice, this list of conditions and the following disclaimer. > +# > +# * Redistributions in binary form must reproduce the above copyright > +# notice, this list of conditions and the following disclaimer in the > +# documentation and/or other materials provided with the distribution. > +# > +# * Neither the name of Red Hat nor the names of its contributors may be > +# used to endorse or promote products derived from this software without > +# specific prior written permission. > +# > +# THIS SOFTWARE IS PROVIDED BY RED HAT AND CONTRIBUTORS ''AS IS'' AND > +# ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, > +# THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A > +# PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL RED HAT OR > +# CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, > +# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT > +# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF > +# USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND > +# ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, > +# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT > +# OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF > +# SUCH DAMAGE. > + > +# This tests the Python plugin thoroughly by issuing client commands > +# through libnbd and checking we get the expected results. It uses an > +# associated plugin (test-python-plugin.sh). test-python-plugin.py This text can use python docstring: """ This tests ... """ > + > +import os > +import sys > +import pickle > +import codecs > + > +# Python has proven very difficult to valgrind, therefore it is disabled. > +if "NBDKIT_VALGRIND" in os.environ: > + print ("$s: skipping Python test under valgrind" % __file__) > + sys.exit (77) > + > +try: > + import nbd > +except: except ImportError: > + print ("%s: skipped because cannot import python libnbd" % __file__) > + sys.exit (77) > + > +try: > + srcdir = os.environ['SRCDIR'] > +except: except KeyError: > + print ("%s: FAIL: test failed because $SRCDIR is not set" % __file__) > + sys.exit (1) > + > +def test (cfg): test will fool testing frameworks, expecting that this is a test function. Maybe call this handle()? or case()? > + h = nbd.NBD () > + cfg = codecs.encode (pickle.dumps (cfg), "base64").decode() base64.b64encode() is better, avoiding unwanted newlines. > + cmd = ["nbdkit", "-v", "-s", "--exit-with-parent", > + "python", srcdir + "/test-python-plugin.py", > + "cfg=" + cfg] > + h.connect_command (cmd) > + return h > + > +# Test we can send an empty pickled test configuration and do nothing > +# else. This is just to ensure the machinery of the test works. > +h = test ({}) So we have now running nbdkit that will exit the python collects when h is implicitly closed when creating a new handle? This is fragile, but can be solved with the help of a testing framework. > + > +# Test the size. > +h = test ({"size": 512}) > +assert h.get_size() == 512 > +h = test ({"size": 1024*1024}) > +assert h.get_size() == 1024*1024 These tests will fail when on the first error, which is less useful. With very little work we can convert this to pytest: def test_size(): h = test() assert h.get_size() == 512 To run the test you use: pytest test-python.py > + > +# Test each flag call. > +h = test ({"size": 512, "is_rotational": True}) > +assert h.is_rotational() > +h = test ({"size": 512, "is_rotational": False}) > +assert not h.is_rotational() When this fails, you will get unhelpful output: assert not True But with pytest you get much more useful error, something like: def test_is_rotational(): h = handle ({"size": 512, "is_rotational": True}) > assert not h.is_rotational E assert not True E + where True = <test.handle.<locals>.H object at 0x7faa3358e450>.is_rotational (I faked the handle object for this example) > + > +h = test ({"size": 512, "can_multi_conn": True}) > +assert h.can_multi_conn() > +h = test ({"size": 512, "can_multi_conn": False}) > +assert not h.can_multi_conn() > + > +h = test ({"size": 512, "can_write": True}) > +assert not h.is_read_only() > +h = test ({"size": 512, "can_write": False}) > +assert h.is_read_only() > + > +h = test ({"size": 512, "can_flush": True}) > +assert h.can_flush() > +h = test ({"size": 512, "can_flush": False}) > +assert not h.can_flush() > + > +h = test ({"size": 512, "can_trim": True}) > +assert h.can_trim() > +h = test ({"size": 512, "can_trim": False}) > +assert not h.can_trim() > + > +# nbdkit can always zero because it emulates it. > +#h = test ({"size": 512, "can_zero": True}) > +#assert h.can_zero() > +#h = test ({"size": 512, "can_zero": False}) > +#assert not h.can_zero() > + > +h = test ({"size": 512, "can_fast_zero": True}) > +assert h.can_fast_zero() > +h = test ({"size": 512, "can_fast_zero": False}) > +assert not h.can_fast_zero() > + > +h = test ({"size": 512, "can_fua": "none"}) > +assert not h.can_fua() > +h = test ({"size": 512, "can_fua": "emulate"}) > +assert h.can_fua() > +h = test ({"size": 512, "can_fua": "native"}) > +assert h.can_fua() > + > +h = test ({"size": 512, "can_cache": "none"}) > +assert not h.can_cache() > +h = test ({"size": 512, "can_cache": "emulate"}) > +assert h.can_cache() > +h = test ({"size": 512, "can_cache": "native"}) > +assert h.can_cache() > + > +# Not yet implemented: can_extents. > + > +# Test pread. > +h = test ({"size": 512}) > +buf = h.pread (512, 0) > + > +# Test pwrite + flags. > +h = test ({"size": 512}) > +h.pwrite (buf, 0) > + > +h = test ({"size": 512, "can_fua": "native", "pwrite_expect_fua": True}) > +h.pwrite (buf, 0, nbd.CMD_FLAG_FUA) > + > +# Test flush. > +h = test ({"size": 512, "can_flush": True}) > +h.flush () > + > +# Test trim + flags. > +h = test ({"size": 512, "can_trim": True}) > +h.trim (512, 0) > + > +h = test ({"size": 512, > + "can_trim": True, "can_fua": "native", "trim_expect_fua": True}) This becomes messy. With the dict does not fit in one line, or contains more than few keys it is more readable to use one key: value pair per line. > +h.trim (512, 0, nbd.CMD_FLAG_FUA) > + > +# Test zero + flags. > +h = test ({"size": 512, "can_zero": True}) > +h.zero (512, 0, nbd.CMD_FLAG_NO_HOLE) > + > +h = test ({"size": 512, > + "can_zero": True, "can_fua": "native", "zero_expect_fua": True}) > +h.zero (512, 0, nbd.CMD_FLAG_NO_HOLE | nbd.CMD_FLAG_FUA) > + > +h = test ({"size": 512, "can_zero": True, "zero_expect_may_trim": True}) > +h.zero (512, 0, 0) # absence of nbd.CMD_FLAG_NO_HOLE > + > +h = test ({"size": 512, > + "can_zero": True, "can_fast_zero": True, > + "zero_expect_fast_zero": True}) > +h.zero (512, 0, nbd.CMD_FLAG_NO_HOLE | nbd.CMD_FLAG_FAST_ZERO) > + > +# Test cache. > +h = test ({"size": 512, "can_cache": "native"}) > +h.cache (512, 0) > diff --git a/tests/test.py b/tests/test.py > deleted file mode 100644 > index ac80d96..0000000 > --- a/tests/test.py > +++ /dev/null > @@ -1,60 +0,0 @@ > -import nbdkit > - > -disk = bytearray(1024*1024) > - > - > -def api_version(): > - return 2 > - > - > -def config_complete(): > - print ("set_error = %r" % nbdkit.set_error) > - > - > -def open(readonly): > - return 1 > - > - > -def get_size(h): > - global disk > - return len(disk) > - > - > -def can_write(h): > - return True > - > - > -def can_flush(h): > - return True > - > - > -def is_rotational(h): > - return False > - > - > -def can_trim(h): > - return True > - > - > -def pread(h, count, offset, flags): > - global disk > - return disk[offset:offset+count] > - > - > -def pwrite(h, buf, offset, flags): > - global disk > - end = offset + len(buf) > - disk[offset:end] = buf > - > - > -def flush(h, flags): > - pass > - > - > -def trim(h, count, offset, flags): > - pass > - > - > -def zero(h, count, offset, flags): > - global disk > - disk[offset:offset+count] = bytearray(count) > -- > 2.23.0 > Otherwise very nice tests! Nir