Pino Toscano
2020-Jan-10 14:28 UTC
[Libguestfs] [PATCH 0/7] Various Python pycodestyle fixes
Fixes the majority of the pycodestyle issues in the Python bindings, leaving only an overly length line in setup.py. Add a simple optional pycodestyle-based test to avoid regressions in the future. Pino Toscano (7): python: PEP 8: adapt empty lines python: PEP 8: adapt whitespaces in lines python: tests: catch specific exception python: tests: improve variable naming python: tests: remove unused imports python: improve errors in inspect_vm example python: add a pycodestyle test config.sh.in | 1 + generator/python.ml | 5 ++--- m4/guestfs-python.m4 | 3 +++ python/Makefile.am | 5 +++++ python/examples/inspect_vm.py | 7 +++++-- python/t/test020Create.py | 1 + python/t/test030CreateFlags.py | 3 ++- python/t/test040CreateMultiple.py | 2 ++ python/t/test050HandleProperties.py | 1 + python/t/test070OptArgs.py | 1 - python/t/test080Version.py | 2 -- python/t/test090RetValues.py | 10 +++++----- python/t/test100Launch.py | 1 - python/t/test410CloseEvent.py | 1 - python/t/test420LogMessages.py | 1 - python/t/test430ProgressMessages.py | 3 ++- python/t/test800ExplicitClose.py | 1 - python/t/test820RHBZ912499.py | 3 +-- python/t/test910Libvirt.py | 1 - python/t/tests_helper.py.in | 2 +- python/test-pycodestyle.sh | 30 +++++++++++++++++++++++++++++ 21 files changed, 61 insertions(+), 23 deletions(-) create mode 100755 python/test-pycodestyle.sh -- 2.24.1
Pino Toscano
2020-Jan-10 14:28 UTC
[Libguestfs] [PATCH 1/7] python: PEP 8: adapt empty lines
Add or remove empty lines to match the needed ones around blocks/functions/etc. Adapt the generation of guestfs.py to emit the separating empty line before adding a new function/alias, rather than after it. This is just formatting, no behaviour changes. --- generator/python.ml | 5 ++--- python/t/test020Create.py | 1 + python/t/test030CreateFlags.py | 1 + python/t/test040CreateMultiple.py | 2 ++ python/t/test050HandleProperties.py | 1 + python/t/test430ProgressMessages.py | 2 ++ 6 files changed, 9 insertions(+), 3 deletions(-) diff --git a/generator/python.ml b/generator/python.ml index fd297321e..f5e400110 100644 --- a/generator/python.ml +++ b/generator/python.ml @@ -786,7 +786,6 @@ class GuestFS(object): \"\"\"Delete an event callback.\"\"\" self._check_not_closed() libguestfsmod.delete_event_callback(self._o, event_handle) - "; let map_join f l @@ -803,6 +802,7 @@ class GuestFS(object): args ^ map_join (fun optarg -> sprintf ", %s=None" (name_of_optargt optarg)) optargs in + pr "\n"; pr " def %s(%s):\n" f.name (indent_python decl_string (9 + len_name) 78); @@ -900,12 +900,11 @@ class GuestFS(object): ); pr " return r\n"; - pr "\n"; (* Aliases. *) List.iter ( fun alias -> - pr " %s = %s\n\n" alias f.name + pr "\n %s = %s\n" alias f.name ) f.non_c_aliases ) (actions |> external_functions |> sort) diff --git a/python/t/test020Create.py b/python/t/test020Create.py index 16e19ec76..277fa7051 100644 --- a/python/t/test020Create.py +++ b/python/t/test020Create.py @@ -18,6 +18,7 @@ import unittest import guestfs + class Test020Create(unittest.TestCase): def test_create(self): _ = guestfs.GuestFS(python_return_dict=True) diff --git a/python/t/test030CreateFlags.py b/python/t/test030CreateFlags.py index b69e488b3..ab522fab4 100644 --- a/python/t/test030CreateFlags.py +++ b/python/t/test030CreateFlags.py @@ -18,6 +18,7 @@ import unittest import guestfs + class Test030CreateFlags(unittest.TestCase): def test_create_flags(self): g = guestfs.GuestFS(python_return_dict=True, diff --git a/python/t/test040CreateMultiple.py b/python/t/test040CreateMultiple.py index 190ef256b..d73501edc 100644 --- a/python/t/test040CreateMultiple.py +++ b/python/t/test040CreateMultiple.py @@ -18,9 +18,11 @@ import unittest import guestfs + def ignore(_): pass + class Test040CreateMultiple(unittest.TestCase): def test_create_multiple(self): g1 = guestfs.GuestFS(python_return_dict=True) diff --git a/python/t/test050HandleProperties.py b/python/t/test050HandleProperties.py index 59de8b806..131482c8f 100644 --- a/python/t/test050HandleProperties.py +++ b/python/t/test050HandleProperties.py @@ -19,6 +19,7 @@ import unittest import warnings import guestfs + class Test050HandleProperties(unittest.TestCase): def test_verbose(self): g = guestfs.GuestFS(python_return_dict=True) diff --git a/python/t/test430ProgressMessages.py b/python/t/test430ProgressMessages.py index cbc404cd0..3dab4bd1c 100644 --- a/python/t/test430ProgressMessages.py +++ b/python/t/test430ProgressMessages.py @@ -21,10 +21,12 @@ import guestfs callback_invoked = 0 + def callback(ev, eh, buf, array): global callback_invoked callback_invoked += 1 + class Test430ProgressMessages(unittest.TestCase): def test_progress_messages(self): global callback_invoked -- 2.24.1
Pino Toscano
2020-Jan-10 14:28 UTC
[Libguestfs] [PATCH 2/7] python: PEP 8: adapt whitespaces in lines
Fix continuation indentation, and whitespaces around operators. This is just code reformatting, with no behaviour changes; no content changed beside whitespaces, so "git diff -w" gives an empty diff. --- python/t/test030CreateFlags.py | 2 +- python/t/test820RHBZ912499.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/python/t/test030CreateFlags.py b/python/t/test030CreateFlags.py index ab522fab4..bdfffa44e 100644 --- a/python/t/test030CreateFlags.py +++ b/python/t/test030CreateFlags.py @@ -22,5 +22,5 @@ import guestfs class Test030CreateFlags(unittest.TestCase): def test_create_flags(self): g = guestfs.GuestFS(python_return_dict=True, - environment=False) + environment=False) g.parse_environment() diff --git a/python/t/test820RHBZ912499.py b/python/t/test820RHBZ912499.py index f008f0c74..557d750d2 100644 --- a/python/t/test820RHBZ912499.py +++ b/python/t/test820RHBZ912499.py @@ -38,7 +38,7 @@ class Test820RHBZ912499(unittest.TestCase): def setUp(self): # Create a test disk. self.filename = os.getcwd() + "/820-rhbz912499.img" - guestfs.GuestFS().disk_create(self.filename, "raw", 1024*1024*1024) + guestfs.GuestFS().disk_create(self.filename, "raw", 1024 * 1024 * 1024) # Create a new domain. This won't work, it will just hang when # booted. But that's sufficient for the test. -- 2.24.1
Pino Toscano
2020-Jan-10 14:28 UTC
[Libguestfs] [PATCH 3/7] python: tests: catch specific exception
When trying to import libvirt, catch the specific exception that is raised when an importing fails. --- python/t/tests_helper.py.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/t/tests_helper.py.in b/python/t/tests_helper.py.in index 93783430e..38f0dc521 100644 --- a/python/t/tests_helper.py.in +++ b/python/t/tests_helper.py.in @@ -56,7 +56,7 @@ def skipUnlessLibvirtHasCPointer(): """ try: import libvirt - except: + except ImportError: return unittest.skip("could not import libvirt") # Check we're using the version of libvirt-python that has # c_pointer() methods. -- 2.24.1
Pino Toscano
2020-Jan-10 14:28 UTC
[Libguestfs] [PATCH 4/7] python: tests: improve variable naming
Use a slightly more descriptive name, as also pointed out by pycodestyle. No functional changes. --- python/t/test090RetValues.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/python/t/test090RetValues.py b/python/t/test090RetValues.py index 17b57c954..4a6508462 100644 --- a/python/t/test090RetValues.py +++ b/python/t/test090RetValues.py @@ -91,12 +91,12 @@ class Test090PythonRetValues(unittest.TestCase): g = guestfs.GuestFS() self.assertEqual(g.internal_test_rstructlist("0"), []) - l = g.internal_test_rstructlist("5") - self.assertIsInstance(l, list) - self.assertEqual(len(l), 5) + retlist = g.internal_test_rstructlist("5") + self.assertIsInstance(retlist, list) + self.assertEqual(len(retlist), 5) for i in range(0, 5): - self.assertIsInstance(l[i], dict) - self.assertEqual(l[i]["pv_name"], "pv%d" % i) + self.assertIsInstance(retlist[i], dict) + self.assertEqual(retlist[i]["pv_name"], "pv%d" % i) self.assertRaises(RuntimeError, g.internal_test_rstructlisterr) -- 2.24.1
Pino Toscano
2020-Jan-10 14:28 UTC
[Libguestfs] [PATCH 5/7] python: tests: remove unused imports
No functional changes. --- python/t/test070OptArgs.py | 1 - python/t/test080Version.py | 2 -- python/t/test100Launch.py | 1 - python/t/test410CloseEvent.py | 1 - python/t/test420LogMessages.py | 1 - python/t/test430ProgressMessages.py | 1 - python/t/test800ExplicitClose.py | 1 - python/t/test820RHBZ912499.py | 1 - python/t/test910Libvirt.py | 1 - 9 files changed, 10 deletions(-) diff --git a/python/t/test070OptArgs.py b/python/t/test070OptArgs.py index 2f07ab0ee..b0dd40b4d 100644 --- a/python/t/test070OptArgs.py +++ b/python/t/test070OptArgs.py @@ -16,7 +16,6 @@ # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. import unittest -import os import guestfs diff --git a/python/t/test080Version.py b/python/t/test080Version.py index 4fc791ada..f77eea54b 100644 --- a/python/t/test080Version.py +++ b/python/t/test080Version.py @@ -16,8 +16,6 @@ # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. import unittest -import sys -import os import guestfs from .tests_helper import * diff --git a/python/t/test100Launch.py b/python/t/test100Launch.py index 0f6aa6b8d..ae0e582ad 100644 --- a/python/t/test100Launch.py +++ b/python/t/test100Launch.py @@ -16,7 +16,6 @@ # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. import unittest -import os import guestfs diff --git a/python/t/test410CloseEvent.py b/python/t/test410CloseEvent.py index 1a94dd5b3..00cd2e10a 100644 --- a/python/t/test410CloseEvent.py +++ b/python/t/test410CloseEvent.py @@ -16,7 +16,6 @@ # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. import unittest -import os import guestfs close_invoked = 0 diff --git a/python/t/test420LogMessages.py b/python/t/test420LogMessages.py index bec3b9fd4..b25dd43dc 100644 --- a/python/t/test420LogMessages.py +++ b/python/t/test420LogMessages.py @@ -16,7 +16,6 @@ # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. import unittest -import os import guestfs log_invoked = 0 diff --git a/python/t/test430ProgressMessages.py b/python/t/test430ProgressMessages.py index 3dab4bd1c..b081ba564 100644 --- a/python/t/test430ProgressMessages.py +++ b/python/t/test430ProgressMessages.py @@ -16,7 +16,6 @@ # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. import unittest -import os import guestfs callback_invoked = 0 diff --git a/python/t/test800ExplicitClose.py b/python/t/test800ExplicitClose.py index c380e992c..47a1ebc8d 100644 --- a/python/t/test800ExplicitClose.py +++ b/python/t/test800ExplicitClose.py @@ -18,7 +18,6 @@ # Test implicit vs explicit closes of the handle (RHBZ#717786). import unittest -import os import guestfs close_invoked = 0 diff --git a/python/t/test820RHBZ912499.py b/python/t/test820RHBZ912499.py index 557d750d2..3dec11352 100644 --- a/python/t/test820RHBZ912499.py +++ b/python/t/test820RHBZ912499.py @@ -24,7 +24,6 @@ import unittest import random import string import os -import sys import guestfs from .tests_helper import * diff --git a/python/t/test910Libvirt.py b/python/t/test910Libvirt.py index b605b15be..3672a70e7 100644 --- a/python/t/test910Libvirt.py +++ b/python/t/test910Libvirt.py @@ -22,7 +22,6 @@ import unittest import os -import sys import guestfs from .tests_helper import * -- 2.24.1
Pino Toscano
2020-Jan-10 14:28 UTC
[Libguestfs] [PATCH 6/7] python: improve errors in inspect_vm example
When validating user input, print an error message and exit, instead of either asserting or raising a non-existing exception. --- python/examples/inspect_vm.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/python/examples/inspect_vm.py b/python/examples/inspect_vm.py index 22cd70f28..dcf3b538c 100644 --- a/python/examples/inspect_vm.py +++ b/python/examples/inspect_vm.py @@ -3,7 +3,9 @@ import sys import guestfs -assert(len(sys.argv) == 2) +if len(sys.argv) < 2: + print("inspect_vm: missing disk image to inspect", file=sys.stderr) + sys.exit(1) disk = sys.argv[1] # All new Python code should pass python_return_dict=True @@ -21,7 +23,8 @@ g.launch() # Ask libguestfs to inspect for operating systems. roots = g.inspect_os() if len(roots) == 0: - raise(Error("inspect_vm: no operating systems found")) + print("inspect_vm: no operating systems found", file=sys.stderr) + sys.exit(1) for root in roots: print("Root device: %s" % root) -- 2.24.1
Pino Toscano
2020-Jan-10 14:28 UTC
[Libguestfs] [PATCH 7/7] python: add a pycodestyle test
Look for pycodestyle, and use it to check all the Python sources (tests and auxiliary scripts included) of the Python bindings. --- config.sh.in | 1 + m4/guestfs-python.m4 | 3 +++ python/Makefile.am | 5 +++++ python/test-pycodestyle.sh | 30 ++++++++++++++++++++++++++++++ 4 files changed, 39 insertions(+) create mode 100755 python/test-pycodestyle.sh diff --git a/config.sh.in b/config.sh.in index c777f096c..02100f703 100644 --- a/config.sh.in +++ b/config.sh.in @@ -20,3 +20,4 @@ # mostly used in other shell scripts. export XMLLINT="@XMLLINT@" +export PYCODESTYLE="@PYCODESTYLE@" diff --git a/m4/guestfs-python.m4 b/m4/guestfs-python.m4 index ac857915f..505eba5df 100644 --- a/m4/guestfs-python.m4 +++ b/m4/guestfs-python.m4 @@ -86,6 +86,9 @@ AS_IF([test "x$enable_python" != "xno"],[ PYTHON_EXT_SUFFIX=$python_ext_suffix fi AC_MSG_RESULT([$PYTHON_EXT_SUFFIX]) + + AC_CHECK_PROGS([PYCODESTYLE],[pycodestyle],[no]) + AM_CONDITIONAL([HAVE_PYCODESTYLE], [test "x$PYCODESTYLE" != "xno"]) fi AC_SUBST(PYTHON_PREFIX) diff --git a/python/Makefile.am b/python/Makefile.am index f0a4b55bd..7286ac906 100644 --- a/python/Makefile.am +++ b/python/Makefile.am @@ -38,6 +38,7 @@ EXTRA_DIST = \ setup.py.in \ run-bindtests \ run-python-tests \ + test-pycodestyle.sh \ t/__init__.py \ t/README \ t/test[0-9]*.py @@ -117,6 +118,10 @@ if ENABLE_APPLIANCE TESTS += run-python-tests endif ENABLE_APPLIANCE +if HAVE_PYCODESTYLE +TESTS += test-pycodestyle.sh +endif + endif HAVE_PYTHON # Extra clean. diff --git a/python/test-pycodestyle.sh b/python/test-pycodestyle.sh new file mode 100755 index 000000000..2a5bf3fac --- /dev/null +++ b/python/test-pycodestyle.sh @@ -0,0 +1,30 @@ +#!/bin/bash - +# libguestfs +# Copyright (C) 2020 Red Hat Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + +set -e + +$TEST_FUNCTIONS +skip_if_skipped + +# Gather the list of Python sources. +# (-u is passed to to sort to avoid duplicates in case builddir==srcdir) +files="$(find "$srcdir" . -name '*.py' | sort -u)" + +# Ignore: +# E501 line too long (95 > 79 characters) +$PYCODESTYLE --ignore=E501 $files -- 2.24.1
Richard W.M. Jones
2020-Jan-14 09:56 UTC
Re: [Libguestfs] [PATCH 7/7] python: add a pycodestyle test
Series looks straightforward to me, ACK series. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 100 libraries supported. http://fedoraproject.org/wiki/MinGW