From ed0c4e7609fa47ca549ebc2522516ab3d60bd1ae Mon Sep 17 00:00:00 2001 From: Miro Hrončok Date: May 08 2018 08:44:25 +0000 Subject: [PATCH 1/16] Add tox support to tox.ini, py27 only for now --- diff --git a/tox.ini b/tox.ini index 05f2c97..057a0e1 100644 --- a/tox.ini +++ b/tox.ini @@ -15,3 +15,16 @@ minversion=2.0 python_functions=test should python_files=test_* functest_* addopts=--functional testing/ --cov-report=term-missing --cov libtaskotron + +[tox] +envlist = py27 + +[testenv] +deps = -rrequirements.txt +commands = python -m pytest {posargs} +# setup.py has from utils import... +setenv = PYTHONPATH = {toxinidir} +# needs hawkey, koji +sitepackages = True +# tests read HOME +passenv = HOME From 1c8d0574fc31d87c9f8cf83ac6cf98b25710cbfe Mon Sep 17 00:00:00 2001 From: Miro Hrončok Date: May 08 2018 08:45:47 +0000 Subject: [PATCH 2/16] Python 3: Fix except syntax python-modernize -wnf lib2to3.fixes.fix_except https://portingguide.readthedocs.io/en/latest/exceptions.html#the-new-except-syntax --- diff --git a/data/ansible/library/acquire_subject.py b/data/ansible/library/acquire_subject.py index 180589f..373122b 100644 --- a/data/ansible/library/acquire_subject.py +++ b/data/ansible/library/acquire_subject.py @@ -50,7 +50,7 @@ def main(): subject_handler = infer_subject_handler(mod) try: subjects = subject_handler(mod) - except exc.TaskotronError, e: + except exc.TaskotronError as e: mod.fail_json(msg=e.msg) mod.exit_json(msg="Successfuly acquired subjects", changed=True, subjects=subjects) diff --git a/data/ansible/library/taskotron_koji.py b/data/ansible/library/taskotron_koji.py index 4f953ea..79bc699 100644 --- a/data/ansible/library/taskotron_koji.py +++ b/data/ansible/library/taskotron_koji.py @@ -37,7 +37,7 @@ def main(): try: kojidirective = KojiDirective() data = kojidirective.process(mod) - except exc.TaskotronError, e: + except exc.TaskotronError as e: mod.fail_json(msg=e) subjects = ' '.join(data['downloaded_rpms']) diff --git a/libtaskotron/buildbot_utils.py b/libtaskotron/buildbot_utils.py index 94f463c..762f215 100644 --- a/libtaskotron/buildbot_utils.py +++ b/libtaskotron/buildbot_utils.py @@ -22,7 +22,7 @@ def parse_jobid(jobid): try: builder, buildid = jobid.split('/') return (builder, buildid) - except ValueError, e: + except ValueError as e: log.debug(e) log.warning( 'Invalid jobid format detected, resetting to default ' diff --git a/libtaskotron/config.py b/libtaskotron/config.py index a9f7096..3c86cbc 100644 --- a/libtaskotron/config.py +++ b/libtaskotron/config.py @@ -295,7 +295,7 @@ def parse_yaml_from_file(filename): with open(filename, 'r') as datafile: try: return yaml.safe_load(datafile.read()) - except yaml.YAMLError, e: + except yaml.YAMLError as e: raise exc.TaskotronConfigError(e) diff --git a/libtaskotron/directives/distgit_directive.py b/libtaskotron/directives/distgit_directive.py index c83d953..10daa62 100644 --- a/libtaskotron/directives/distgit_directive.py +++ b/libtaskotron/directives/distgit_directive.py @@ -194,7 +194,7 @@ class DistGitDirective(BaseDirective): output_data['downloaded_files'].append( file_utils.download(url, '.', localpath) ) - except exc.TaskotronRemoteError, e: + except exc.TaskotronRemoteError as e: if e.errno == 404 and ignore_missing: log.debug('File not found, ignoring: %s', url) else: diff --git a/libtaskotron/directives/resultsdb_directive.py b/libtaskotron/directives/resultsdb_directive.py index 34efc29..ed95561 100644 --- a/libtaskotron/directives/resultsdb_directive.py +++ b/libtaskotron/directives/resultsdb_directive.py @@ -140,7 +140,7 @@ class ResultsdbDirective(BaseDirective): self.resultsdb.get_testcase(name) self._ensured_testcases.append(name) return - except resultsdb_api.ResultsDBapiException, e: + except resultsdb_api.ResultsDBapiException as e: if not e.message.startswith('Testcase not found'): raise e @@ -224,7 +224,7 @@ class ResultsdbDirective(BaseDirective): gitconfig = configparser.ConfigParser() gitconfig.read(gitconfig_filename) task_repo_url = gitconfig['remote "origin"']['url'] - except TypeError, e: + except TypeError as e: log.exception(e) raise TaskotronDirectiveError("Could not find task's git remote 'origin' url" "in %s" % gitconfig_filename) @@ -304,7 +304,7 @@ class ResultsdbDirective(BaseDirective): log.debug('Result saved in ResultsDB:\n%s', pprint.pformat(result)) detail._internal['resultsdb_result_id'] = result['id'] - except resultsdb_api.ResultsDBapiException, e: + except resultsdb_api.ResultsDBapiException as e: log.error(e) log.error("Failed to store to ResultsDB: `%s` `%s` `%s`", detail.item, checkname, detail.outcome) diff --git a/libtaskotron/executor.py b/libtaskotron/executor.py index 79ab07d..e1d6934 100644 --- a/libtaskotron/executor.py +++ b/libtaskotron/executor.py @@ -24,7 +24,7 @@ from libtaskotron.directives import resultsdb_directive try: from libtaskotron.ext.disposable import vm -except ImportError, e: +except ImportError as e: raise exc.TaskotronImportError(e) @@ -69,7 +69,7 @@ class Executor(object): self.task_vm.ipaddr) return self.task_vm.ipaddr - except vm.TestcloudInstanceError, e: + except vm.TestcloudInstanceError as e: if retries <= 0: raise exc.TaskotronMinionError('Disposable client failed ' 'to boot: %s', e) @@ -132,7 +132,7 @@ class Executor(object): subprocess.check_call(['ansible-playbook', '--syntax-check', playbook]) log.debug('Playbook is syntactically correct: %s', playbook) - except subprocess.CalledProcessError, e: + except subprocess.CalledProcessError as e: log.error('Syntax check failed for playbook %s: %s', playbook, e) raise exc.TaskotronPlaybookError(e) diff --git a/libtaskotron/file_utils.py b/libtaskotron/file_utils.py index fdf8e50..149ddb3 100644 --- a/libtaskotron/file_utils.py +++ b/libtaskotron/file_utils.py @@ -42,7 +42,7 @@ def makedirs(fullpath): try: os.makedirs(fullpath) assert os.path.isdir(fullpath) - except OSError, e: + except OSError as e: if e.errno == 17: # "[Errno 17] File exists" # if it is a directory everything is ok if os.path.isdir(fullpath): @@ -120,13 +120,13 @@ def download(url, dirname, filename=None, cachedir=None): log.debug('Downloading%s: %s', ' (cached)' if cachedir else '', url) try: _download(url, dl_dest) - except requests.exceptions.RequestException, e: + except requests.exceptions.RequestException as e: log.exception('Download failed for: %s', url) # the file can be incomplete, remove if os.path.exists(dl_dest): try: os.remove(dl_dest) - except OSError, e: + except OSError as e: log.exception('Could not delete incomplete file: %s', dl_dest) raise TaskotronRemoteError(e, errno=e.response.status_code) diff --git a/libtaskotron/image_utils.py b/libtaskotron/image_utils.py index 60a01b6..01f21b8 100644 --- a/libtaskotron/image_utils.py +++ b/libtaskotron/image_utils.py @@ -12,7 +12,7 @@ from libtaskotron.logger import log try: from libtaskotron.ext.fedora import rpm_utils -except ImportError, e: +except ImportError as e: raise exc.TaskotronImportError(e) From de533d6b66816fae2b64a4ff8ae4936dd6eb45ad Mon Sep 17 00:00:00 2001 From: Miro Hrončok Date: May 08 2018 08:48:04 +0000 Subject: [PATCH 3/16] Python 3: Fix print invocation python-modernize -wnf libmodernize.fixes.fix_print Manually removed double (()) in data/ansible/module_utils/koji.py. https://portingguide.readthedocs.io/en/latest/builtins.html#print-function --- diff --git a/data/ansible/module_utils/koji.py b/data/ansible/module_utils/koji.py index 0df30fe..da0b207 100644 --- a/data/ansible/module_utils/koji.py +++ b/data/ansible/module_utils/koji.py @@ -1,3 +1,4 @@ +from __future__ import print_function import ast from libtaskotron.ext.fedora.koji_utils import KojiClient from libtaskotron.ext.fedora import rpm_utils diff --git a/docs/generate_directive_docs.py b/docs/generate_directive_docs.py index dd3f2c7..d67e894 100644 --- a/docs/generate_directive_docs.py +++ b/docs/generate_directive_docs.py @@ -17,6 +17,7 @@ # along with Ansible. If not, see . # +from __future__ import print_function import os import glob import sys @@ -108,7 +109,7 @@ def write_data(text, options, outputname, module): f.write(text.encode('utf-8')) f.close() else: - print text + print(text) def list_modules(module_dir): @@ -241,7 +242,7 @@ def process_module(module, options, env, template, outputname, module_map): if os.path.basename(fname)[-3:] != '.py': return - print "rendering: %s" % module + print("rendering: %s" % module) doc, examples = get_docstring(fname, verbose=options.verbose) @@ -337,8 +338,8 @@ def process_category(category, categories, options, env, template, outputname): category_file_path = os.path.join(options.output_dir, "list_of_%s_modules.rst" % category) category_file = open(category_file_path, "w") - print "*** recording category %s in %s ***" \ - % (category, category_file_path) + print("*** recording category %s in %s ***" \ + % (category, category_file_path)) category = category.replace("_", " ") category = category.title() @@ -445,14 +446,14 @@ def validate_options(options): ''' validate option parser options ''' if not options.module_dir: - print >>sys.stderr, "--module-dir is required" + print("--module-dir is required", file=sys.stderr) sys.exit(1) if not os.path.exists(options.module_dir): - print >>sys.stderr, "--module-dir does not exist: %s" \ - % options.module_dir + print("--module-dir does not exist: %s" \ + % options.module_dir, file=sys.stderr) sys.exit(1) if not options.template_dir: - print "--template-dir must be specified" + print("--template-dir must be specified") sys.exit(1) @@ -522,7 +523,7 @@ def get_docstring(filename, verbose=False): traceback.print_exc() # temp if verbose is True: traceback.print_exc() - print "unable to parse %s" % filename + print("unable to parse %s" % filename) return doc, plainexamples if __name__ == '__main__': diff --git a/dodo.py b/dodo.py index bc90142..e2ab324 100644 --- a/dodo.py +++ b/dodo.py @@ -1,6 +1,7 @@ #! /usr/bin/env python # encoding: utf-8 +from __future__ import print_function import os import codecs import re diff --git a/libtaskotron/check.py b/libtaskotron/check.py index 7f58036..aa0bf9a 100644 --- a/libtaskotron/check.py +++ b/libtaskotron/check.py @@ -6,6 +6,7 @@ '''Helper tools for managing check status, outcome, and output.''' from __future__ import absolute_import +from __future__ import print_function import pprint import collections import yaml @@ -170,7 +171,7 @@ class CheckDetail(object): self.output.append(message) if printout: - print message + print(message) @classmethod diff --git a/libtaskotron/os_utils.py b/libtaskotron/os_utils.py index bb99978..6bc36ef 100644 --- a/libtaskotron/os_utils.py +++ b/libtaskotron/os_utils.py @@ -6,6 +6,7 @@ ''' Utility methods related to an operating system ''' from __future__ import absolute_import +from __future__ import print_function import os import subprocess @@ -71,7 +72,7 @@ def popen_rt(cmd, stderr=subprocess.STDOUT, bufsize=1, **kwargs): proc = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=stderr, bufsize=bufsize, **kwargs) output = [] for line in iter(proc.stdout.readline, ''): - print line, + print(line, end=' ') output.append(line) (stdoutdata, stderrdata) = proc.communicate() diff --git a/testing/test_config.py b/testing/test_config.py index 14dae1d..4020b47 100644 --- a/testing/test_config.py +++ b/testing/test_config.py @@ -5,6 +5,7 @@ '''Unit tests for libtaskotron/config.py''' +from __future__ import print_function import os import pytest import mock @@ -39,7 +40,7 @@ class TestConfig(object): '''By default we should have a testing profile''' assert os.getenv(config.PROFILE_VAR) == config.ProfileName.TESTING conf = config.get_config() - print conf + print(conf) assert conf.profile == config.ProfileName.TESTING assert isinstance(conf, config.TestingConfig) @@ -54,7 +55,7 @@ class TestConfig(object): def test_load_defaults(self): '''Test _load_defaults() function''' for attr, value in vars(config.ProfileName).items(): - print attr, value + print(attr, value) if attr.startswith('_'): continue conf = config._load_defaults(value) From bf97bc739e760d1d78ddb3f30759af5e1697e57c Mon Sep 17 00:00:00 2001 From: Miro Hrončok Date: May 08 2018 09:03:20 +0000 Subject: [PATCH 4/16] Python 3: Deal with moved imports * StringIO - use io.StringIO with unicode literals everywwhere * ConfigParser - try/except the import * urrlib(2) - try/except the import --- diff --git a/libtaskotron/ext/fedora/yumrepoinfo.py b/libtaskotron/ext/fedora/yumrepoinfo.py index 457ddf8..00314ec 100644 --- a/libtaskotron/ext/fedora/yumrepoinfo.py +++ b/libtaskotron/ext/fedora/yumrepoinfo.py @@ -6,11 +6,17 @@ '''A wrapper object for yumrepoinfo.conf to access its information easily''' from __future__ import absolute_import -import ConfigParser import os -import urllib2 import socket +try: + import configparser + from urllib.request import urlopen + from urllib.error import URLError +except ImportError: + import ConfigParser as configparser + from urllib2 import urlopen, URLError + from libtaskotron import config from libtaskotron.logger import log from libtaskotron import arch_utils @@ -75,7 +81,7 @@ class YumRepoInfo(object): [os.path.join(confdir, 'yumrepoinfo.conf') for confdir in config.CONF_DIRS]) self.resolve_retry = resolve_retry - self.parser = ConfigParser.SafeConfigParser(defaults= + self.parser = configparser.SafeConfigParser(defaults= {'arch': self.arch}) if not self.filelist: @@ -145,7 +151,7 @@ class YumRepoInfo(object): ''' try: return self.parser.get(reponame, key) - except ConfigParser.Error as e: + except configparser.Error as e: raise exc.TaskotronConfigError("Can't retrieve key '%s' from repo " "'%s': %s" % (key, reponame, e)) diff --git a/testing/functest_yumrepoinfo_directive.py b/testing/functest_yumrepoinfo_directive.py index 2488277..303d28c 100644 --- a/testing/functest_yumrepoinfo_directive.py +++ b/testing/functest_yumrepoinfo_directive.py @@ -5,8 +5,7 @@ import pytest import mock -import StringIO -import mock +from io import StringIO from libtaskotron.directives import yumrepoinfo_directive from libtaskotron.exceptions import TaskotronDirectiveError @@ -81,7 +80,7 @@ class TestYumrepoinfoDirective(object): yumrepoinfo object instead of falling back to the default system arch""" ref_arch = 'i386' repoinfo = yumrepoinfo.YumRepoInfo(filelist=[], arch='x86_64') - repoinfo.parser.readfp(StringIO.StringIO(TEST_CONF)) + repoinfo.parser.readfp(StringIO(TEST_CONF)) stub_getrepoinfo = mock.MagicMock(return_value=repoinfo) monkeypatch.setattr(yumrepoinfo, 'get_yumrepoinfo', stub_getrepoinfo) diff --git a/testing/test_config.py b/testing/test_config.py index 4020b47..f04830b 100644 --- a/testing/test_config.py +++ b/testing/test_config.py @@ -9,7 +9,8 @@ from __future__ import print_function import os import pytest import mock -from StringIO import StringIO +from io import StringIO + from libtaskotron import config from libtaskotron import exceptions as exc @@ -68,13 +69,13 @@ class TestConfig(object): def test_load_file_empty(self): '''Test _load_file() function with empty file''' - contents = StringIO('') + contents = StringIO(u'') conf_object = config._load_file(contents) assert conf_object == {} def test_load_file_commented(self): '''Test _load_file() function with fully commented out file''' - contents = StringIO(''' + contents = StringIO(u''' # first commented line # second: line # last line @@ -84,7 +85,7 @@ class TestConfig(object): def test_load_file_options(self): '''Test _load_file() function with some options present''' - contents = StringIO('''# a header comment + contents = StringIO(u'''# a header comment option1: value1 # option2: value2 option3: 15 @@ -99,22 +100,22 @@ option5: False''') def test_load_file_invalid_syntax(self): '''Test _load_file() function with invalid syntax''' with pytest.raises(exc.TaskotronConfigError): - contents = StringIO('a single string (not dict)') + contents = StringIO(u'a single string (not dict)') config._load_file(contents) with pytest.raises(exc.TaskotronConfigError): - contents = StringIO('tab:\t #this is invalid in YAML') + contents = StringIO(u'tab:\t #this is invalid in YAML') config._load_file(contents) def test_load_file_invalid_type(self): '''Test _load_file() function with invalid option type''' # tmpdir is string, with string it should pass - contents = StringIO('tmpdir: foo') + contents = StringIO(u'tmpdir: foo') config._load_file(contents) # but with anything else, it should fail with pytest.raises(exc.TaskotronConfigError): - contents = StringIO('tmpdir: False') + contents = StringIO(u'tmpdir: False') config._load_file(contents) def test_merge_config(self): diff --git a/testing/test_yumrepoinfo.py b/testing/test_yumrepoinfo.py index dc38218..8e4364a 100644 --- a/testing/test_yumrepoinfo.py +++ b/testing/test_yumrepoinfo.py @@ -6,15 +6,19 @@ '''Unit tests for libtaskotron/yumrepoinfo.py''' import pytest -import StringIO -import ConfigParser import mock +from io import StringIO + +try: + import configparser +except ImportError: + import ConfigParser as configparser from libtaskotron.ext.fedora import yumrepoinfo from libtaskotron import exceptions as exc -TEST_CONF='''\ +TEST_CONF=u'''\ [DEFAULT] baseurl = http://download.fedoraproject.org/pub/fedora/linux baseurl_altarch = http://download.fedoraproject.org/pub/fedora-secondary @@ -70,14 +74,14 @@ class TestYumRepoInfo(object): monkeypatch.setattr(yumrepoinfo.YumRepoInfo, '_switch_to_mirror', mock.Mock()) # create YumRepoInfo initialized with TEST_CONF self.repoinfo = yumrepoinfo.YumRepoInfo(filelist=[]) - self.repoinfo.parser.readfp(StringIO.StringIO(TEST_CONF)) + self.repoinfo.parser.readfp(StringIO(TEST_CONF)) @pytest.fixture def disable_disk_loading(self, monkeypatch): '''Patch ConfigParser not to load from disk.''' # instead of reading from disk, just return the input parameter - monkeypatch.setattr(ConfigParser.RawConfigParser, 'read', + monkeypatch.setattr(configparser.RawConfigParser, 'read', lambda self, cfg: cfg) @pytest.fixture @@ -88,13 +92,13 @@ class TestYumRepoInfo(object): def test_init_arch(self, disable_disk_loading): '''Test YumRepoInfo(arch=)''' repoinfo = yumrepoinfo.YumRepoInfo(arch='x86_64', filelist=[]) - repoinfo.parser.readfp(StringIO.StringIO(TEST_CONF)) + repoinfo.parser.readfp(StringIO(TEST_CONF)) assert '/x86_64/' in repoinfo.get('rawhide', 'url') def test_init_basearch(self, disable_disk_loading): '''i686 should be converted to i386''' repoinfo = yumrepoinfo.YumRepoInfo(arch='i686', filelist=[]) - repoinfo.parser.readfp(StringIO.StringIO(TEST_CONF)) + repoinfo.parser.readfp(StringIO(TEST_CONF)) assert '/i386/' in repoinfo.get('rawhide', 'url') def test_init_empty_filelist(self, monkeypatch): @@ -102,7 +106,7 @@ class TestYumRepoInfo(object): # make sure the disk is not read def _raise_alarm(x): assert False, 'This must not be called' - monkeypatch.setattr(ConfigParser.RawConfigParser, 'read', _raise_alarm) + monkeypatch.setattr(configparser.RawConfigParser, 'read', _raise_alarm) repoinfo = yumrepoinfo.YumRepoInfo(filelist=[]) assert not repoinfo.repos() @@ -168,14 +172,14 @@ class TestYumRepoInfo(object): def test_top_parent_raise(self): '''top_parent() must detect an infinite parent cycle''' - cfg = '''\ + cfg = u'''\ [repo1] parent = repo2 [repo2] parent = repo1 ''' repoinfo = yumrepoinfo.YumRepoInfo(filelist=[]) - repoinfo.parser.readfp(StringIO.StringIO(cfg)) + repoinfo.parser.readfp(StringIO(cfg)) with pytest.raises(exc.TaskotronConfigError): repoinfo.top_parent('repo1') From d834a2afe810237310cbbaa7d01c7d0286169078 Mon Sep 17 00:00:00 2001 From: Miro Hrončok Date: May 08 2018 09:06:26 +0000 Subject: [PATCH 5/16] Python 3: Convert dict values to list explicitly --- diff --git a/libtaskotron/arch_utils.py b/libtaskotron/arch_utils.py index d95b5aa..9f44874 100644 --- a/libtaskotron/arch_utils.py +++ b/libtaskotron/arch_utils.py @@ -31,7 +31,7 @@ class Arches(object): meta = ['noarch', 'src'] #: all known architectures - known = sorted(set(binary.keys() + list(itertools.chain(*binary.values())) + meta)) + known = sorted(set(list(binary.keys()) + list(itertools.chain(*binary.values())) + meta)) def basearch(arch=None): From 0049e0c369cd6c3a4fe9819e17fb73a78764db8a Mon Sep 17 00:00:00 2001 From: Miro Hrončok Date: May 08 2018 09:09:49 +0000 Subject: [PATCH 6/16] Python 3: Use relative import where needed --- diff --git a/testing/functest_koji_utils.py b/testing/functest_koji_utils.py index 3b0781a..4daca7f 100644 --- a/testing/functest_koji_utils.py +++ b/testing/functest_koji_utils.py @@ -11,7 +11,7 @@ import os from libtaskotron.ext.fedora import koji_utils from libtaskotron import file_utils -from test_file_utils import mock_download +from .test_file_utils import mock_download class TestKojiClient(): diff --git a/testing/functest_yumrepoinfo.py b/testing/functest_yumrepoinfo.py index fdd93cd..baf4495 100644 --- a/testing/functest_yumrepoinfo.py +++ b/testing/functest_yumrepoinfo.py @@ -11,7 +11,7 @@ import mock from libtaskotron.ext.fedora import yumrepoinfo from libtaskotron import exceptions as exc -from test_yumrepoinfo import TEST_CONF +from .test_yumrepoinfo import TEST_CONF @pytest.mark.usefixtures('setup') diff --git a/testing/functest_yumrepoinfo_directive.py b/testing/functest_yumrepoinfo_directive.py index 303d28c..cbb8f60 100644 --- a/testing/functest_yumrepoinfo_directive.py +++ b/testing/functest_yumrepoinfo_directive.py @@ -12,7 +12,7 @@ from libtaskotron.exceptions import TaskotronDirectiveError from libtaskotron.ext.fedora import yumrepoinfo -from test_yumrepoinfo import TEST_CONF +from .test_yumrepoinfo import TEST_CONF class TestYumrepoinfoDirective(object): diff --git a/testing/test_koji_utils.py b/testing/test_koji_utils.py index f63b942..0f6008f 100644 --- a/testing/test_koji_utils.py +++ b/testing/test_koji_utils.py @@ -17,7 +17,7 @@ from libtaskotron import config from libtaskotron import file_utils from libtaskotron import arch_utils -from test_file_utils import mock_download +from .test_file_utils import mock_download # http://stackoverflow.com/questions/3190706/nonlocal-keyword-in-python-2-x def create_multicall(first, second): From 472101e67677b4d648b42ad7825af90eb86f10f4 Mon Sep 17 00:00:00 2001 From: Miro Hrončok Date: May 08 2018 09:17:51 +0000 Subject: [PATCH 7/16] Python 3: Define basestring in python_utils --- diff --git a/libtaskotron/check.py b/libtaskotron/check.py index aa0bf9a..e3ef8ad 100644 --- a/libtaskotron/check.py +++ b/libtaskotron/check.py @@ -12,6 +12,7 @@ import collections import yaml from . import python_utils +from .python_utils import basestring from . import exceptions as exc from .logger import log diff --git a/libtaskotron/config.py b/libtaskotron/config.py index 3c86cbc..edf17fa 100644 --- a/libtaskotron/config.py +++ b/libtaskotron/config.py @@ -17,6 +17,7 @@ from libtaskotron.logger import log from libtaskotron.config_defaults import (Config, ProductionConfig, TestingConfig, ProfileName, RuntaskModeName) from libtaskotron import file_utils +from libtaskotron.python_utils import basestring CONF_DIRS = [ # local checkout dir first, then system wide dir os.path.abspath(os.path.dirname(libtaskotron.__file__) + '/../conf'), diff --git a/libtaskotron/directives/bodhi_directive.py b/libtaskotron/directives/bodhi_directive.py index c80bf52..02998eb 100644 --- a/libtaskotron/directives/bodhi_directive.py +++ b/libtaskotron/directives/bodhi_directive.py @@ -10,6 +10,7 @@ from libtaskotron.logger import log from libtaskotron.exceptions import TaskotronDirectiveError import libtaskotron.ext.fedora.bodhi_utils as bodhi from libtaskotron.ext.fedora.koji_utils import KojiClient +from libtaskotron.python_utils import basestring from libtaskotron import config from libtaskotron import exceptions as exc diff --git a/libtaskotron/directives/koji_directive.py b/libtaskotron/directives/koji_directive.py index 229d192..c4e8d79 100644 --- a/libtaskotron/directives/koji_directive.py +++ b/libtaskotron/directives/koji_directive.py @@ -10,6 +10,7 @@ import libtaskotron.exceptions as exc from libtaskotron.ext.fedora.koji_utils import KojiClient from libtaskotron.ext.fedora import rpm_utils from libtaskotron.logger import log +from libtaskotron.python_utils import basestring from libtaskotron import file_utils DOCUMENTATION = """ diff --git a/libtaskotron/directives/yumrepoinfo_directive.py b/libtaskotron/directives/yumrepoinfo_directive.py index 1d1500f..552d9a8 100644 --- a/libtaskotron/directives/yumrepoinfo_directive.py +++ b/libtaskotron/directives/yumrepoinfo_directive.py @@ -5,6 +5,8 @@ from __future__ import absolute_import +from libtaskotron.python_utils import basestring + DOCUMENTATION = """ module: yumrepoinfo_directive short_description: translate Koji tags into YUM repository URLs diff --git a/libtaskotron/ext/fedora/bodhi_utils.py b/libtaskotron/ext/fedora/bodhi_utils.py index 0065268..9df8ff9 100644 --- a/libtaskotron/ext/fedora/bodhi_utils.py +++ b/libtaskotron/ext/fedora/bodhi_utils.py @@ -12,6 +12,7 @@ from libtaskotron import config from libtaskotron import exceptions as exc from libtaskotron.logger import log from libtaskotron import python_utils +from libtaskotron.python_utils import basestring from . import rpm_utils diff --git a/libtaskotron/python_utils.py b/libtaskotron/python_utils.py index 825ea9f..5cb206a 100644 --- a/libtaskotron/python_utils.py +++ b/libtaskotron/python_utils.py @@ -10,6 +10,10 @@ import collections import libtaskotron.exceptions as exc +try: + basestring = basestring +except NameError: + basestring = str def iterable(obj, item_type=None): '''Decide whether ``obj`` is an :class:`~collections.Iterable` (you can diff --git a/testing/test_python_utils.py b/testing/test_python_utils.py index 20837b5..4286922 100644 --- a/testing/test_python_utils.py +++ b/testing/test_python_utils.py @@ -9,6 +9,7 @@ import pytest from libtaskotron import python_utils import libtaskotron.exceptions as exc +from libtaskotron.python_utils import basestring class TestCollectionOf: From 4653222e360b19a7ba7ec868cf544c8076809a70 Mon Sep 17 00:00:00 2001 From: Miro Hrončok Date: May 08 2018 09:30:11 +0000 Subject: [PATCH 8/16] Python 3: Drop iteritems() This uses items() everywhere and is a bit less effective on Python 2, however I think the readability counts more. sed -i 's/iteritems/items/' --- diff --git a/docs/generate_directive_docs.py b/docs/generate_directive_docs.py index d67e894..664ad80 100644 --- a/docs/generate_directive_docs.py +++ b/docs/generate_directive_docs.py @@ -284,7 +284,7 @@ def process_module(module, options, env, template, outputname, module_map): del doc['version_added'] if 'parameters' in doc: - for (k, v) in doc['parameters'].iteritems(): + for (k, v) in doc['parameters'].items(): all_keys.append(k) all_keys = sorted(all_keys) doc['option_keys'] = all_keys diff --git a/libtaskotron/check.py b/libtaskotron/check.py index e3ef8ad..ae2ddeb 100644 --- a/libtaskotron/check.py +++ b/libtaskotron/check.py @@ -321,7 +321,7 @@ def export_YAML(check_details): data['checkname'] = detail.checkname if detail._internal: data['_internal'] = detail._internal - for key, value in detail.keyvals.iteritems(): + for key, value in detail.keyvals.items(): if key in RESERVED_KEYS: log.warn("Reserved key '%s' found in keyvals. Ignoring for " "export purposes.", key) diff --git a/libtaskotron/directives/resultsdb_directive.py b/libtaskotron/directives/resultsdb_directive.py index ed95561..469f2ad 100644 --- a/libtaskotron/directives/resultsdb_directive.py +++ b/libtaskotron/directives/resultsdb_directive.py @@ -209,7 +209,7 @@ class ResultsdbDirective(BaseDirective): log.debug('Namespace %s found in the safe namespaces.', ns) return - for ns, repos in conf_ns['namespaces_whitelist'].iteritems(): + for ns, repos in conf_ns['namespaces_whitelist'].items(): if checkname.startswith(ns+'.'): log.debug('Namespace %s found in the namespace whitelist.', ns) ns_repos = repos From 3f1404cbb22704451efb737e9b98e699417bfd9a Mon Sep 17 00:00:00 2001 From: Miro Hrončok Date: May 08 2018 10:06:25 +0000 Subject: [PATCH 9/16] Python 3: Use univeral_newlines with subprocess to get text, not bytes --- diff --git a/libtaskotron/os_utils.py b/libtaskotron/os_utils.py index 6bc36ef..3f3e1bf 100644 --- a/libtaskotron/os_utils.py +++ b/libtaskotron/os_utils.py @@ -69,7 +69,7 @@ def popen_rt(cmd, stderr=subprocess.STDOUT, bufsize=1, **kwargs): if 'stdout' in kwargs: raise exc.TaskotronValueError('stdout parameter not allowed, it will be overridden') - proc = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=stderr, bufsize=bufsize, **kwargs) + proc = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=stderr, bufsize=bufsize, universal_newlines=True, **kwargs) output = [] for line in iter(proc.stdout.readline, ''): print(line, end=' ') From 10a7768f302ddd8294ceb1bbf7f74e54a710d9d4 Mon Sep 17 00:00:00 2001 From: Miro Hrončok Date: May 08 2018 10:44:58 +0000 Subject: [PATCH 10/16] Python 3: Add SectionNameInterpolation for ConfigParser This is a bit hacky, for context see https://stackoverflow.com/a/43415907/1839451 and https://stackoverflow.com/a/47239445/1839451 --- diff --git a/libtaskotron/ext/fedora/yumrepoinfo.py b/libtaskotron/ext/fedora/yumrepoinfo.py index 00314ec..c5bb125 100644 --- a/libtaskotron/ext/fedora/yumrepoinfo.py +++ b/libtaskotron/ext/fedora/yumrepoinfo.py @@ -49,6 +49,26 @@ def get_yumrepoinfo(arch=None, filelist=None): return _yumrepoinfo[arch] +try: + class SectionNameInterpolation(configparser.BasicInterpolation): + '''In order to make ConfigParser work like it once used to work + we add this crazy hack inspired by this Stack Overflow answer + https://stackoverflow.com/a/43415907/1839451''' + def before_get(self, parser, section, option, value, defaults): + value = value.replace('%(__name__)s', section) + return super().before_get(parser, section, option, value, defaults) + + class YumRepoInfoConfigParser(configparser.SafeConfigParser): + '''A SafeConfigParser with above SectionNameInterpolation''' + def __init__(self, *args, **kwargs): + kwargs['interpolation'] = SectionNameInterpolation() + super().__init__(*args, **kwargs) +except AttributeError: + # On Python 2, interpolation is hardcoded and the above doesn't work + # The behavior however is consistent with the above + YumRepoInfoConfigParser = configparser.SafeConfigParser + + class YumRepoInfo(object): '''This class is a wrapper for easily accessing repoinfo.conf file.''' @@ -81,8 +101,7 @@ class YumRepoInfo(object): [os.path.join(confdir, 'yumrepoinfo.conf') for confdir in config.CONF_DIRS]) self.resolve_retry = resolve_retry - self.parser = configparser.SafeConfigParser(defaults= - {'arch': self.arch}) + self.parser = YumRepoInfoConfigParser(defaults={'arch': self.arch}) if not self.filelist: # no data should be loaded From b768d474e6571f3374f0b9817f297299eb61e688 Mon Sep 17 00:00:00 2001 From: Miro Hrončok Date: May 08 2018 10:51:29 +0000 Subject: [PATCH 11/16] Python 3: Add cmp implementation to the utility library --- diff --git a/libtaskotron/check.py b/libtaskotron/check.py index ae2ddeb..e70cb7f 100644 --- a/libtaskotron/check.py +++ b/libtaskotron/check.py @@ -12,7 +12,7 @@ import collections import yaml from . import python_utils -from .python_utils import basestring +from .python_utils import basestring, cmp from . import exceptions as exc from .logger import log diff --git a/libtaskotron/python_utils.py b/libtaskotron/python_utils.py index 5cb206a..027547e 100644 --- a/libtaskotron/python_utils.py +++ b/libtaskotron/python_utils.py @@ -121,3 +121,17 @@ def reverse_argparse(args, ignore=()): cmdline.extend(segment) return cmdline + + +def cmp(x, y): + """ + Replacement for built-in function cmp that was removed in Python 3 + + Compare the two objects x and y and return an integer according to + the outcome. The return value is negative if x < y, zero if x == y + and strictly positive if x > y. + + https://portingguide.readthedocs.io/en/latest/comparisons.html#the-cmp-function + """ + + return (x > y) - (x < y) From 540a61d595c053b669a4aa37ef81111d31c43322 Mon Sep 17 00:00:00 2001 From: Miro Hrončok Date: May 08 2018 10:59:58 +0000 Subject: [PATCH 12/16] Python 3: Mock open differently on Python 2 and 3 --- diff --git a/testing/test_config.py b/testing/test_config.py index f04830b..7b05288 100644 --- a/testing/test_config.py +++ b/testing/test_config.py @@ -7,6 +7,7 @@ from __future__ import print_function import os +import sys import pytest import mock from io import StringIO @@ -15,6 +16,10 @@ from libtaskotron import config from libtaskotron import exceptions as exc +PYVER = sys.version_info.major +builtin_open = '__builtin__.open' if PYVER < 3 else 'builtins.open' + + @pytest.mark.usefixtures('setup') class TestConfig(object): @@ -176,7 +181,7 @@ option5: False''') file_data = '%' mocked_open = mock.mock_open(read_data=file_data) - with mock.patch('__builtin__.open', mocked_open): + with mock.patch(builtin_open, mocked_open): with pytest.raises(exc.TaskotronConfigError): config.parse_yaml_from_file('mocked_filename') @@ -184,7 +189,7 @@ option5: False''') file_data = ':' mocked_open = mock.mock_open(read_data=file_data) - with mock.patch('__builtin__.open', mocked_open): + with mock.patch(builtin_open, mocked_open): with pytest.raises(exc.TaskotronConfigError): config.parse_yaml_from_file('mocked_filename') diff --git a/testing/test_taskotron_result.py b/testing/test_taskotron_result.py index a84ec40..a84e9e5 100644 --- a/testing/test_taskotron_result.py +++ b/testing/test_taskotron_result.py @@ -7,8 +7,12 @@ from libtaskotron import taskotron_result from libtaskotron.check import CheckDetail, export_YAML, import_YAML import mock import os +import sys import argparse +PYVER = sys.version_info.major +builtin_open = '__builtin__.open' if PYVER < 3 else 'builtins.open' + OUTPUT = "" FILE = """results: @@ -36,7 +40,7 @@ class TestTaskotronResult(object): stub_file = mock.MagicMock() stub_file.write = save_output - with mock.patch('__builtin__.open', mock.MagicMock(return_value=stub_file), create=True): + with mock.patch(builtin_open, mock.MagicMock(return_value=stub_file), create=True): taskotron_result.main() assert OUTPUT == export_YAML(CheckDetail(**vars(test_args))) @@ -56,7 +60,7 @@ class TestTaskotronResult(object): monkeypatch.setattr(os.path, 'isfile', mock.MagicMock(return_value=True)) - with mock.patch('__builtin__.open', mock.MagicMock(return_value=stub_file), create=True): + with mock.patch(builtin_open, mock.MagicMock(return_value=stub_file), create=True): taskotron_result.main() assert OUTPUT == export_YAML(import_YAML(FILE)+[CheckDetail(**vars(test_args))]) @@ -73,7 +77,7 @@ class TestTaskotronResult(object): stub_file = mock.MagicMock() stub_file.write = save_output - with mock.patch('__builtin__.open', mock.MagicMock(return_value=stub_file), create=True): + with mock.patch(builtin_open, mock.MagicMock(return_value=stub_file), create=True): taskotron_result.main() assert OUTPUT == export_YAML(CheckDetail(**vars(test_args))) From aa1ad984f2082df4d04a95a719388f199f6da7f6 Mon Sep 17 00:00:00 2001 From: Miro Hrončok Date: May 08 2018 11:10:20 +0000 Subject: [PATCH 13/16] Python 3: Add automatically converted sortkeyNEVR from cmpNEVR Use it when sorting things --- diff --git a/libtaskotron/ext/fedora/koji_utils.py b/libtaskotron/ext/fedora/koji_utils.py index ba7a0f4..4056bd9 100644 --- a/libtaskotron/ext/fedora/koji_utils.py +++ b/libtaskotron/ext/fedora/koji_utils.py @@ -73,7 +73,7 @@ class KojiClient(object): assert len(build[0]) <= 1, 'More than one build returned with latest=True' nvrs.append(build[0][0]['nvr']) - sorted_nvrs = sorted(nvrs, cmp=rpm_utils.cmpNEVR, reverse=True) + sorted_nvrs = sorted(nvrs, key=rpm_utils.sortkeyNEVR, reverse=True) if sorted_nvrs: return sorted_nvrs[0] diff --git a/libtaskotron/ext/fedora/rpm_utils.py b/libtaskotron/ext/fedora/rpm_utils.py index 971d33f..cab8c22 100644 --- a/libtaskotron/ext/fedora/rpm_utils.py +++ b/libtaskotron/ext/fedora/rpm_utils.py @@ -6,6 +6,7 @@ ''' Utility methods related to RPM ''' from __future__ import absolute_import +import functools import re import hawkey import subprocess @@ -105,6 +106,9 @@ def cmpNEVR(nevr1, nevr2): return int(rpmver1.evr_cmp(rpmver2, sack)) +sortkeyNEVR = functools.cmp_to_key(cmpNEVR) + + def install(pkgs): '''Install packages from system repositories using DNF. Either root or sudo access required. From acd2a4fee25f11ae28960a9fdfc739cc4f0e4796 Mon Sep 17 00:00:00 2001 From: Miro Hrončok Date: May 08 2018 11:18:26 +0000 Subject: [PATCH 14/16] Python 3: Remove unicode type specific tests --- diff --git a/testing/test_python_utils.py b/testing/test_python_utils.py index 4286922..6cba9bb 100644 --- a/testing/test_python_utils.py +++ b/testing/test_python_utils.py @@ -28,7 +28,11 @@ class TestCollectionOf: def test_not_iterable(self): assert not python_utils.iterable('a') assert not python_utils.iterable(u'a') - assert not python_utils.iterable(unicode('foo')) + try: + assert not python_utils.iterable(unicode('foo')) + except NameError: + # Python 3 no such thing as unicode, str already tested + pass assert not python_utils.iterable(3) assert not python_utils.iterable(3.14) assert not python_utils.iterable(None) @@ -41,8 +45,11 @@ class TestCollectionOf: assert python_utils.iterable(['a', 'b'], str) assert python_utils.iterable(['a', 'b'], basestring) - assert not python_utils.iterable(['a', 'b'], unicode) - + try: + assert not python_utils.iterable(['a', 'b'], unicode) + except NameError: + # Python 3 no such thing as unicode, str already tested + pass assert python_utils.iterable([[], []], list) # empty classes From 49ff4255fb520a43d7eb3f9ee5c73c897aa20ae2 Mon Sep 17 00:00:00 2001 From: Miro Hrončok Date: May 08 2018 12:11:05 +0000 Subject: [PATCH 15/16] Python 3: Workaround a bug in xunitparser https://github.com/laurentb/xunitparser/issues/10 --- diff --git a/libtaskotron/directives/xunit_directive.py b/libtaskotron/directives/xunit_directive.py index 807c648..78fa15c 100644 --- a/libtaskotron/directives/xunit_directive.py +++ b/libtaskotron/directives/xunit_directive.py @@ -71,6 +71,10 @@ exports it to a variable which is then passed to ``resultsdb`` directive:: directive_class = 'XunitDirective' +# https://github.com/laurentb/xunitparser/issues/10 +xunitparser.TestSuite._cleanup = False + + class XunitDirective(BaseDirective): aggregations = ['none', 'allpass'] From 4da2b19badb153c40f471a3b366b7d9f05ed9475 Mon Sep 17 00:00:00 2001 From: Miro Hrončok Date: May 08 2018 12:13:13 +0000 Subject: [PATCH 16/16] Python 3: Add py36 tox testenv to tox.ini --- diff --git a/tox.ini b/tox.ini index 057a0e1..4e057f0 100644 --- a/tox.ini +++ b/tox.ini @@ -17,7 +17,7 @@ python_files=test_* functest_* addopts=--functional testing/ --cov-report=term-missing --cov libtaskotron [tox] -envlist = py27 +envlist = py27,py36 [testenv] deps = -rrequirements.txt