Merge lp:~harlowja/cloud-init/cloud-init-fix-test-times into lp:~cloud-init-dev/cloud-init/trunk

Proposed by Joshua Harlow on 2016-05-16
Status: Merged
Merged at revision: 1219
Proposed branch: lp:~harlowja/cloud-init/cloud-init-fix-test-times
Merge into: lp:~cloud-init-dev/cloud-init/trunk
Diff against target: 175 lines (+27/-20)
3 files modified
cloudinit/sources/DataSourceOpenStack.py (+8/-4)
test-requirements.txt (+3/-0)
tests/unittests/test_datasource/test_openstack.py (+16/-16)
To merge this branch: bzr merge lp:~harlowja/cloud-init/cloud-init-fix-test-times
Reviewer Review Type Date Requested Status
cloud-init commiters 2016-05-16 Pending
Review via email: mp+294854@code.launchpad.net

Commit Message

Fixes up tests taking forever due to retries and timeouts.

Timing info:

Fixed timing via `time nosetests`:

real 0m12.376s
user 0m6.797s
sys 0m0.616s

Not fixed timing via `time nosetests`:

real 1m12.886s
user 0m7.202s
sys 0m0.666s

To post a comment you must log in.
Scott Moser (smoser) wrote :

I don't *need* nose-timer for this, right?
I just ask because python-nose-timer is not available in trusty, and also isn't in ubuntu main, so putting it as a build-dep (in order to run tests during a package build) is less than ideal.

I'm not entirely opposed to it, but if there is some easy way that we can do without it, i'd be interested.

https://launchpad.net/ubuntu/+source/python-nose-timer

Joshua Harlow (harlowja) wrote :

Right right, maybe let me exclude that. The timer part is really only for detecting the times, not for fixing them. We can leave the inclusion of that to https://code.launchpad.net/~harlowja/cloud-init/cloud-init-nose-timer/+merge/294569 instead, this at least has the fixes for thinks taking to much time (due to retrying and such).

1220. By Joshua Harlow on 2016-05-24

Make the usage of 'nose-timer' optional

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'cloudinit/sources/DataSourceOpenStack.py'
2--- cloudinit/sources/DataSourceOpenStack.py 2016-04-04 16:31:28 +0000
3+++ cloudinit/sources/DataSourceOpenStack.py 2016-05-24 21:05:39 +0000
4@@ -103,7 +103,7 @@
5 self.metadata_address = url2base.get(avail_url)
6 return bool(avail_url)
7
8- def get_data(self):
9+ def get_data(self, retries=5, timeout=5):
10 try:
11 if not self.wait_for_metadata_service():
12 return False
13@@ -115,7 +115,9 @@
14 'Crawl of openstack metadata service',
15 read_metadata_service,
16 args=[self.metadata_address],
17- kwargs={'ssl_details': self.ssl_details})
18+ kwargs={'ssl_details': self.ssl_details,
19+ 'retries': retries,
20+ 'timeout': timeout})
21 except openstack.NonReadable:
22 return False
23 except (openstack.BrokenMetadata, IOError):
24@@ -153,8 +155,10 @@
25 return sources.instance_id_matches_system_uuid(self.get_instance_id())
26
27
28-def read_metadata_service(base_url, ssl_details=None):
29- reader = openstack.MetadataReader(base_url, ssl_details=ssl_details)
30+def read_metadata_service(base_url, ssl_details=None,
31+ timeout=5, retries=5):
32+ reader = openstack.MetadataReader(base_url, ssl_details=ssl_details,
33+ timeout=timeout, retries=retries)
34 return reader.read_v2()
35
36
37
38=== modified file 'test-requirements.txt'
39--- test-requirements.txt 2016-05-12 18:35:18 +0000
40+++ test-requirements.txt 2016-05-24 21:05:39 +0000
41@@ -3,6 +3,9 @@
42 mock
43 nose
44
45+# Only needed if u want to know the test times
46+# nose-timer
47+
48 # Only really needed on older versions of python
49 contextlib2
50 setuptools
51
52=== modified file 'tests/unittests/test_datasource/test_openstack.py'
53--- tests/unittests/test_datasource/test_openstack.py 2016-05-12 20:43:11 +0000
54+++ tests/unittests/test_datasource/test_openstack.py 2016-05-24 21:05:39 +0000
55@@ -135,13 +135,17 @@
56 body=get_request_callback)
57
58
59+def _read_metadata_service():
60+ return ds.read_metadata_service(BASE_URL, retries=0, timeout=0.1)
61+
62+
63 class TestOpenStackDataSource(test_helpers.HttprettyTestCase):
64 VERSION = 'latest'
65
66 @hp.activate
67 def test_successful(self):
68 _register_uris(self.VERSION, EC2_FILES, EC2_META, OS_FILES)
69- f = ds.read_metadata_service(BASE_URL)
70+ f = _read_metadata_service()
71 self.assertEqual(VENDOR_DATA, f.get('vendordata'))
72 self.assertEqual(CONTENT_0, f['files']['/etc/foo.cfg'])
73 self.assertEqual(CONTENT_1, f['files']['/etc/bar/bar.cfg'])
74@@ -163,7 +167,7 @@
75 @hp.activate
76 def test_no_ec2(self):
77 _register_uris(self.VERSION, {}, {}, OS_FILES)
78- f = ds.read_metadata_service(BASE_URL)
79+ f = _read_metadata_service()
80 self.assertEqual(VENDOR_DATA, f.get('vendordata'))
81 self.assertEqual(CONTENT_0, f['files']['/etc/foo.cfg'])
82 self.assertEqual(CONTENT_1, f['files']['/etc/bar/bar.cfg'])
83@@ -178,8 +182,7 @@
84 if k.endswith('meta_data.json'):
85 os_files.pop(k, None)
86 _register_uris(self.VERSION, {}, {}, os_files)
87- self.assertRaises(openstack.NonReadable, ds.read_metadata_service,
88- BASE_URL)
89+ self.assertRaises(openstack.NonReadable, _read_metadata_service)
90
91 @hp.activate
92 def test_bad_uuid(self):
93@@ -190,8 +193,7 @@
94 if k.endswith('meta_data.json'):
95 os_files[k] = json.dumps(os_meta)
96 _register_uris(self.VERSION, {}, {}, os_files)
97- self.assertRaises(openstack.BrokenMetadata, ds.read_metadata_service,
98- BASE_URL)
99+ self.assertRaises(openstack.BrokenMetadata, _read_metadata_service)
100
101 @hp.activate
102 def test_userdata_empty(self):
103@@ -200,7 +202,7 @@
104 if k.endswith('user_data'):
105 os_files.pop(k, None)
106 _register_uris(self.VERSION, {}, {}, os_files)
107- f = ds.read_metadata_service(BASE_URL)
108+ f = _read_metadata_service()
109 self.assertEqual(VENDOR_DATA, f.get('vendordata'))
110 self.assertEqual(CONTENT_0, f['files']['/etc/foo.cfg'])
111 self.assertEqual(CONTENT_1, f['files']['/etc/bar/bar.cfg'])
112@@ -213,7 +215,7 @@
113 if k.endswith('vendor_data.json'):
114 os_files.pop(k, None)
115 _register_uris(self.VERSION, {}, {}, os_files)
116- f = ds.read_metadata_service(BASE_URL)
117+ f = _read_metadata_service()
118 self.assertEqual(CONTENT_0, f['files']['/etc/foo.cfg'])
119 self.assertEqual(CONTENT_1, f['files']['/etc/bar/bar.cfg'])
120 self.assertFalse(f.get('vendordata'))
121@@ -225,8 +227,7 @@
122 if k.endswith('vendor_data.json'):
123 os_files[k] = '{' # some invalid json
124 _register_uris(self.VERSION, {}, {}, os_files)
125- self.assertRaises(openstack.BrokenMetadata, ds.read_metadata_service,
126- BASE_URL)
127+ self.assertRaises(openstack.BrokenMetadata, _read_metadata_service)
128
129 @hp.activate
130 def test_metadata_invalid(self):
131@@ -235,8 +236,7 @@
132 if k.endswith('meta_data.json'):
133 os_files[k] = '{' # some invalid json
134 _register_uris(self.VERSION, {}, {}, os_files)
135- self.assertRaises(openstack.BrokenMetadata, ds.read_metadata_service,
136- BASE_URL)
137+ self.assertRaises(openstack.BrokenMetadata, _read_metadata_service)
138
139 @hp.activate
140 def test_datasource(self):
141@@ -245,7 +245,7 @@
142 None,
143 helpers.Paths({}))
144 self.assertIsNone(ds_os.version)
145- found = ds_os.get_data()
146+ found = ds_os.get_data(timeout=0.1, retries=0)
147 self.assertTrue(found)
148 self.assertEqual(2, ds_os.version)
149 md = dict(ds_os.metadata)
150@@ -269,7 +269,7 @@
151 None,
152 helpers.Paths({}))
153 self.assertIsNone(ds_os.version)
154- found = ds_os.get_data()
155+ found = ds_os.get_data(timeout=0.1, retries=0)
156 self.assertFalse(found)
157 self.assertIsNone(ds_os.version)
158
159@@ -288,7 +288,7 @@
160 'timeout': 0,
161 }
162 self.assertIsNone(ds_os.version)
163- found = ds_os.get_data()
164+ found = ds_os.get_data(timeout=0.1, retries=0)
165 self.assertFalse(found)
166 self.assertIsNone(ds_os.version)
167
168@@ -311,7 +311,7 @@
169 'timeout': 0,
170 }
171 self.assertIsNone(ds_os.version)
172- found = ds_os.get_data()
173+ found = ds_os.get_data(timeout=0.1, retries=0)
174 self.assertFalse(found)
175 self.assertIsNone(ds_os.version)
176