Some tests were failing, some tests were being skipped, and some tests didn't run with the right paths because automake was setting more environment variables than I thought it would. These patches fix all of that.
Hey clumens welcome back we got you a present: ============================================================================ Testsuite summary for anaconda 20.12 ============================================================================ # TOTAL: 3 # PASS: 3 # SKIP: 0 # XFAIL: 0 # FAIL: 0 # XPASS: 0 # ERROR: 0 ============================================================================
- Remove the executable bit from the regex_tests, which was causing nosetests to skip them - Override the $srcdir set by automake in runpylint.sh and run_check_accelerators.sh. - Add more useful output to the regex tests - Fix the name of the PROXY_URL_REGEX in proxy_test - Add PYTHONPATH when running tests outside automake --- tests/Makefile.am | 3 ++- tests/accelerators/run_check_accelerators.sh | 2 +- tests/nosetests.sh | 7 +++++-- tests/pylint/runpylint.sh | 7 ++++++- tests/regex_tests/groupparse_test.py | 2 +- tests/regex_tests/proxy_test.py | 6 +++--- tests/regex_tests/username_test.py | 4 ++-- tests/testenv.sh | 14 ++++++++++++++ 8 files changed, 34 insertions(+), 11 deletions(-) mode change 100755 => 100644 tests/regex_tests/groupparse_test.py mode change 100755 => 100644 tests/regex_tests/proxy_test.py mode change 100755 => 100644 tests/regex_tests/username_test.py create mode 100644 tests/testenv.sh
diff --git a/tests/Makefile.am b/tests/Makefile.am index 0f63278..eb2b98b 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -30,7 +30,7 @@ # # nosetests will be run for any python files found in a directory matching *_tests
-AM_TESTS_ENVIRONMENT = PYTHONPATH='$(abs_top_builddir)'/pyanaconda/isys/.libs:'$(abs_top_srcdir)'/pyanaconda/isys:'$(abs_top_srcdir)'/pyanaconda:'$(abs_top_srcdir)'; top_srcdir='$(abs_top_srcdir)'; top_builddir='$(abs_top_builddir)'; export PYTHONPATH; export top_srcdir; export top_builddir; +AM_TESTS_ENVIRONMENT = top_srcdir="$(top_srcdir)" top_builddir="$(top_builddir)" ; . $(srcdir)/testenv.sh ; TEST_EXTENSIONS = .sh
dist_check_DATA = pylint/pylint-false-positives @@ -40,6 +40,7 @@ dist_check_SCRIPTS = accelerators/check_accelerators.py \ accelerators/run_check_accelerators.sh \ nosetests.sh \ pylint/runpylint.sh \ + testenv.sh \ $(srcdir)/*_tests/*.py
TESTS = nosetests.sh \ diff --git a/tests/accelerators/run_check_accelerators.sh b/tests/accelerators/run_check_accelerators.sh index ac4969a..a49cda1 100755 --- a/tests/accelerators/run_check_accelerators.sh +++ b/tests/accelerators/run_check_accelerators.sh @@ -1,6 +1,6 @@ #!/bin/sh
: "${top_srcdir:=$(dirname "$0")/../..}" -: "${srcdir:=${top_srcdir}/tests/accelerators}" +srcdir="${top_srcdir}/tests/accelerators"
find "${top_srcdir}" -name '*.glade' -exec "${srcdir}/check_accelerators.py" {} + diff --git a/tests/nosetests.sh b/tests/nosetests.sh index ee4b07c..1ad86c0 100755 --- a/tests/nosetests.sh +++ b/tests/nosetests.sh @@ -1,11 +1,14 @@ #!/bin/sh
-echo $PYTHONPATH - # Use the directory above the one containing the script as the default for # $top_srcdir : "${top_srcdir:=$(dirname "$0")/..}"
+# If no PYTHONPATH is set, import the test environment +if [ -z "$PYTHONPATH" ]; then + . ${top_srcdir}/tests/testenv.sh +fi + # If no tests were selected, select all of them if [ $# -eq 0 ]; then set -- "${top_srcdir}"/tests/*_tests diff --git a/tests/pylint/runpylint.sh b/tests/pylint/runpylint.sh index 72283d9..2cf5797 100755 --- a/tests/pylint/runpylint.sh +++ b/tests/pylint/runpylint.sh @@ -16,7 +16,12 @@ if [ -z "$top_srcdir" ]; then fi
: "${top_srcdir:=$(dirname "$0")/../..}" -: "${srcdir:=${top_srcdir}/tests/pylint}" +srcdir="${top_srcdir}/tests/pylint" + +# If no PYTHONPATH is set, import the test environment +if [ -z "$PYTHONPATH" ]; then + . ${top_srcdir}/tests/testenv.sh +fi
FALSE_POSITIVES="${srcdir}"/pylint-false-positives
diff --git a/tests/regex_tests/groupparse_test.py b/tests/regex_tests/groupparse_test.py old mode 100755 new mode 100644 index 96fd9d7..2802cad --- a/tests/regex_tests/groupparse_test.py +++ b/tests/regex_tests/groupparse_test.py @@ -55,7 +55,7 @@ class GroupParseTestCase(unittest.TestCase): self.assertEqual(GROUPLIST_FANCY_PARSE.match(group).groups(), result) except AssertionError as error: got_error = True - print(error) + print("Group parse error: `%s' did not not parse as `%s'" % (group, result))
if got_error: self.fail() diff --git a/tests/regex_tests/proxy_test.py b/tests/regex_tests/proxy_test.py old mode 100755 new mode 100644 index 1b0e561..0e551f1 --- a/tests/regex_tests/proxy_test.py +++ b/tests/regex_tests/proxy_test.py @@ -20,7 +20,7 @@
import unittest
-from pyanaconda.regexes import PROXY_URL +from pyanaconda.regexes import PROXY_URL_PARSE
class ProxyRegexTestCase(unittest.TestCase): def proxy_regex_test(self): @@ -110,10 +110,10 @@ class ProxyRegexTestCase(unittest.TestCase): got_error = False for proxy, result in tests: try: - self.assertEqual(PROXY_URL.match(proxy).groups(), result) + self.assertEqual(PROXY_URL_PARSE.match(proxy).groups(), result) except AssertionError as error: got_error = True - print error + print("Proxy parse error: `%s' did not parse as `%s'" % (proxy, result))
if got_error: self.fail() diff --git a/tests/regex_tests/username_test.py b/tests/regex_tests/username_test.py old mode 100755 new mode 100644 index 9f2ac2e..9877410 --- a/tests/regex_tests/username_test.py +++ b/tests/regex_tests/username_test.py @@ -31,14 +31,14 @@ class UsernameRegexTestCase(unittest.TestCase): self.assertIsNotNone(expression.match(good)) except AssertionError as error: got_error = True - print(error) + print("Good string %s did not match expression" % good)
for bad in badlist: try: self.assertIsNone(expression.match(bad)) except AssertionError as error: got_error = True - print(error) + print("Bad string %s matched expression" % bad)
if got_error: self.fail() diff --git a/tests/testenv.sh b/tests/testenv.sh new file mode 100644 index 0000000..b4749b4 --- /dev/null +++ b/tests/testenv.sh @@ -0,0 +1,14 @@ +#!/bin/sh + +if [ -z "$top_srcdir" ]; then + echo "*** top_srcdir must be set" + exit 1 +fi + +# If not top_builddir is set, use top_srcdir +: "${top_builddir:=$top_srcdir}" + +PYTHONPATH="${top_builddir}/pyanaconda/isys/.libs:${top_srcdir}/pyanaconda:${top_srcdir}" +export PYTHONPATH +export top_srcdir +export top_builddir
--- pyanaconda/regexes.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/pyanaconda/regexes.py b/pyanaconda/regexes.py index 418ce13..44acc0b 100644 --- a/pyanaconda/regexes.py +++ b/pyanaconda/regexes.py @@ -41,7 +41,7 @@ GECOS_VALID = re.compile(r'^[^:]*$') # both for a maximum total of 32. The empty string is not allowed.
# a base expression without anchors, helpful for building other expressions -_USERNAME_BASE = r'[a-zA-Z0-9._](a-zA-Z0-9._-]{,30}$|[a-zA-Z0-9._-]{,31})' +_USERNAME_BASE = r'[a-zA-Z0-9._](([a-zA-Z0-9._-]{,30}$)|([a-zA-Z0-9._-]{,31}))'
USERNAME_VALID = re.compile(r'^' + _USERNAME_BASE + '$') GROUPNAME_VALID = USERNAME_VALID
--- pyanaconda/installclasses/fedora.py | 2 +- pyanaconda/keyboard.py | 2 +- pyanaconda/network.py | 2 +- pyanaconda/nm.py | 2 -- pyanaconda/ui/gui/hubs/progress.py | 2 +- pyanaconda/ui/gui/spokes/langsupport.py | 4 +--- pyanaconda/ui/gui/spokes/network.py | 2 +- pyanaconda/ui/gui/spokes/welcome.py | 9 ++++----- 8 files changed, 10 insertions(+), 15 deletions(-)
diff --git a/pyanaconda/installclasses/fedora.py b/pyanaconda/installclasses/fedora.py index 00eae33..2977211 100644 --- a/pyanaconda/installclasses/fedora.py +++ b/pyanaconda/installclasses/fedora.py @@ -66,7 +66,7 @@ class InstallClass(BaseInstallClass): dev = network.NetworkDevice(ROOT_PATH + network.netscriptsDir, devName) try: dev.loadIfcfgFile() - except IOError as e: + except IOError: continue dev.set(('ONBOOT', 'yes')) dev.writeIfcfgFile() diff --git a/pyanaconda/keyboard.py b/pyanaconda/keyboard.py index 934b701..5b1643c 100644 --- a/pyanaconda/keyboard.py +++ b/pyanaconda/keyboard.py @@ -517,7 +517,7 @@ class XklWrapper(object): # BUG?: if the last layout in the list is activated and removed, # state.group may be equal to n_groups if cur_group >= num_groups: - cur_group = num_groups - 1; + cur_group = num_groups - 1
layout = self._rec.layouts[cur_group] variant = self._rec.variants[cur_group] diff --git a/pyanaconda/network.py b/pyanaconda/network.py index f70c0a2..3c7f638 100644 --- a/pyanaconda/network.py +++ b/pyanaconda/network.py @@ -339,7 +339,7 @@ def dumpMissingDefaultIfcfgs(): log.debug("network: dumping ifcfg file for default autoconnection on %s", devname) nm.nm_update_settings_of_device(devname, [['connection', 'autoconnect', False, None]]) log.debug("network: setting autoconnect of %s to False" , devname) - except nm.DeviceSettingsNotFoundError as e: + except nm.DeviceSettingsNotFoundError: log.debug("network: no ifcfg file for %s", devname) rv = True
diff --git a/pyanaconda/nm.py b/pyanaconda/nm.py index 1109d6e..76d4288 100644 --- a/pyanaconda/nm.py +++ b/pyanaconda/nm.py @@ -277,8 +277,6 @@ def nm_device_ip_config(name, version=4): UnknownDeviceError if device is not found PropertyNotFoundError if ip configuration is not found """ - retval = [] - state = nm_device_property(name, "State") if state != NetworkManager.DeviceState.ACTIVATED: return [] diff --git a/pyanaconda/ui/gui/hubs/progress.py b/pyanaconda/ui/gui/hubs/progress.py index 45b4734..e89f7cf 100644 --- a/pyanaconda/ui/gui/hubs/progress.py +++ b/pyanaconda/ui/gui/hubs/progress.py @@ -29,7 +29,7 @@ import sys import glob
from pyanaconda.i18n import _ -from pyanaconda.localization import langcode_matches_locale, find_best_locale_match +from pyanaconda.localization import find_best_locale_match from pyanaconda.product import productName from pyanaconda.flags import flags from pyanaconda.constants import THREAD_INSTALL, THREAD_CONFIGURATION, DEFAULT_LANG diff --git a/pyanaconda/ui/gui/spokes/langsupport.py b/pyanaconda/ui/gui/spokes/langsupport.py index 125e3b5..39e93d2 100644 --- a/pyanaconda/ui/gui/spokes/langsupport.py +++ b/pyanaconda/ui/gui/spokes/langsupport.py @@ -20,14 +20,12 @@ # Vratislav Podzimek vpodzime@redhat.com #
-from gi.repository import Gtk, Pango +from gi.repository import Pango from pyanaconda.flags import flags from pyanaconda.i18n import N_ -from pyanaconda.iutil import strip_accents from pyanaconda.ui.gui.spokes import NormalSpoke from pyanaconda.ui.gui.categories.localization import LocalizationCategory from pyanaconda.ui.gui.spokes.lib.lang_locale_handler import LangLocaleHandler -from pyanaconda.ui.gui.utils import set_treeview_selection from pyanaconda import localization
import re diff --git a/pyanaconda/ui/gui/spokes/network.py b/pyanaconda/ui/gui/spokes/network.py index 97c2174..ee76fe2 100644 --- a/pyanaconda/ui/gui/spokes/network.py +++ b/pyanaconda/ui/gui/spokes/network.py @@ -43,7 +43,7 @@ from pyanaconda.ui.gui.utils import gtk_call_once, enlightbox from pyanaconda.ui.common import FirstbootSpokeMixIn
from pyanaconda import network -from pyanaconda.nm import nm_activated_devices, nm_device_setting_value, nm_dbus_ay_to_ipv6 +from pyanaconda.nm import nm_device_setting_value, nm_dbus_ay_to_ipv6
from gi.repository import GLib, GObject, Pango, Gio, NetworkManager, NMClient import dbus diff --git a/pyanaconda/ui/gui/spokes/welcome.py b/pyanaconda/ui/gui/spokes/welcome.py index 63218ac..219c344 100644 --- a/pyanaconda/ui/gui/spokes/welcome.py +++ b/pyanaconda/ui/gui/spokes/welcome.py @@ -24,10 +24,9 @@ import sys import re import langtable
-from gi.repository import Gtk, Pango from pyanaconda.ui.gui.hubs.summary import SummaryHub from pyanaconda.ui.gui.spokes import StandaloneSpoke -from pyanaconda.ui.gui.utils import enlightbox, set_treeview_selection +from pyanaconda.ui.gui.utils import enlightbox from pyanaconda.ui.gui.spokes.lib.lang_locale_handler import LangLocaleHandler
from pyanaconda import localization @@ -36,7 +35,7 @@ from pyanaconda import keyboard from pyanaconda import flags from pyanaconda import geoloc from pyanaconda.i18n import _ -from pyanaconda.iutil import is_unsupported_hw, strip_accents +from pyanaconda.iutil import is_unsupported_hw from pyanaconda.constants import DEFAULT_LANG
import logging @@ -150,7 +149,7 @@ class WelcomeLanguageSpoke(LangLocaleHandler, StandaloneSpoke): self._languageEntry = self.builder.get_object("languageEntry") self._langSelection = self.builder.get_object("languageViewSelection") self._langSelectedRenderer = self.builder.get_object("langSelectedRenderer") - self._langSelectedColumn = self.builder.get_object("langSelectedColumn"); + self._langSelectedColumn = self.builder.get_object("langSelectedColumn") self._langView = self.builder.get_object("languageView") self._localeView = self.builder.get_object("localeView") self._localeStore = self.builder.get_object("localeStore") @@ -243,7 +242,7 @@ class WelcomeLanguageSpoke(LangLocaleHandler, StandaloneSpoke):
# Signal handlers. def on_lang_selection_changed(self, selection): - (store, selected) = selection.get_selected_rows() + (_store, selected) = selection.get_selected_rows() LangLocaleHandler.on_lang_selection_changed(self, selection)
if not selected and hasattr(self.window, "set_may_continue"):
--- pyanaconda/network.py | 2 +- pyanaconda/packaging/yumpayload.py | 2 +- pyanaconda/ui/gui/spokes/welcome.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/pyanaconda/network.py b/pyanaconda/network.py index 3c7f638..692d655 100644 --- a/pyanaconda/network.py +++ b/pyanaconda/network.py @@ -433,7 +433,7 @@ def get_ks_network_data(devname, ifcfg_suffix=None): try: device_cfg.loadIfcfgFile() except IOError as e: - log.debug("get_ks_network_data %s: %s" % (ifcfg_suffix, e)) + log.debug("get_ks_network_data %s: %s", ifcfg_suffix, e) return None retval = kickstartNetworkData(ifcfg=device_cfg) if retval and devname in nm.nm_activated_devices(): diff --git a/pyanaconda/packaging/yumpayload.py b/pyanaconda/packaging/yumpayload.py index 5ca770a..142678c 100644 --- a/pyanaconda/packaging/yumpayload.py +++ b/pyanaconda/packaging/yumpayload.py @@ -1230,7 +1230,7 @@ reposdir=%s # check dependencies log.info("checking dependencies") (code, msgs) = self._yum.buildTransaction(unfinished_transactions_check=False) - log.debug("buildTransaction = (%s, %s)" % (code, msgs)) + log.debug("buildTransaction = (%s, %s)", code, msgs) self._removeTxSaveFile() if code == 0: # empty transaction? diff --git a/pyanaconda/ui/gui/spokes/welcome.py b/pyanaconda/ui/gui/spokes/welcome.py index 219c344..68fd007 100644 --- a/pyanaconda/ui/gui/spokes/welcome.py +++ b/pyanaconda/ui/gui/spokes/welcome.py @@ -116,7 +116,7 @@ class WelcomeLanguageSpoke(LangLocaleHandler, StandaloneSpoke): # does not support typing ASCII chars, append the 'us' layout new_layouts.append("us") else: - log.error("Failed to get layout for chosen locale '%s'" % locale) + log.error("Failed to get layout for chosen locale '%s'", locale) new_layouts = ["us"]
self.data.keyboard.x_layouts = new_layouts
--- pyanaconda/ui/gui/spokes/langsupport.py | 1 + pyanaconda/ui/gui/spokes/welcome.py | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/pyanaconda/ui/gui/spokes/langsupport.py b/pyanaconda/ui/gui/spokes/langsupport.py index 39e93d2..dc6b815 100644 --- a/pyanaconda/ui/gui/spokes/langsupport.py +++ b/pyanaconda/ui/gui/spokes/langsupport.py @@ -47,6 +47,7 @@ class LangsupportSpoke(LangLocaleHandler, NormalSpoke):
def __init__(self, *args, **kwargs): NormalSpoke.__init__(self, *args, **kwargs) + LangLocaleHandler.__init__(self, *args, **kwargs) self._selected_locales = set()
def initialize(self): diff --git a/pyanaconda/ui/gui/spokes/welcome.py b/pyanaconda/ui/gui/spokes/welcome.py index 68fd007..85352d6 100644 --- a/pyanaconda/ui/gui/spokes/welcome.py +++ b/pyanaconda/ui/gui/spokes/welcome.py @@ -54,6 +54,7 @@ class WelcomeLanguageSpoke(LangLocaleHandler, StandaloneSpoke):
def __init__(self, *args, **kwargs): StandaloneSpoke.__init__(self, *args, **kwargs) + LangLocaleHandler.__init__(self, *args, **kwargs) self._xklwrapper = keyboard.XklWrapper.get_instance() self._origStrings = {}
@@ -246,7 +247,7 @@ class WelcomeLanguageSpoke(LangLocaleHandler, StandaloneSpoke): LangLocaleHandler.on_lang_selection_changed(self, selection)
if not selected and hasattr(self.window, "set_may_continue"): - self.window.set_may_continue(False) + self.window.set_may_continue(False)
def on_locale_selection_changed(self, selection): (store, selected) = selection.get_selected_rows()
Pylint does not handle the _make, _asdict, _replace and _fields attributes of namedtuple objects, and it looks like they have no interest in doing so. See http://www.logilab.org/ticket/33617 --- pyanaconda/ui/gui/spokes/advstorage/iscsi.py | 1 + pyanaconda/ui/gui/spokes/filter.py | 5 +++++ 2 files changed, 6 insertions(+)
diff --git a/pyanaconda/ui/gui/spokes/advstorage/iscsi.py b/pyanaconda/ui/gui/spokes/advstorage/iscsi.py index 0c96543..020ca16 100644 --- a/pyanaconda/ui/gui/spokes/advstorage/iscsi.py +++ b/pyanaconda/ui/gui/spokes/advstorage/iscsi.py @@ -331,6 +331,7 @@ class ISCSIDialog(GUIObject):
def _login(self, credentials): for row in self._store: + # pylint: disable-msg=E1101 obj = NodeStoreRow._make(row)
if not obj.selected: diff --git a/pyanaconda/ui/gui/spokes/filter.py b/pyanaconda/ui/gui/spokes/filter.py index fe8c354..32d2a49 100644 --- a/pyanaconda/ui/gui/spokes/filter.py +++ b/pyanaconda/ui/gui/spokes/filter.py @@ -202,6 +202,7 @@ class SearchPage(FilterPage): return hasattr(device, "fcp_lun") and self._lunEntry.get_text() in device.fcp_lun
def visible_func(self, model, itr, *args): + # pylint: disable-msg=E1101 obj = DiskStoreRow._make(model[itr]) device = self.storage.devicetree.getDeviceByName(obj.name, hidden=True) return self._filter_func(device) @@ -269,6 +270,7 @@ class MultipathPage(FilterPage): if not flags.mpath: return False
+ # pylint: disable-msg=E1101 obj = DiskStoreRow._make(model[itr]) device = self.storage.devicetree.getDeviceByName(obj.name, hidden=True) return self.ismember(device) and self._filter_func(device) @@ -356,6 +358,7 @@ class OtherPage(FilterPage): return False
def visible_func(self, model, itr, *args): + # pylint: disable-msg=E1101 obj = DiskStoreRow._make(model[itr]) device = self.storage.devicetree.getDeviceByName(obj.name, hidden=True) return self.ismember(device) and self._filter_func(device) @@ -373,6 +376,7 @@ class RaidPage(FilterPage): if not flags.dmraid: return False
+ # pylint: disable-msg=E1101 obj = DiskStoreRow._make(model[itr]) device = self.storage.devicetree.getDeviceByName(obj.name, hidden=True) return self.ismember(device) @@ -393,6 +397,7 @@ class ZPage(FilterPage): return
def visible_func(self, model, itr, *args): + # pylint: disable-msg=E1101 obj = DiskStoreRow._make(model[itr]) device = self.storage.devicetree.getDeviceByName(obj.name, hidden=True) return self.ismember(device)
On Sun, 2013-09-08 at 12:33 -0400, David Shea wrote:
Pylint does not handle the _make, _asdict, _replace and _fields attributes of namedtuple objects, and it looks like they have no interest in doing so. See http://www.logilab.org/ticket/33617
That's because those are hardly ever needed. We should use NamedTuple(iterable) instead of NamedTuple._make(iterable). Please do this change instead of disabling pylint on those places.
Otherwise these all look good to me. Thanks for removing a lot of mess I've introduced with my recent patches.
On 09/09/2013 04:06 AM, Vratislav Podzimek wrote:
On Sun, 2013-09-08 at 12:33 -0400, David Shea wrote:
Pylint does not handle the _make, _asdict, _replace and _fields attributes of namedtuple objects, and it looks like they have no interest in doing so. See http://www.logilab.org/ticket/33617
That's because those are hardly ever needed. We should use NamedTuple(iterable) instead of NamedTuple._make(iterable). Please do this change instead of disabling pylint on those places.
Otherwise these all look good to me. Thanks for removing a lot of mess I've introduced with my recent patches.
I'll change this locally. In this case, I think that "obj = TupleClass(*row)" looks better than "obj = TupleClass._make(row)", but in general I don't like the idea of changing our code just because pylint doesn't understand a particular feature of python.
Some tests were failing, some tests were being skipped, and some tests didn't run with the right paths because automake was setting more environment variables than I thought it would. These patches fix all of that.
Hey clumens welcome back we got you a present:
Testsuite summary for anaconda 20.12
# TOTAL: 3 # PASS: 3 # SKIP: 0 # XFAIL: 0 # FAIL: 0 # XPASS: 0
# ERROR: 0
Cool, glad to see it! I'll update the VMs with the latest pylint to make this dream a reality.
- Chris
anaconda-patches@lists.fedorahosted.org
-
Chris Lumens -
David Shea -
Vratislav Podzimek