On Sat, Nov 23, 2019 at 1:42 AM Nir Soffer <[email protected]> wrote: > > On Fri, Nov 22, 2019 at 9:55 PM Richard W.M. Jones <[email protected]> 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.
This patch adds support for buffer protocol: https://www.redhat.com/archives/libguestfs/2019-November/msg00200.html (I forgot to add nbdkit to the title) > > > + > > +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 _______________________________________________ Libguestfs mailing list [email protected] https://www.redhat.com/mailman/listinfo/libguestfs
