pylint has gotten picky about this recently, and this is similar to other patches I made for blivet and pyparted. In a couple of these places, I think we are being too clever, but that is likely a fair bit more work. --- pyanaconda/anaconda_argparse.py | 3 ++- pyanaconda/flags.py | 3 ++- pyanaconda/iutil.py | 4 ++-- pyanaconda/nm.py | 5 +++-- pyanaconda/ui/tui/simpleline/base.py | 3 ++- scripts/makebumpver | 10 ++++------ tests/pylint/markup.py | 2 +- tests/pylint/pointless-override.py | 2 +- 8 files changed, 17 insertions(+), 15 deletions(-)
diff --git a/pyanaconda/anaconda_argparse.py b/pyanaconda/anaconda_argparse.py index 916397f..a70365f 100644 --- a/pyanaconda/anaconda_argparse.py +++ b/pyanaconda/anaconda_argparse.py @@ -28,6 +28,7 @@ import os import sys import fcntl import termios +import types import struct
from argparse import ArgumentParser, ArgumentError, HelpFormatter, Namespace @@ -178,7 +179,7 @@ class AnacondaArgumentParser(ArgumentParser): :rtype: Namespace """ namespace = Namespace() - if boot_cmdline is None or type(boot_cmdline) is str: + if boot_cmdline is None or isinstance(boot_cmdline, types.StringType): bootargs = BootArgs(boot_cmdline) else: bootargs = boot_cmdline diff --git a/pyanaconda/flags.py b/pyanaconda/flags.py index 33d8d95..c817234 100644 --- a/pyanaconda/flags.py +++ b/pyanaconda/flags.py @@ -19,6 +19,7 @@
import selinux import shlex +import types import glob from pyanaconda.constants import SELINUX_DEFAULT, CMDLINE_APPEND from collections import OrderedDict @@ -113,7 +114,7 @@ class BootArgs(OrderedDict): filenames can contain *, ?, and character ranges expressed with [] """ readfiles = [] - if type(filenames) == str: + if isinstance(filenames, types.StringType): filenames = [filenames]
# Expand any filename globs diff --git a/pyanaconda/iutil.py b/pyanaconda/iutil.py index 6798c41..30df3d5 100644 --- a/pyanaconda/iutil.py +++ b/pyanaconda/iutil.py @@ -1045,14 +1045,14 @@ _ASCIIlower_table = string.maketrans(string.ascii_uppercase, string.ascii_lowerc
def _toASCII(s): """Convert a unicode string to ASCII""" - if type(s) == types.UnicodeType: + if isinstance(s, types.UnicodeType): # Decompose the string using the NFK decomposition, which in addition # to the canonical decomposition replaces characters based on # compatibility equivalence (e.g., ROMAN NUMERAL ONE has its own code # point but it's really just a capital I), so that we can keep as much # of the ASCII part of the string as possible. s = unicodedata.normalize('NKFD', s).encode('ascii', 'ignore') - elif type(s) != types.StringType: + elif isinstance(s, types.StringType): s = '' return s
diff --git a/pyanaconda/nm.py b/pyanaconda/nm.py index 5cbf5d3..092dffd 100644 --- a/pyanaconda/nm.py +++ b/pyanaconda/nm.py @@ -23,6 +23,7 @@ from gi.repository import Gio, GLib from gi.repository import NetworkManager import struct import socket +import types import logging log = logging.getLogger("anaconda")
@@ -984,9 +985,9 @@ def _gvariant_settings(settings, updated_key1, updated_key2, value, default_type
if type_str is None: # infer the new value type for string and boolean - if type(value) is type(True): + if isinstance(value, types.BooleanType): type_str = 'b' - if type(value) is type(''): + elif isinstance(value, types.StringType): type_str = 's'
if type_str is not None: diff --git a/pyanaconda/ui/tui/simpleline/base.py b/pyanaconda/ui/tui/simpleline/base.py index 952c5a8..991f659 100644 --- a/pyanaconda/ui/tui/simpleline/base.py +++ b/pyanaconda/ui/tui/simpleline/base.py @@ -25,6 +25,7 @@ import sys import Queue import getpass import threading +import types import functools from pyanaconda.threads import threadMgr, AnacondaThread from pyanaconda.ui.communication import hubQ @@ -580,7 +581,7 @@ class UIScreen(object): w.render(self.app.width) if isinstance(w, Widget): self._print_long_widget(w) - elif type(w) == str: + elif isinstance(w, types.StringType): print(w.decode("utf-8")) else: # not a widget, just print its unicode representation diff --git a/scripts/makebumpver b/scripts/makebumpver index f092bb4..558010c 100755 --- a/scripts/makebumpver +++ b/scripts/makebumpver @@ -146,9 +146,9 @@ class MakeBumpVer: ret = proc[0].strip('\n').split('\n')
if len(ret) == 1 and ret[0].find('@') != -1: - ret = ret[0].split('@')[0] + ret = [ret[0].split('@')[0]] elif len(ret) == 1: - ret = ret[0] + ret = [ret[0]] else: ret = filter(lambda x: x != '', ret)
@@ -274,11 +274,9 @@ class MakeBumpVer: fields = line.split(' ') commit = fields[0]
- summary = self._getCommitDetail(commit, "%s") + summary = self._getCommitDetail(commit, "%s")[0] body = self._getCommitDetail(commit, "%b") - if type(body) is not list: - body = [body] - author = self._getCommitDetail(commit, "%aE") + author = self._getCommitDetail(commit, "%aE")[0]
if self.rhel: rhbz = set() diff --git a/tests/pylint/markup.py b/tests/pylint/markup.py index 690eb13..4217472 100644 --- a/tests/pylint/markup.py +++ b/tests/pylint/markup.py @@ -112,7 +112,7 @@ class MarkupChecker(BaseChecker):
@check_messages("invalid-markup", "invalid-markup-element", "unescaped-markup", "invalid-translated-markup", "invalid-translated-markup-element", "invalid-pango-translation", "unnecessary-markup") def visit_const(self, node): - if type(node.value) not in (types.StringType, types.UnicodeType): + if isinstance(node.value, types.StringType) or isinstance(node.value, types.UnicodeType): return
if not is_markup(node.value): diff --git a/tests/pylint/pointless-override.py b/tests/pylint/pointless-override.py index c8d4776..d10ba42 100644 --- a/tests/pylint/pointless-override.py +++ b/tests/pylint/pointless-override.py @@ -153,7 +153,7 @@ class PointlessAssignment(PointlessData):
@classmethod def check_equal(cls, node, other): - if type(node) != type(other): + if not isinstance(node, other.__class__): return False if isinstance(node, astroid.Const): return node.value == other.value
This also changes them to start with KSTEST_ instead of TEST_, as that's the prefix we look for to know to pass them in. --- tests/kickstart_tests/basic-ostree.sh | 6 +++--- tests/kickstart_tests/nfs-repo-and-addon.sh | 24 ++++++++++++------------ tests/kickstart_tests/run_kickstart_tests.sh | 8 +++++--- 3 files changed, 20 insertions(+), 18 deletions(-)
diff --git a/tests/kickstart_tests/basic-ostree.sh b/tests/kickstart_tests/basic-ostree.sh index 0416f4f..22c5cc1 100755 --- a/tests/kickstart_tests/basic-ostree.sh +++ b/tests/kickstart_tests/basic-ostree.sh @@ -26,12 +26,12 @@ prepare() { ks=$1 tmpdir=$2
- if [[ "${TEST_OSTREE_REPO}" == "" ]]; then - echo $TEST_OSTREE_REPO is not set. + if [[ "${KSTEST_OSTREE_REPO}" == "" ]]; then + echo $KSTEST_OSTREE_REPO is not set. return 1 fi
- sed -e "/ostreesetup/ s|REPO|${TEST_OSTREE_REPO}|" ${ks} > ${tmpdir}/kickstart.ks + sed -e "/ostreesetup/ s|REPO|${KSTEST_OSTREE_REPO}|" ${ks} > ${tmpdir}/kickstart.ks echo ${tmpdir}/kickstart.ks }
diff --git a/tests/kickstart_tests/nfs-repo-and-addon.sh b/tests/kickstart_tests/nfs-repo-and-addon.sh index be4e421..a6e35a8 100755 --- a/tests/kickstart_tests/nfs-repo-and-addon.sh +++ b/tests/kickstart_tests/nfs-repo-and-addon.sh @@ -25,30 +25,30 @@ prepare() { ks=$1 tmpdir=$2
- if [[ "${TEST_ADDON_NFS_REPO}" == "" ]]; then - echo $TEST_ADDON_NFS_REPO is not set. + if [[ "${KSTEST_ADDON_NFS_REPO}" == "" ]]; then + echo $KSTEST_ADDON_NFS_REPO is not set. return 1 fi
- if [[ "${TEST_ADDON_HTTP_REPO}" == "" ]]; then - echo $TEST_ADDON_HTTP_REPO is not set. + if [[ "${KSTEST_ADDON_HTTP_REPO}" == "" ]]; then + echo $KSTEST_ADDON_HTTP_REPO is not set. return 1 fi
- if [[ "${TEST_NFS_SERVER}" == "" ]]; then - echo $TEST_NFS_SERVER is not set + if [[ "${KSTEST_NFS_SERVER}" == "" ]]; then + echo $KSTEST_NFS_SERVER is not set return 1 fi
- if [[ "${TEST_NFS_PATH}" == "" ]]; then - echo $TEST_NFS_PATH is not set + if [[ "${KSTEST_NFS_PATH}" == "" ]]; then + echo $KSTEST_NFS_PATH is not set return 1 fi
- sed -e "/^nfs/ s|NFS-SERVER|${TEST_NFS_SERVER}|" \ - -e "/^nfs/ s|NFS-PATH|${TEST_NFS_PATH}|" \ - -e "/^repo/ s|NFS-ADDON-REPO|${TEST_ADDON_NFS_REPO}|" \ - -e "/^repo/ s|HTTP-ADDON-REPO|${TEST_ADDON_HTTP_REPO}|" ${ks} > ${tmpdir}/kickstart.ks + sed -e "/^nfs/ s|NFS-SERVER|${KSTEST_NFS_SERVER}|" \ + -e "/^nfs/ s|NFS-PATH|${KSTEST_NFS_PATH}|" \ + -e "/^repo/ s|NFS-ADDON-REPO|${KSTEST_ADDON_NFS_REPO}|" \ + -e "/^repo/ s|HTTP-ADDON-REPO|${KSTEST_ADDON_HTTP_REPO}|" ${ks} > ${tmpdir}/kickstart.ks echo ${tmpdir}/kickstart.ks }
diff --git a/tests/kickstart_tests/run_kickstart_tests.sh b/tests/kickstart_tests/run_kickstart_tests.sh index 0e2990c..61350f7 100755 --- a/tests/kickstart_tests/run_kickstart_tests.sh +++ b/tests/kickstart_tests/run_kickstart_tests.sh @@ -68,13 +68,15 @@ fi KEEPIT=${KEEPIT:-0}
# This is for environment variables that parallel needs to pass to -# remote systems. Put anything here that test cases care about or -# they won't work when run on some systems. +# remote systems. # # NOTE: You will also need to add these to the list in /etc/sudoers # if you are using env_reset there, or they will not get passed from # this script to parallel. -env_args="--env TEST_OSTREE_REPO --env TEST_NFS_SERVER --env TEST_NFS_PATH --env TEST_ADDON_NFS_REPO --env TEST_ADDON_HTTP_REPO" +env_args=$(printenv | while read line; do + v="$(echo $line | cut -d'=' -f1)" + [[ "${v}" =~ ^KSTEST_ ]] && echo "--env ${v}" + done)
# Round up all the kickstart tests we want to run, skipping those that are not # executable as well as this file itself.
This makes it way easier to just run a single test - you simply provide it as a command line argument. If no tests are given as arguments, it defaults to using find as before.
This also involved reorganizing command line processing a bit to also let it accept the same command line arguments as run_one_ks.sh does. --- tests/kickstart_tests/run_kickstart_tests.sh | 61 ++++++++++++++++++++-------- 1 file changed, 43 insertions(+), 18 deletions(-)
diff --git a/tests/kickstart_tests/run_kickstart_tests.sh b/tests/kickstart_tests/run_kickstart_tests.sh index 61350f7..169ef68 100755 --- a/tests/kickstart_tests/run_kickstart_tests.sh +++ b/tests/kickstart_tests/run_kickstart_tests.sh @@ -47,19 +47,9 @@
# The boot.iso location can come from one of two different places: # (1) $TEST_BOOT_ISO, if this script is being called from "make check" -# (2) The command line, if this script is being called directly. -IMAGE="" -if [[ "${TEST_BOOT_ISO}" != "" ]]; then - IMAGE=${TEST_BOOT_ISO} -elif [[ $# != 0 ]]; then - IMAGE=$1 - shift -fi - -if [[ ! -e "${IMAGE}" ]]; then - echo "Required boot.iso does not exist; skipping." - exit 77 -fi +# (2) The command line, if this script is being called directly. That will +# be checked below. +IMAGE="${TEST_BOOT_ISO}"
# Possible values for this parameter: # 0 - Keep nothing (the default) @@ -67,6 +57,34 @@ fi # 2 - Keep log files and disk images (will take up a lot of space) KEEPIT=${KEEPIT:-0}
+while getopts ":i:k:" opt; do + case $opt in + i) + # If this wasn't set from the environment, set it from the command line + # here. If it never gets set, we'll catch that later and error out. + IMAGE=$OPTARG + ;; + + k) + # This overrides either the $KEEPIT environment variable, or the default + # setting from above. + KEEPIT=$OPTARG + ;; + + *) + echo "Usage: run_kickstart_tests.sh [-i boot.iso] [-k 0|1|2] [tests]" + exit 1 + ;; + esac +done + +if [[ ! -e "${IMAGE}" ]]; then + echo "Required boot.iso does not exist; skipping." + exit 77 +fi + +shift $((OPTIND - 1)) + # This is for environment variables that parallel needs to pass to # remote systems. # @@ -78,9 +96,16 @@ env_args=$(printenv | while read line; do [[ "${v}" =~ ^KSTEST_ ]] && echo "--env ${v}" done)
-# Round up all the kickstart tests we want to run, skipping those that are not -# executable as well as this file itself. -find kickstart_tests -name '*sh' -a -perm -o+x -a ! -wholename 'kickstart_tests/run_*.sh' | \ +# We get the list of tests from one of two places: +# (1) From the command line, all the other arguments. +# (2) From finding all scripts in kickstart_tests/ that are executable and are +# not support files. +if [[ $# != 0 ]]; then + tests="$*" +else + tests=$(find kickstart_tests -name '*sh' -a -perm -o+x -a ! -wholename 'kickstart_tests/run_*.sh') +fi + if [[ "$TEST_REMOTES" != "" ]]; then _IMAGE=kickstart_tests/$(basename ${IMAGE})
@@ -107,7 +132,7 @@ if [[ "$TEST_REMOTES" != "" ]]; then
parallel --no-notice ${remote_args} \ ${env_args} --jobs ${TEST_JOBS:-2} \ - sudo kickstart_tests/run_one_ks.sh -i ${_IMAGE} -k ${KEEPIT} {} + sudo kickstart_tests/run_one_ks.sh -i ${_IMAGE} -k ${KEEPIT} {} ::: ${tests} rc=$?
# (3) Get all the results back from the remote systems, which will have already @@ -134,7 +159,7 @@ if [[ "$TEST_REMOTES" != "" ]]; then exit ${rc} else parallel --no-notice ${env_args} --jobs ${TEST_JOBS:-2} \ - sudo kickstart_tests/run_one_ks.sh -i ${IMAGE} -k ${KEEPIT} {} + sudo kickstart_tests/run_one_ks.sh -i ${IMAGE} -k ${KEEPIT} {} ::: ${tests}
# For future expansion - any cleanup code can go in between the variable # setting and the exit, like in the other branch of the if-else above.
On 05/07/2015 01:41 PM, Chris Lumens wrote:
pylint has gotten picky about this recently, and this is similar to other patches I made for blivet and pyparted. In a couple of these places, I think we are being too clever, but that is likely a fair bit more work.
pyanaconda/anaconda_argparse.py | 3 ++- pyanaconda/flags.py | 3 ++- pyanaconda/iutil.py | 4 ++-- pyanaconda/nm.py | 5 +++-- pyanaconda/ui/tui/simpleline/base.py | 3 ++- scripts/makebumpver | 10 ++++------ tests/pylint/markup.py | 2 +- tests/pylint/pointless-override.py | 2 +- 8 files changed, 17 insertions(+), 15 deletions(-)
These three all look fine.
anaconda-patches@lists.fedorahosted.org