Merge ~raharper/cloud-init:cloud-test-add-pylint-and-fix into cloud-init:master

Proposed by Ryan Harper
Status: Merged
Merged at revision: a110e483e8644ab73e69853ea11b6c4c6cfa04b6
Proposed branch: ~raharper/cloud-init:cloud-test-add-pylint-and-fix
Merge into: cloud-init:master
Diff against target: 884 lines (+121/-266)
29 files modified
.pylintrc (+1/-1)
cloudinit/cmd/tests/test_clean.py (+1/-1)
cloudinit/cmd/tests/test_status.py (+1/-1)
cloudinit/tests/helpers.py (+35/-0)
dev/null (+0/-172)
tests/cloud_tests/__init__.py (+6/-0)
tests/cloud_tests/bddeb.py (+4/-5)
tests/cloud_tests/collect.py (+3/-3)
tests/cloud_tests/config.py (+3/-1)
tests/cloud_tests/testcases/base.py (+2/-1)
tests/cloud_tests/testcases/modules/set_hostname_fqdn.py (+1/-1)
tests/cloud_tests/util.py (+1/-1)
tests/unittests/test_cs_util.py (+1/-0)
tests/unittests/test_datasource/test_azure.py (+14/-17)
tests/unittests/test_datasource/test_digitalocean.py (+6/-3)
tests/unittests/test_datasource/test_ec2.py (+2/-1)
tests/unittests/test_distros/test_create_users.py (+5/-2)
tests/unittests/test_distros/test_netconfig.py (+0/-3)
tests/unittests/test_handler/test_handler_lxd.py (+0/-3)
tests/unittests/test_handler/test_handler_power_state.py (+0/-3)
tests/unittests/test_handler/test_handler_yum_add_repo.py (+2/-8)
tests/unittests/test_handler/test_handler_zypper_add_repo.py (+1/-6)
tests/unittests/test_reporting.py (+1/-1)
tests/unittests/test_templating.py (+1/-1)
tests/unittests/test_util.py (+3/-3)
tests/unittests/test_vmware_config_file.py (+2/-1)
tools/make-mime.py (+1/-1)
tools/mock-meta.py (+21/-24)
tox.ini (+3/-2)
Reviewer Review Type Date Requested Status
Scott Moser Approve
Server Team CI bot continuous-integration Approve
Joshua Powers (community) Needs Fixing
Review via email: mp+334868@code.launchpad.net

Description of the change

pylint: Update pylint to 1.7.1, run on tests/ and fix complaints.

- Update tox.ini to invoke pylint v1.7.1.
- Modify .pylintrc generated-members ignore mocked object members (m_.*)
- Replace "dangerous" params defaulting to {}
- Fix up cloud_tests use of platforms
- Cast some instance objects to with dict()
- Handle python2.7 vs 3+ ConfigParser use of readfp (deprecated)
- Update use of assertEqual(<boolean>, value) to assert<Boolean>(value)
- replace depricated assertRegexp -> assertRegex
- Remove useless test-class calls to super class
- Assign class property accessors a result and use it
- Fix missing class member in CepkoResultTests
- Fix Cheetah test import

To post a comment you must log in.
Revision history for this message
Server Team CI bot (server-team-bot) wrote :

FAILED: Continuous integration, rev:5335e1632ddabdbad9ffb011720929ae4cd8ebe3
https://jenkins.ubuntu.com/server/job/cloud-init-ci/589/
Executed test runs:
    SUCCESS: Checkout
    FAILED: Unit & Style Tests

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/589/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Joshua Powers (powersj) wrote :

Thanks for doing this! Can you add comment to description/commit message about meta-data getting added to the integration tests?

Looks like a few tip-pylint errors, one of which is why I asked about getting rid of tools/hacking.py.

Otherwise I ran a full lxd and nocloud-kvm test run with no failures:
lxd: https://paste.ubuntu.com/26128706/
nocloud-kvm: https://paste.ubuntu.com/26128704/

review: Needs Fixing
Revision history for this message
Ryan Harper (raharper) wrote :

Didn't mean to run on tools for now.

w.r.t the meta-data bits, I'll defer to Scott, he did that first bit.

On Wed, Dec 6, 2017 at 5:15 PM, Joshua Powers <email address hidden>
wrote:

> Review: Needs Fixing
>
> Thanks for doing this! Can you add comment to description/commit message
> about meta-data getting added to the integration tests?
>
> Looks like a few tip-pylint errors, one of which is why I asked about
> getting rid of tools/hacking.py.
>
> Otherwise I ran a full lxd and nocloud-kvm test run with no failures:
> lxd: https://paste.ubuntu.com/26128706/
> nocloud-kvm: https://paste.ubuntu.com/26128704/
>
>
> --
> https://code.launchpad.net/~raharper/cloud-init/+git/
> cloud-init/+merge/334868
> You are the owner of ~raharper/cloud-init:cloud-test-add-pylint-and-fix.
>

Revision history for this message
Scott Moser (smoser) wrote :

assuming c-i is happy with this, then i am too.
please do mention tools/hacking.py removal in your commit message.
it is obsolete now as we use 'hacking' module in tox.ini for flake8

Revision history for this message
Scott Moser (smoser) :
review: Approve
Revision history for this message
Server Team CI bot (server-team-bot) wrote :

FAILED: Continuous integration, rev:ca2ce2ecb149ab6b36ec525d234f513c676210d1
https://jenkins.ubuntu.com/server/job/cloud-init-ci/597/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    FAILED: MAAS Compatability Testing

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/597/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :

FAILED: Continuous integration, rev:abb799ecc8827ac1f84012234e53fb002b70bab4
https://jenkins.ubuntu.com/server/job/cloud-init-ci/598/
Executed test runs:
    SUCCESS: Checkout
    FAILED: Unit & Style Tests

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/598/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Server Team CI bot (server-team-bot) wrote :

PASSED: Continuous integration, rev:d265c4416eda1345cd1bca6c0784226aa1d2472e
https://jenkins.ubuntu.com/server/job/cloud-init-ci/599/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    SUCCESS: MAAS Compatability Testing
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/599/rebuild

review: Approve (continuous-integration)
Revision history for this message
Scott Moser (smoser) wrote :

locally i've been through this locally
 tox && ./tools/run-centos 6 --rpm --srpm --unittest &&
   ./tools/run-centos 7 --rpm --srpm --unittest

And c-i is just about to agree
 https://jenkins.ubuntu.com/server/job/cloud-init-ci/599/console

I am making one change though. i'm going to move the '_tricky' bit for assertRaisesRegex back to the bottom of the file. it seems to fit better there with the other bits that are already there. (and i've tested the above).

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/.pylintrc b/.pylintrc
2index b160ce7..3ad3692 100644
3--- a/.pylintrc
4+++ b/.pylintrc
5@@ -56,5 +56,5 @@ ignored-classes=optparse.Values,thread._local
6 # List of members which are set dynamically and missed by pylint inference
7 # system, and so shouldn't trigger E1101 when accessed. Python regular
8 # expressions are accepted.
9-generated-members=types,http.client,command_handlers
10+generated-members=types,http.client,command_handlers,m_.*
11
12diff --git a/cloudinit/cmd/tests/test_clean.py b/cloudinit/cmd/tests/test_clean.py
13index af438aa..1379740 100644
14--- a/cloudinit/cmd/tests/test_clean.py
15+++ b/cloudinit/cmd/tests/test_clean.py
16@@ -151,7 +151,7 @@ class TestClean(CiTestCase):
17 'sys.argv': {'new': ['clean', '--logs']}},
18 clean.main)
19
20- self.assertEqual(0, context_manager.exception.code)
21+ self.assertRaisesCodeEqual(0, context_manager.exception.code)
22 self.assertFalse(
23 os.path.exists(self.log1), 'Unexpected log {0}'.format(self.log1))
24
25diff --git a/cloudinit/cmd/tests/test_status.py b/cloudinit/cmd/tests/test_status.py
26index 8ec9b5b..6d4a11e 100644
27--- a/cloudinit/cmd/tests/test_status.py
28+++ b/cloudinit/cmd/tests/test_status.py
29@@ -347,7 +347,7 @@ class TestStatus(CiTestCase):
30 '_is_cloudinit_disabled': (False, ''),
31 'Init': {'side_effect': self.init_class}},
32 status.main)
33- self.assertEqual(0, context_manager.exception.code)
34+ self.assertRaisesCodeEqual(0, context_manager.exception.code)
35 self.assertEqual('status: running\n', m_stdout.getvalue())
36
37 # vi: ts=4 expandtab syntax=python
38diff --git a/cloudinit/tests/helpers.py b/cloudinit/tests/helpers.py
39index feb884a..f3dc779 100644
40--- a/cloudinit/tests/helpers.py
41+++ b/cloudinit/tests/helpers.py
42@@ -19,9 +19,22 @@ try:
43 except ImportError:
44 from contextlib2 import ExitStack
45
46+try:
47+ from configparser import ConfigParser
48+except ImportError:
49+ from ConfigParser import ConfigParser
50+
51 from cloudinit import helpers as ch
52 from cloudinit import util
53
54+# older unittest2.TestCase (centos6) do not have assertRaisesRegex
55+# And setting assertRaisesRegex to assertRaisesRegexp causes
56+# https://github.com/PyCQA/pylint/issues/1653 . So the workaround.
57+if not hasattr(unittest2.TestCase, 'assertRaisesRegex'):
58+ def _tricky(*args, **kwargs):
59+ return unittest2.TestCase.assertRaisesRegexp
60+ unittest2.TestCase.assertRaisesRegex = _tricky
61+
62 # Used for skipping tests
63 SkipTest = unittest2.SkipTest
64
65@@ -113,6 +126,16 @@ class TestCase(unittest2.TestCase):
66 self.addCleanup(m.stop)
67 setattr(self, attr, p)
68
69+ # prefer python3 read_file over readfp but allow fallback
70+ def parse_and_read(self, contents):
71+ parser = ConfigParser()
72+ if hasattr(parser, 'read_file'):
73+ parser.read_file(contents)
74+ elif hasattr(parser, 'readfp'):
75+ # pylint: disable=W1505
76+ parser.readfp(contents)
77+ return parser
78+
79
80 class CiTestCase(TestCase):
81 """This is the preferred test case base class unless user
82@@ -158,6 +181,18 @@ class CiTestCase(TestCase):
83 dir = self.tmp_dir()
84 return os.path.normpath(os.path.abspath(os.path.join(dir, path)))
85
86+ def assertRaisesCodeEqual(self, expected, found):
87+ """Handle centos6 having different context manager for assertRaises.
88+ with assertRaises(Exception) as e:
89+ raise Exception("BOO")
90+
91+ centos6 will have e.exception as an integer.
92+ anything nwere will have it as something with a '.code'"""
93+ if isinstance(found, int):
94+ self.assertEqual(expected, found)
95+ else:
96+ self.assertEqual(expected, found.code)
97+
98
99 class ResourceUsingTestCase(CiTestCase):
100
101diff --git a/tests/cloud_tests/__init__.py b/tests/cloud_tests/__init__.py
102index 98c1d6c..dd43698 100644
103--- a/tests/cloud_tests/__init__.py
104+++ b/tests/cloud_tests/__init__.py
105@@ -10,6 +10,12 @@ TESTCASES_DIR = os.path.join(BASE_DIR, 'testcases')
106 TEST_CONF_DIR = os.path.join(BASE_DIR, 'testcases')
107 TREE_BASE = os.sep.join(BASE_DIR.split(os.sep)[:-2])
108
109+# This domain contains reverse lookups for hostnames that are used.
110+# The primary reason is so sudo will return quickly when it attempts
111+# to look up the hostname. i9n is just short for 'integration'.
112+# see also bug 1730744 for why we had to do this.
113+CI_DOMAIN = "i9n.cloud-init.io"
114+
115
116 def _initialize_logging():
117 """Configure logging for cloud_tests."""
118diff --git a/tests/cloud_tests/bddeb.py b/tests/cloud_tests/bddeb.py
119index c259dfe..a6d5069 100644
120--- a/tests/cloud_tests/bddeb.py
121+++ b/tests/cloud_tests/bddeb.py
122@@ -8,8 +8,7 @@ import tempfile
123
124 from cloudinit import util as c_util
125 from tests.cloud_tests import (config, LOG)
126-from tests.cloud_tests.platforms import (platforms, images, snapshots,
127- instances)
128+from tests.cloud_tests import platforms
129 from tests.cloud_tests.stage import (PlatformComponent, run_stage, run_single)
130
131 pre_reqs = ['devscripts', 'equivs', 'git', 'tar']
132@@ -85,18 +84,18 @@ def setup_build(args):
133 # set up image
134 LOG.info('acquiring image for os: %s', args.build_os)
135 img_conf = config.load_os_config(platform.platform_name, args.build_os)
136- image_call = partial(images.get_image, platform, img_conf)
137+ image_call = partial(platforms.get_image, platform, img_conf)
138 with PlatformComponent(image_call) as image:
139
140 # set up snapshot
141- snapshot_call = partial(snapshots.get_snapshot, image)
142+ snapshot_call = partial(platforms.get_snapshot, image)
143 with PlatformComponent(snapshot_call) as snapshot:
144
145 # create instance with cloud-config to set it up
146 LOG.info('creating instance to build deb in')
147 empty_cloud_config = "#cloud-config\n{}"
148 instance_call = partial(
149- instances.get_instance, snapshot, empty_cloud_config,
150+ platforms.get_instance, snapshot, empty_cloud_config,
151 use_desc='build cloud-init deb')
152 with PlatformComponent(instance_call) as instance:
153
154diff --git a/tests/cloud_tests/collect.py b/tests/cloud_tests/collect.py
155index db5ee99..4805cea 100644
156--- a/tests/cloud_tests/collect.py
157+++ b/tests/cloud_tests/collect.py
158@@ -64,9 +64,9 @@ def collect_test_data(args, snapshot, os_name, test_name):
159 # skip the testcase with a warning
160 req_features = test_config.get('required_features', [])
161 if any(feature not in snapshot.features for feature in req_features):
162- LOG.warn('test config %s requires features not supported by image, '
163- 'skipping.\nrequired features: %s\nsupported features: %s',
164- test_name, req_features, snapshot.features)
165+ LOG.warning('test config %s requires features not supported by image, '
166+ 'skipping.\nrequired features: %s\nsupported features: %s',
167+ test_name, req_features, snapshot.features)
168 return ({}, 0)
169
170 # if there are user data overrides required for this test case, apply them
171diff --git a/tests/cloud_tests/config.py b/tests/cloud_tests/config.py
172index 52fc2bd..8bd569f 100644
173--- a/tests/cloud_tests/config.py
174+++ b/tests/cloud_tests/config.py
175@@ -92,7 +92,7 @@ def load_platform_config(platform_name, require_enabled=False):
176
177
178 def load_os_config(platform_name, os_name, require_enabled=False,
179- feature_overrides={}):
180+ feature_overrides=None):
181 """Load configuration for os.
182
183 @param platform_name: platform name to load os config for
184@@ -101,6 +101,8 @@ def load_os_config(platform_name, os_name, require_enabled=False,
185 @param feature_overrides: feature flag overrides to merge with features
186 @return_value: config dict
187 """
188+ if feature_overrides is None:
189+ feature_overrides = {}
190 main_conf = c_util.read_conf(RELEASES_CONF)
191 default = main_conf['default_release_config']
192 image = main_conf['releases'][os_name]
193diff --git a/tests/cloud_tests/testcases/base.py b/tests/cloud_tests/testcases/base.py
194index 1706f59..1c5b540 100644
195--- a/tests/cloud_tests/testcases/base.py
196+++ b/tests/cloud_tests/testcases/base.py
197@@ -12,7 +12,8 @@ from cloudinit import util as c_util
198 class CloudTestCase(unittest.TestCase):
199 """Base test class for verifiers."""
200
201- data = None
202+ # data gets populated in get_suite.setUpClass
203+ data = {}
204 conf = None
205 _cloud_config = None
206
207diff --git a/tests/cloud_tests/testcases/modules/set_hostname_fqdn.py b/tests/cloud_tests/testcases/modules/set_hostname_fqdn.py
208index eb6f065..a405b30 100644
209--- a/tests/cloud_tests/testcases/modules/set_hostname_fqdn.py
210+++ b/tests/cloud_tests/testcases/modules/set_hostname_fqdn.py
211@@ -1,7 +1,7 @@
212 # This file is part of cloud-init. See LICENSE file for license information.
213
214 """cloud-init Integration Test Verify Script."""
215-from tests.cloud_tests.instances.nocloudkvm import CI_DOMAIN
216+from tests.cloud_tests import CI_DOMAIN
217 from tests.cloud_tests.testcases import base
218
219
220diff --git a/tests/cloud_tests/util.py b/tests/cloud_tests/util.py
221index c5cd697..2aedcd0 100644
222--- a/tests/cloud_tests/util.py
223+++ b/tests/cloud_tests/util.py
224@@ -262,7 +262,7 @@ def shell_safe(cmd):
225 out = subprocess.check_output(
226 ["getopt", "--shell", "sh", "--options", "", "--", "--"] + list(cmd))
227 # out contains ' -- <data>\n'. drop the ' -- ' and the '\n'
228- return out[4:-1].decode()
229+ return out.decode()[4:-1]
230
231
232 def shell_pack(cmd):
233diff --git a/tests/unittests/test_cs_util.py b/tests/unittests/test_cs_util.py
234index ee88520..2a1095b 100644
235--- a/tests/unittests/test_cs_util.py
236+++ b/tests/unittests/test_cs_util.py
237@@ -35,6 +35,7 @@ class CepkoMock(Cepko):
238 # touched the underlying Cepko class methods.
239 class CepkoResultTests(test_helpers.TestCase):
240 def setUp(self):
241+ self.c = Cepko()
242 raise test_helpers.SkipTest('This test is completely useless')
243
244 def test_getitem(self):
245diff --git a/tests/unittests/test_datasource/test_azure.py b/tests/unittests/test_datasource/test_azure.py
246index 226c214..5ab4889 100644
247--- a/tests/unittests/test_datasource/test_azure.py
248+++ b/tests/unittests/test_datasource/test_azure.py
249@@ -36,9 +36,9 @@ def construct_valid_ovf_env(data=None, pubkeys=None, userdata=None):
250 """
251 for key, dval in data.items():
252 if isinstance(dval, dict):
253- val = dval.get('text')
254- attrs = ' ' + ' '.join(["%s='%s'" % (k, v) for k, v in dval.items()
255- if k != 'text'])
256+ val = dict(dval).get('text')
257+ attrs = ' ' + ' '.join(["%s='%s'" % (k, v) for k, v
258+ in dict(dval).items() if k != 'text'])
259 else:
260 val = dval
261 attrs = ""
262@@ -897,9 +897,6 @@ class TestCanDevBeReformatted(CiTestCase):
263 setattr(self, sattr, patcher.start())
264 self.addCleanup(patcher.stop)
265
266- def setUp(self):
267- super(TestCanDevBeReformatted, self).setUp()
268-
269 def patchup(self, devs):
270 bypath = {}
271 for path, data in devs.items():
272@@ -954,14 +951,14 @@ class TestCanDevBeReformatted(CiTestCase):
273 '/dev/sda3': {'num': 3},
274 }}})
275 value, msg = dsaz.can_dev_be_reformatted("/dev/sda")
276- self.assertFalse(False, value)
277+ self.assertFalse(value)
278 self.assertIn("3 or more", msg.lower())
279
280 def test_no_partitions_is_false(self):
281 """A disk with no partitions can not be formatted."""
282 self.patchup({'/dev/sda': {}})
283 value, msg = dsaz.can_dev_be_reformatted("/dev/sda")
284- self.assertEqual(False, value)
285+ self.assertFalse(value)
286 self.assertIn("not partitioned", msg.lower())
287
288 def test_two_partitions_not_ntfs_false(self):
289@@ -973,7 +970,7 @@ class TestCanDevBeReformatted(CiTestCase):
290 '/dev/sda2': {'num': 2, 'fs': 'ext4', 'files': []},
291 }}})
292 value, msg = dsaz.can_dev_be_reformatted("/dev/sda")
293- self.assertFalse(False, value)
294+ self.assertFalse(value)
295 self.assertIn("not ntfs", msg.lower())
296
297 def test_two_partitions_ntfs_populated_false(self):
298@@ -986,7 +983,7 @@ class TestCanDevBeReformatted(CiTestCase):
299 'files': ['secret.txt']},
300 }}})
301 value, msg = dsaz.can_dev_be_reformatted("/dev/sda")
302- self.assertFalse(False, value)
303+ self.assertFalse(value)
304 self.assertIn("files on it", msg.lower())
305
306 def test_two_partitions_ntfs_empty_is_true(self):
307@@ -998,7 +995,7 @@ class TestCanDevBeReformatted(CiTestCase):
308 '/dev/sda2': {'num': 2, 'fs': 'ntfs', 'files': []},
309 }}})
310 value, msg = dsaz.can_dev_be_reformatted("/dev/sda")
311- self.assertEqual(True, value)
312+ self.assertTrue(value)
313 self.assertIn("safe for", msg.lower())
314
315 def test_one_partition_not_ntfs_false(self):
316@@ -1009,7 +1006,7 @@ class TestCanDevBeReformatted(CiTestCase):
317 '/dev/sda1': {'num': 1, 'fs': 'zfs'},
318 }}})
319 value, msg = dsaz.can_dev_be_reformatted("/dev/sda")
320- self.assertEqual(False, value)
321+ self.assertFalse(value)
322 self.assertIn("not ntfs", msg.lower())
323
324 def test_one_partition_ntfs_populated_false(self):
325@@ -1021,7 +1018,7 @@ class TestCanDevBeReformatted(CiTestCase):
326 'files': ['file1.txt', 'file2.exe']},
327 }}})
328 value, msg = dsaz.can_dev_be_reformatted("/dev/sda")
329- self.assertEqual(False, value)
330+ self.assertFalse(value)
331 self.assertIn("files on it", msg.lower())
332
333 def test_one_partition_ntfs_empty_is_true(self):
334@@ -1032,7 +1029,7 @@ class TestCanDevBeReformatted(CiTestCase):
335 '/dev/sda1': {'num': 1, 'fs': 'ntfs', 'files': []}
336 }}})
337 value, msg = dsaz.can_dev_be_reformatted("/dev/sda")
338- self.assertEqual(True, value)
339+ self.assertTrue(value)
340 self.assertIn("safe for", msg.lower())
341
342 def test_one_partition_ntfs_empty_with_dataloss_file_is_true(self):
343@@ -1044,7 +1041,7 @@ class TestCanDevBeReformatted(CiTestCase):
344 'files': ['dataloss_warning_readme.txt']}
345 }}})
346 value, msg = dsaz.can_dev_be_reformatted("/dev/sda")
347- self.assertEqual(True, value)
348+ self.assertTrue(value)
349 self.assertIn("safe for", msg.lower())
350
351 def test_one_partition_through_realpath_is_true(self):
352@@ -1059,7 +1056,7 @@ class TestCanDevBeReformatted(CiTestCase):
353 'realpath': '/dev/sdb1'}
354 }}})
355 value, msg = dsaz.can_dev_be_reformatted(epath)
356- self.assertEqual(True, value)
357+ self.assertTrue(value)
358 self.assertIn("safe for", msg.lower())
359
360 def test_three_partition_through_realpath_is_false(self):
361@@ -1078,7 +1075,7 @@ class TestCanDevBeReformatted(CiTestCase):
362 'realpath': '/dev/sdb3'}
363 }}})
364 value, msg = dsaz.can_dev_be_reformatted(epath)
365- self.assertEqual(False, value)
366+ self.assertFalse(value)
367 self.assertIn("3 or more", msg.lower())
368
369
370diff --git a/tests/unittests/test_datasource/test_digitalocean.py b/tests/unittests/test_datasource/test_digitalocean.py
371index ec32173..3127014 100644
372--- a/tests/unittests/test_datasource/test_digitalocean.py
373+++ b/tests/unittests/test_datasource/test_digitalocean.py
374@@ -199,9 +199,8 @@ class TestDataSourceDigitalOcean(CiTestCase):
375
376 class TestNetworkConvert(CiTestCase):
377
378- @mock.patch('cloudinit.net.get_interfaces_by_mac')
379- def _get_networking(self, m_get_by_mac):
380- m_get_by_mac.return_value = {
381+ def _get_networking(self):
382+ self.m_get_by_mac.return_value = {
383 '04:01:57:d1:9e:01': 'ens1',
384 '04:01:57:d1:9e:02': 'ens2',
385 'b8:ae:ed:75:5f:9a': 'enp0s25',
386@@ -211,6 +210,10 @@ class TestNetworkConvert(CiTestCase):
387 self.assertIn('config', netcfg)
388 return netcfg
389
390+ def setUp(self):
391+ super(TestNetworkConvert, self).setUp()
392+ self.add_patch('cloudinit.net.get_interfaces_by_mac', 'm_get_by_mac')
393+
394 def test_networking_defined(self):
395 netcfg = self._get_networking()
396 self.assertIsNotNone(netcfg)
397diff --git a/tests/unittests/test_datasource/test_ec2.py b/tests/unittests/test_datasource/test_ec2.py
398index ba042ea..f0dc833 100644
399--- a/tests/unittests/test_datasource/test_ec2.py
400+++ b/tests/unittests/test_datasource/test_ec2.py
401@@ -330,7 +330,8 @@ class TestEc2(test_helpers.HttprettyTestCase):
402 ds.fallback_nic = 'eth9'
403 with mock.patch(get_interface_mac_path) as m_get_interface_mac:
404 m_get_interface_mac.return_value = mac1
405- ds.network_config # Will re-crawl network metadata
406+ nc = ds.network_config # Will re-crawl network metadata
407+ self.assertIsNotNone(nc)
408 self.assertIn('Re-crawl of metadata service', self.logs.getvalue())
409 expected = {'version': 1, 'config': [
410 {'mac_address': '06:17:04:d7:26:09',
411diff --git a/tests/unittests/test_distros/test_create_users.py b/tests/unittests/test_distros/test_create_users.py
412index aa13670..5670904 100644
413--- a/tests/unittests/test_distros/test_create_users.py
414+++ b/tests/unittests/test_distros/test_create_users.py
415@@ -7,7 +7,11 @@ from cloudinit.tests.helpers import (TestCase, mock)
416 class MyBaseDistro(distros.Distro):
417 # MyBaseDistro is here to test base Distro class implementations
418
419- def __init__(self, name="basedistro", cfg={}, paths={}):
420+ def __init__(self, name="basedistro", cfg=None, paths=None):
421+ if not cfg:
422+ cfg = {}
423+ if not paths:
424+ paths = {}
425 super(MyBaseDistro, self).__init__(name, cfg, paths)
426
427 def install_packages(self, pkglist):
428@@ -42,7 +46,6 @@ class MyBaseDistro(distros.Distro):
429 @mock.patch("cloudinit.distros.util.subp")
430 class TestCreateUser(TestCase):
431 def setUp(self):
432- super(TestCase, self).setUp()
433 self.dist = MyBaseDistro()
434
435 def _useradd2call(self, args):
436diff --git a/tests/unittests/test_distros/test_netconfig.py b/tests/unittests/test_distros/test_netconfig.py
437index c4bd11b..8d0b263 100644
438--- a/tests/unittests/test_distros/test_netconfig.py
439+++ b/tests/unittests/test_distros/test_netconfig.py
440@@ -188,9 +188,6 @@ hn0: flags=8843<UP,BROADCAST,RUNNING,SIMPLEX,MULTICAST> metric 0 mtu 1500
441 status: active
442 """
443
444- def setUp(self):
445- super(TestNetCfgDistro, self).setUp()
446-
447 def _get_distro(self, dname, renderers=None):
448 cls = distros.fetch(dname)
449 cfg = settings.CFG_BUILTIN
450diff --git a/tests/unittests/test_handler/test_handler_lxd.py b/tests/unittests/test_handler/test_handler_lxd.py
451index e0d9ab6..a205498 100644
452--- a/tests/unittests/test_handler/test_handler_lxd.py
453+++ b/tests/unittests/test_handler/test_handler_lxd.py
454@@ -25,9 +25,6 @@ class TestLxd(t_help.CiTestCase):
455 }
456 }
457
458- def setUp(self):
459- super(TestLxd, self).setUp()
460-
461 def _get_cloud(self, distro):
462 cls = distros.fetch(distro)
463 paths = helpers.Paths({})
464diff --git a/tests/unittests/test_handler/test_handler_power_state.py b/tests/unittests/test_handler/test_handler_power_state.py
465index 85a0fe0..3c72642 100644
466--- a/tests/unittests/test_handler/test_handler_power_state.py
467+++ b/tests/unittests/test_handler/test_handler_power_state.py
468@@ -9,9 +9,6 @@ from cloudinit.tests.helpers import mock
469
470
471 class TestLoadPowerState(t_help.TestCase):
472- def setUp(self):
473- super(self.__class__, self).setUp()
474-
475 def test_no_config(self):
476 # completely empty config should mean do nothing
477 (cmd, _timeout, _condition) = psc.load_power_state({})
478diff --git a/tests/unittests/test_handler/test_handler_yum_add_repo.py b/tests/unittests/test_handler/test_handler_yum_add_repo.py
479index b7adbe5..b90a3af 100644
480--- a/tests/unittests/test_handler/test_handler_yum_add_repo.py
481+++ b/tests/unittests/test_handler/test_handler_yum_add_repo.py
482@@ -5,10 +5,6 @@ from cloudinit import util
483
484 from cloudinit.tests import helpers
485
486-try:
487- from configparser import ConfigParser
488-except ImportError:
489- from ConfigParser import ConfigParser
490 import logging
491 import shutil
492 from six import StringIO
493@@ -58,8 +54,7 @@ class TestConfig(helpers.FilesystemMockingTestCase):
494 self.patchUtils(self.tmp)
495 cc_yum_add_repo.handle('yum_add_repo', cfg, None, LOG, [])
496 contents = util.load_file("/etc/yum.repos.d/epel_testing.repo")
497- parser = ConfigParser()
498- parser.readfp(StringIO(contents))
499+ parser = self.parse_and_read(StringIO(contents))
500 expected = {
501 'epel_testing': {
502 'name': 'Extra Packages for Enterprise Linux 5 - Testing',
503@@ -95,8 +90,7 @@ class TestConfig(helpers.FilesystemMockingTestCase):
504 self.patchUtils(self.tmp)
505 cc_yum_add_repo.handle('yum_add_repo', cfg, None, LOG, [])
506 contents = util.load_file("/etc/yum.repos.d/puppetlabs_products.repo")
507- parser = ConfigParser()
508- parser.readfp(StringIO(contents))
509+ parser = self.parse_and_read(StringIO(contents))
510 expected = {
511 'puppetlabs_products': {
512 'name': 'Puppet Labs Products El 6 - $basearch',
513diff --git a/tests/unittests/test_handler/test_handler_zypper_add_repo.py b/tests/unittests/test_handler/test_handler_zypper_add_repo.py
514index 315c2a5..72ab6c0 100644
515--- a/tests/unittests/test_handler/test_handler_zypper_add_repo.py
516+++ b/tests/unittests/test_handler/test_handler_zypper_add_repo.py
517@@ -9,10 +9,6 @@ from cloudinit import util
518 from cloudinit.tests import helpers
519 from cloudinit.tests.helpers import mock
520
521-try:
522- from configparser import ConfigParser
523-except ImportError:
524- from ConfigParser import ConfigParser
525 import logging
526 from six import StringIO
527
528@@ -70,8 +66,7 @@ class TestConfig(helpers.FilesystemMockingTestCase):
529 root_d = self.tmp_dir()
530 cc_zypper_add_repo._write_repos(cfg['repos'], root_d)
531 contents = util.load_file("%s/testing-foo.repo" % root_d)
532- parser = ConfigParser()
533- parser.readfp(StringIO(contents))
534+ parser = self.parse_and_read(StringIO(contents))
535 expected = {
536 'testing-foo': {
537 'name': 'test-foo',
538diff --git a/tests/unittests/test_reporting.py b/tests/unittests/test_reporting.py
539index 571420e..e15ba6c 100644
540--- a/tests/unittests/test_reporting.py
541+++ b/tests/unittests/test_reporting.py
542@@ -126,7 +126,7 @@ class TestBaseReportingHandler(TestCase):
543
544 def test_base_reporting_handler_is_abstract(self):
545 regexp = r".*abstract.*publish_event.*"
546- self.assertRaisesRegexp(TypeError, regexp, handlers.ReportingHandler)
547+ self.assertRaisesRegex(TypeError, regexp, handlers.ReportingHandler)
548
549
550 class TestLogHandler(TestCase):
551diff --git a/tests/unittests/test_templating.py b/tests/unittests/test_templating.py
552index b911d92..53154d3 100644
553--- a/tests/unittests/test_templating.py
554+++ b/tests/unittests/test_templating.py
555@@ -14,7 +14,7 @@ from cloudinit import templater
556 try:
557 import Cheetah
558 HAS_CHEETAH = True
559- Cheetah # make pyflakes happy, as Cheetah is not used here
560+ c = Cheetah # make pyflakes and pylint happy, as Cheetah is not used here
561 except ImportError:
562 HAS_CHEETAH = False
563
564diff --git a/tests/unittests/test_util.py b/tests/unittests/test_util.py
565index 71f5952..787ca20 100644
566--- a/tests/unittests/test_util.py
567+++ b/tests/unittests/test_util.py
568@@ -695,9 +695,9 @@ class TestSubp(helpers.CiTestCase):
569 util.write_file(noshebang, 'true\n')
570
571 os.chmod(noshebang, os.stat(noshebang).st_mode | stat.S_IEXEC)
572- self.assertRaisesRegexp(util.ProcessExecutionError,
573- 'Missing #! in script\?',
574- util.subp, (noshebang,))
575+ self.assertRaisesRegex(util.ProcessExecutionError,
576+ 'Missing #! in script\?',
577+ util.subp, (noshebang,))
578
579 def test_returns_none_if_no_capture(self):
580 (out, err) = util.subp(self.stdin2out, data=b'', capture=False)
581diff --git a/tests/unittests/test_vmware_config_file.py b/tests/unittests/test_vmware_config_file.py
582index 808d303..0f8cda9 100644
583--- a/tests/unittests/test_vmware_config_file.py
584+++ b/tests/unittests/test_vmware_config_file.py
585@@ -133,7 +133,8 @@ class TestVmwareConfigFile(CiTestCase):
586
587 conf = Config(cf)
588 with self.assertRaises(ValueError):
589- conf.reset_password()
590+ pw = conf.reset_password
591+ self.assertIsNone(pw)
592
593 cf.clear()
594 cf._insertKey("PASSWORD|RESET", "yes")
595diff --git a/tools/hacking.py b/tools/hacking.py
596deleted file mode 100755
597index e6a0513..0000000
598--- a/tools/hacking.py
599+++ /dev/null
600@@ -1,172 +0,0 @@
601-#!/usr/bin/env python
602-# Copyright (c) 2012, Cloudscaling
603-# All Rights Reserved.
604-#
605-# Licensed under the Apache License, Version 2.0 (the "License"); you may
606-# not use this file except in compliance with the License. You may obtain
607-# a copy of the License at
608-#
609-# http://www.apache.org/licenses/LICENSE-2.0
610-#
611-# Unless required by applicable law or agreed to in writing, software
612-# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
613-# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
614-# License for the specific language governing permissions and limitations
615-# under the License.
616-
617-"""cloudinit HACKING file compliance testing (based off of nova hacking.py)
618-
619-built on top of pep8.py
620-"""
621-
622-import inspect
623-import logging
624-import re
625-import sys
626-
627-import pep8
628-
629-# Don't need this for testing
630-logging.disable('LOG')
631-
632-# N1xx comments
633-# N2xx except
634-# N3xx imports
635-# N4xx docstrings
636-# N[5-9]XX (future use)
637-
638-DOCSTRING_TRIPLE = ['"""', "'''"]
639-VERBOSE_MISSING_IMPORT = False
640-_missingImport = set([])
641-
642-
643-def import_normalize(line):
644- # convert "from x import y" to "import x.y"
645- # handle "from x import y as z" to "import x.y as z"
646- split_line = line.split()
647- if (line.startswith("from ") and "," not in line and
648- split_line[2] == "import" and split_line[3] != "*" and
649- split_line[1] != "__future__" and
650- (len(split_line) == 4 or (len(split_line) == 6 and
651- split_line[4] == "as"))):
652- return "import %s.%s" % (split_line[1], split_line[3])
653- else:
654- return line
655-
656-
657-def cloud_import_alphabetical(physical_line, line_number, lines):
658- """Check for imports in alphabetical order.
659-
660- HACKING guide recommendation for imports:
661- imports in human alphabetical order
662- N306
663- """
664- # handle import x
665- # use .lower since capitalization shouldn't dictate order
666- split_line = import_normalize(physical_line.strip()).lower().split()
667- split_previous = import_normalize(lines[line_number - 2])
668- split_previous = split_previous.strip().lower().split()
669- # with or without "as y"
670- length = [2, 4]
671- if (len(split_line) in length and len(split_previous) in length and
672- split_line[0] == "import" and split_previous[0] == "import"):
673- if split_line[1] < split_previous[1]:
674- return (0, "N306: imports not in alphabetical order (%s, %s)"
675- % (split_previous[1], split_line[1]))
676-
677-
678-def cloud_docstring_start_space(physical_line):
679- """Check for docstring not start with space.
680-
681- HACKING guide recommendation for docstring:
682- Docstring should not start with space
683- N401
684- """
685- pos = max([physical_line.find(i) for i in DOCSTRING_TRIPLE]) # start
686- if (pos != -1 and len(physical_line) > pos + 1):
687- if (physical_line[pos + 3] == ' '):
688- return (pos,
689- "N401: one line docstring should not start with a space")
690-
691-
692-def cloud_todo_format(physical_line):
693- """Check for 'TODO()'.
694-
695- HACKING guide recommendation for TODO:
696- Include your name with TODOs as in "#TODO(termie)"
697- N101
698- """
699- pos = physical_line.find('TODO')
700- pos1 = physical_line.find('TODO(')
701- pos2 = physical_line.find('#') # make sure it's a comment
702- if (pos != pos1 and pos2 >= 0 and pos2 < pos):
703- return pos, "N101: Use TODO(NAME)"
704-
705-
706-def cloud_docstring_one_line(physical_line):
707- """Check one line docstring end.
708-
709- HACKING guide recommendation for one line docstring:
710- A one line docstring looks like this and ends in a period.
711- N402
712- """
713- pos = max([physical_line.find(i) for i in DOCSTRING_TRIPLE]) # start
714- end = max([physical_line[-4:-1] == i for i in DOCSTRING_TRIPLE]) # end
715- if (pos != -1 and end and len(physical_line) > pos + 4):
716- if (physical_line[-5] != '.'):
717- return pos, "N402: one line docstring needs a period"
718-
719-
720-def cloud_docstring_multiline_end(physical_line):
721- """Check multi line docstring end.
722-
723- HACKING guide recommendation for docstring:
724- Docstring should end on a new line
725- N403
726- """
727- pos = max([physical_line.find(i) for i in DOCSTRING_TRIPLE]) # start
728- if (pos != -1 and len(physical_line) == pos):
729- print(physical_line)
730- if (physical_line[pos + 3] == ' '):
731- return (pos, "N403: multi line docstring end on new line")
732-
733-
734-current_file = ""
735-
736-
737-def readlines(filename):
738- """Record the current file being tested."""
739- pep8.current_file = filename
740- return open(filename).readlines()
741-
742-
743-def add_cloud():
744- """Monkey patch pep8 for cloud-init guidelines.
745-
746- Look for functions that start with cloud_
747- and add them to pep8 module.
748-
749- Assumes you know how to write pep8.py checks
750- """
751- for name, function in globals().items():
752- if not inspect.isfunction(function):
753- continue
754- if name.startswith("cloud_"):
755- exec("pep8.%s = %s" % (name, name))
756-
757-
758-if __name__ == "__main__":
759- # NOVA based 'hacking.py' error codes start with an N
760- pep8.ERRORCODE_REGEX = re.compile(r'[EWN]\d{3}')
761- add_cloud()
762- pep8.current_file = current_file
763- pep8.readlines = readlines
764- try:
765- pep8._main()
766- finally:
767- if len(_missingImport) > 0:
768- sys.stderr.write(
769- "%i imports missing in this test environment\n" %
770- len(_missingImport))
771-
772-# vi: ts=4 expandtab
773diff --git a/tools/make-mime.py b/tools/make-mime.py
774index f6a7204..d321479 100755
775--- a/tools/make-mime.py
776+++ b/tools/make-mime.py
777@@ -23,7 +23,7 @@ def file_content_type(text):
778 filename, content_type = text.split(":", 1)
779 return (open(filename, 'r'), filename, content_type.strip())
780 except ValueError:
781- raise argparse.ArgumentError("Invalid value for %r" % (text))
782+ raise argparse.ArgumentError(text, "Invalid value for %r" % (text))
783
784
785 def main():
786diff --git a/tools/mock-meta.py b/tools/mock-meta.py
787index a5d14ab..724f7fc 100755
788--- a/tools/mock-meta.py
789+++ b/tools/mock-meta.py
790@@ -17,6 +17,7 @@ Then:
791 ec2metadata --instance-id
792 """
793
794+import argparse
795 import functools
796 import json
797 import logging
798@@ -27,8 +28,6 @@ import string
799 import sys
800 import yaml
801
802-from optparse import OptionParser
803-
804 try:
805 from BaseHTTPServer import HTTPServer, BaseHTTPRequestHandler
806 import httplib as hclient
807@@ -415,29 +414,27 @@ def setup_logging(log_level, fmt='%(levelname)s: @%(name)s : %(message)s'):
808
809
810 def extract_opts():
811- parser = OptionParser()
812- parser.add_option("-p", "--port", dest="port", action="store", type=int,
813- default=80, metavar="PORT",
814- help=("port from which to serve traffic"
815- " (default: %default)"))
816- parser.add_option("-a", "--addr", dest="address", action="store", type=str,
817- default='::', metavar="ADDRESS",
818- help=("address from which to serve traffic"
819- " (default: %default)"))
820- parser.add_option("-f", '--user-data-file', dest='user_data_file',
821- action='store', metavar='FILE',
822- help=("user data filename to serve back to"
823- "incoming requests"))
824- (options, args) = parser.parse_args()
825- out = dict()
826- out['extra'] = args
827- out['port'] = options.port
828- out['user_data_file'] = None
829- out['address'] = options.address
830- if options.user_data_file:
831- if not os.path.isfile(options.user_data_file):
832+ parser = argparse.ArgumentParser()
833+ parser.add_argument("-p", "--port", dest="port", action="store", type=int,
834+ default=80, metavar="PORT",
835+ help=("port from which to serve traffic"
836+ " (default: %default)"))
837+ parser.add_argument("-a", "--addr", dest="address", action="store",
838+ type=str, default='::', metavar="ADDRESS",
839+ help=("address from which to serve traffic"
840+ " (default: %default)"))
841+ parser.add_argument("-f", '--user-data-file', dest='user_data_file',
842+ action='store', metavar='FILE',
843+ help=("user data filename to serve back to"
844+ "incoming requests"))
845+ parser.add_argument('extra', nargs='*')
846+ args = parser.parse_args()
847+ out = {'port': args.port, 'address': args.address, 'extra': args.extra,
848+ 'user_data_file': None}
849+ if args.user_data_file:
850+ if not os.path.isfile(args.user_data_file):
851 parser.error("Option -f specified a non-existent file")
852- with open(options.user_data_file, 'rb') as fh:
853+ with open(args.user_data_file, 'rb') as fh:
854 out['user_data_file'] = fh.read()
855 return out
856
857diff --git a/tox.ini b/tox.ini
858index 9223220..d7316cc 100644
859--- a/tox.ini
860+++ b/tox.ini
861@@ -21,12 +21,13 @@ setenv =
862 LC_ALL = en_US.utf-8
863
864 [testenv:pylint]
865+basepython = python3
866 deps =
867 # requirements
868 pylint==1.7.1
869 # test-requirements because unit tests are now present in cloudinit tree
870 -r{toxinidir}/test-requirements.txt
871-commands = {envpython} -m pylint {posargs:cloudinit}
872+commands = {envpython} -m pylint {posargs:cloudinit tests tools}
873
874 [testenv:py3]
875 basepython = python3
876@@ -119,7 +120,7 @@ commands = {envpython} -m pyflakes {posargs:cloudinit/ tests/ tools/}
877 deps = pyflakes
878
879 [testenv:tip-pylint]
880-commands = {envpython} -m pylint {posargs:cloudinit}
881+commands = {envpython} -m pylint {posargs:cloudinit tests tools}
882 deps =
883 # requirements
884 pylint

Subscribers

People subscribed via source and target branches