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