Merge lp:~sinzui/juju-release-tools/validate-streams into lp:juju-release-tools
- validate-streams
- Merge into trunk
Status: | Merged |
---|---|
Merged at revision: | 91 |
Proposed branch: | lp:~sinzui/juju-release-tools/validate-streams |
Merge into: | lp:juju-release-tools |
Diff against target: |
459 lines (+438/-1) 3 files modified
tests/test_make_release_notes.py (+1/-1) tests/test_validate_streams.py (+234/-0) validate_streams.py (+203/-0) |
To merge this branch: | bzr merge lp:~sinzui/juju-release-tools/validate-streams |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Aaron Bentley (community) | Approve | ||
Review via email: mp+237975@code.launchpad.net |
Commit message
Description of the change
This branch adds a script that can compare the product items in two streams files and verify that the expected changes exist, that there are no missing or extra items, and that the common items are the same. The script ensures that non-numeric version cannot be in the proposed and release json
Nothing uses this script at this time.
The command line is purpose, new-version old-json, new-json. I compared the release to the proposed json like this to verify that 1.20.10 is the only change between them
./juju-
Comparing old with itself, but expecting 1.20.10 change is an error because the tools are missing
./juju-
I will use this script in assemble-streams in the future.
Curtis Hovey (sinzui) wrote : | # |
Hi Aaron. Thank you for the review. your question has raised a scenario I had not considered.
Retract is like a mode. We are removing a set instead of adding one. The version being published is likely to be the previous version. The version is always passed because Jerf will always be installing that version to create the streams. I assumed that the version is the previous good version.
I don't think your example is real. I cannot think of retracting a later version, and publishing an earlier non-existent version, but...
I can image waking up and find an email from Ian that says there is a security issue with version n, his team have prepared version q. We need to retract earlier n and since something is ready, publish later q. I would certainly want to do one publication, not two to achieve this. I am certain this test would fail because there are two expected differences.
old_tools = make_tools_
new_tools = make_tools_
check_expected_
I am going to change the rules to accommodate this case. This is tricky I think the rule is that when doing a retractions, we need to ask if the version being published is new and need to be included in the expected differences.
- 103. By Curtis Hovey
-
Added can be done because we want to be clear about when we add a new version
and retract an old version. - 104. By Curtis Hovey
-
Clearly state the content is correct when using verbose.
Aaron Bentley (abentley) wrote : | # |
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 14-10-17 01:58 PM, Curtis Hovey wrote:
> I am going to change the rules to accommodate this case. This is
> tricky I think the rule is that when doing a retractions, we need
> to ask if the version being published is new and need to be
> included in the expected differences.
I don't think it's actually that tricky. I think it's something like
added = dict(t for t in new_tools.items() if t[0] not in old_tools)
wrongly_added = dict(t for t in added.items()
removed = dict(t for t in old_tools.items() if t[0] not in new_tools)
if retracted is None:
wrongly_removed = removed
else:
wrongly_removed = dict(t for t in added.items()
Aaron
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1
iQEcBAEBAgAGBQJ
FXgZPXTi/
TYViz/bJQOTuAAg
xeaYtCTkkgK2jfc
vaPwjqnFjZUIuN3
luf56rgHhj/
=Sd2G
-----END PGP SIGNATURE-----
- 105. By Curtis Hovey
-
added is optional. use --added to clearly state a version was added.
Curtis Hovey (sinzui) wrote : | # |
I have a subtle change to the script. Instead of passing the args that might be passed to assemble-streams, the script now expects the caller to clearly state the expected differences.
I changed the script command line to clearly indicate when a version is added:
--added 1.20.9 --retracted 1.20.8
Which also renamed version => added
I have changed added to be optional so that the function does not need to guess about expected differences
check_
The assemble-streams script will need intelligence to know when something is added and retracted. This might not be tricky since it is configured with the reacted version and there is a check if any new tools were created.
Note I made a change to verbose. I used this script for the 1.20.10 release, and I had to check the return code because --verbose didn't clearly tell me that everything was fine. I really wanted a clear message that the content was correct.
- 106. By Curtis Hovey
-
Extracted temp_dir() to tests.utils
Curtis Hovey (sinzui) wrote : | # |
Oh, and I am sure you noted that temp_dir() could be extracted to a utils.py. I have done this to keep the code DRY.
Aaron Bentley (abentley) wrote : | # |
Okay, I think I'm getting the hang of this code, and I'm pretty sure it is not correct. There are two ways check_expected_
1. Do a successful retract. This means that expected_
2. Have other missing versions. This means that the "added" clause will set missing_errors, but the "missing" clause will overwrite its value. There will be an error, but it won't mention "added".
Other comments below.
- 107. By Curtis Hovey
-
Merged tip and resolved conflicts.
- 108. By Curtis Hovey
-
Read the file directly into the stream.
- 109. By Curtis Hovey
-
Do not overwrite missing_errors.
Curtis Hovey (sinzui) wrote : | # |
I have replied to the comments and pushed up some changes. I have another change I need to make, at least a test, and maybe a code change to account for streams.
Aaron Bentley (abentley) wrote : | # |
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 14-10-20 05:55 PM, Curtis Hovey wrote:
> The "versions" keys are arbitrary names :(. juju metadata will
> name it something like "20141020". We only get 1 key in versions,
> but the specification allows many. We don't care if there is 1 or
> many of these versions we only want to get the the next level,
> "items". The goal is to say all items on old are the same as the
> items in new, except we know that we sometimes add items and we
> have one removed items, once published without items, and on a few
> occasions change items.
>
> In Xpath, this is products/
> test for items to be if isinstance(version, dict) and "items" in
> versions:
Okay, the code is fine as it stands, then.
>> + # needs a version to install to make streams, even when
>> it + # intends to remove something. + for n, t in
>> old_expected.
>> + expected_
>
> The goal was to state the expected differences to create a common
> set of old and new tools that must match in name a content.
Deleting entries from old_expected is perfectly sane. But why are
updating expected_
"added" clause. And in the "added" clause, it can only cause a bug,
by falsely preventing the "missing.
> So expected_
> be different to prove nothing else is changed.
No, it doesn't do anything like that. All it does is prevent
"missing.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1
iQEcBAEBAgAGBQJ
4G6YNYWhzp+
ByQO+/SuyvKZYpH
V0a9T/jNit/
py1qmtidjNd4R4q
rSuaU1aVG0ks63E
=GrUS
-----END PGP SIGNATURE-----
- 110. By Curtis Hovey
-
Merged tip, Removed IGNORE.
- 111. By Curtis Hovey
-
Renamed retracted => removed.
- 112. By Curtis Hovey
-
expected_
differences is not needed. - 113. By Curtis Hovey
-
Make code easier to maintain, though a little redudant, by creating smaller
functions with clear purpose. - 114. By Curtis Hovey
-
Revise docstrings.
Curtis Hovey (sinzui) wrote : | # |
My latest changes follow your suggestion to simplify the purpose of the functions to make the code easier to maintain. I split the rules to check for expected versions from rules to find unexpected version.
I also changed all the checks to always return a list to make the contact simple and it is easy for the callee to extend the list of errors.
Aaron Bentley (abentley) wrote : | # |
This looks clear and correct. Thanks!
Preview Diff
1 | === modified file 'tests/test_make_release_notes.py' |
2 | --- tests/test_make_release_notes.py 2014-10-20 18:43:54 +0000 |
3 | +++ tests/test_make_release_notes.py 2014-10-22 21:03:40 +0000 |
4 | @@ -3,7 +3,7 @@ |
5 | from StringIO import StringIO |
6 | from unittest import TestCase |
7 | |
8 | - |
9 | +from utils import temp_dir |
10 | from make_release_notes import ( |
11 | DEVEL, |
12 | get_bugs, |
13 | |
14 | === added file 'tests/test_validate_streams.py' |
15 | --- tests/test_validate_streams.py 1970-01-01 00:00:00 +0000 |
16 | +++ tests/test_validate_streams.py 2014-10-22 21:03:40 +0000 |
17 | @@ -0,0 +1,234 @@ |
18 | +from mock import patch |
19 | +import json |
20 | +from unittest import TestCase |
21 | + |
22 | +from utils import temp_dir |
23 | +from validate_streams import ( |
24 | + check_devel_not_stable, |
25 | + check_expected_changes, |
26 | + check_expected_unchanged, |
27 | + check_agents_content, |
28 | + compare_agents, |
29 | + find_agents, |
30 | + parse_args, |
31 | +) |
32 | + |
33 | + |
34 | +def make_tool_data(version='1.20.7', release='trusty', arch='amd64'): |
35 | + name = '{}-{}-{}'.format(version, release, arch) |
36 | + tool = { |
37 | + "release": "{}".format(release), |
38 | + "version": "{}".format(version), |
39 | + "arch": "{}".format(arch), |
40 | + "size": 8234578, |
41 | + "path": "releases/juju-{}.tgz".format(name), |
42 | + "ftype": "tar.gz", |
43 | + "sha256": "valid_sum" |
44 | + } |
45 | + return name, tool |
46 | + |
47 | + |
48 | +def make_agents_data(release='trusty', arch='amd64', versions=['1.20.7']): |
49 | + return dict(make_tool_data(v, release, arch) for v in versions) |
50 | + |
51 | + |
52 | +def make_product_data(release='trusty', arch='amd64', versions=['1.20.7']): |
53 | + name = 'com.ubuntu.juju:{}:{}'.format(release, arch) |
54 | + items = make_agents_data(release, arch, versions) |
55 | + product = { |
56 | + "version": "{}".format(versions[0]), |
57 | + "arch": "{}".format(arch), |
58 | + "versions": { |
59 | + "20140919": { |
60 | + "items": items |
61 | + } |
62 | + } |
63 | + } |
64 | + return name, product |
65 | + |
66 | + |
67 | +def make_products_data(versions): |
68 | + products = {} |
69 | + for release, arch in (('trusty', 'amd64'), ('trusty', 'i386')): |
70 | + name, product = make_product_data(release, arch, versions) |
71 | + products[name] = product |
72 | + stream = { |
73 | + "products": products, |
74 | + "updated": "Fri, 19 Sep 2014 13:25:28 -0400", |
75 | + "format": "products:1.0", |
76 | + "content_id": "com.ubuntu.juju:released:agents" |
77 | + } |
78 | + return stream |
79 | + |
80 | + |
81 | +class ValidateStreams(TestCase): |
82 | + |
83 | + def test_parge_args(self): |
84 | + # The purpose, release, old json and new json are required. |
85 | + required = ['--added', '1.20.9', 'proposed', 'old/json', 'new/json'] |
86 | + args = parse_args(required) |
87 | + self.assertEqual('proposed', args.purpose) |
88 | + self.assertEqual('old/json', args.old_json) |
89 | + self.assertEqual('new/json', args.new_json) |
90 | + self.assertEqual('1.20.9', args.added) |
91 | + # A bad release version can be removed. |
92 | + args = parse_args(['--removed', 'bad'] + required) |
93 | + self.assertEqual('bad', args.removed) |
94 | + |
95 | + def test_find_agents(self): |
96 | + products = make_products_data(['1.20.7', '1.20.8']) |
97 | + with temp_dir() as wd: |
98 | + file_path = '{}/json'.format(wd) |
99 | + with open(file_path, 'w') as f: |
100 | + f.write(json.dumps(products)) |
101 | + agents = find_agents(file_path) |
102 | + expected = [ |
103 | + '1.20.7-trusty-i386', '1.20.7-trusty-amd64', |
104 | + '1.20.8-trusty-amd64', '1.20.8-trusty-i386'] |
105 | + self.assertEqual(expected, agents.keys()) |
106 | + |
107 | + def test_check_devel_not_stable(self): |
108 | + # devel agents cannot ever got to proposed and release. |
109 | + old_agents = make_agents_data('trusty', 'amd64', ['1.20.7', '1.20.8']) |
110 | + new_agents = make_agents_data( |
111 | + 'trusty', 'amd64', ['1.20.7', '1.20.8', '1.21-alpha1']) |
112 | + # Devel versions can go to testing |
113 | + message = check_devel_not_stable(old_agents, new_agents, 'testing') |
114 | + self.assertEqual([], message) |
115 | + # Devel versions can go to devel |
116 | + message = check_devel_not_stable(old_agents, new_agents, 'devel') |
117 | + self.assertEqual([], message) |
118 | + # Devel versions cannot be proposed. |
119 | + message = check_devel_not_stable(old_agents, new_agents, 'proposed') |
120 | + self.assertEqual( |
121 | + ["Devel versions in proposed stream: " |
122 | + "['1.21-alpha1-trusty-amd64']"], |
123 | + message) |
124 | + # Devel versions cannot be release. |
125 | + message = check_devel_not_stable(old_agents, new_agents, 'release') |
126 | + self.assertEqual( |
127 | + ["Devel versions in release stream: ['1.21-alpha1-trusty-amd64']"], |
128 | + message) |
129 | + |
130 | + def test_check_agents_content(self): |
131 | + old_agents = make_agents_data('trusty', 'amd64', ['1.20.7', '1.20.8']) |
132 | + new_agents = make_agents_data( |
133 | + 'trusty', 'amd64', ['1.20.7', '1.20.8']) |
134 | + new_agents['1.20.7-trusty-amd64']['sha256'] = 'bad_sum' |
135 | + message = check_agents_content(old_agents, new_agents) |
136 | + self.assertEqual( |
137 | + (['Tool 1.20.7-trusty-amd64 sha256 changed from ' |
138 | + 'valid_sum to bad_sum']), |
139 | + message) |
140 | + |
141 | + def test_compare_agents_identical(self): |
142 | + old_agents = make_agents_data('trusty', 'amd64', ['1.20.7', '1.20.8']) |
143 | + new_agents = make_agents_data('trusty', 'amd64', ['1.20.7', '1.20.8']) |
144 | + message = compare_agents( |
145 | + old_agents, new_agents, 'proposed', added=None, removed=None) |
146 | + self.assertIs(None, message) |
147 | + |
148 | + def test_check_expected_changes_with_no_changes(self): |
149 | + new_agents = make_agents_data('trusty', 'amd64', ['1.20.7', '1.20.8']) |
150 | + errors = check_expected_changes(new_agents, added=None, removed=None) |
151 | + self.assertEqual([], errors) |
152 | + |
153 | + def test_check_expected_changes_with_changes(self): |
154 | + new_agents = make_agents_data('trusty', 'amd64', ['1.20.7', '1.20.9']) |
155 | + errors = check_expected_changes( |
156 | + new_agents, added='1.20.9', removed='1.20.8') |
157 | + self.assertEqual([], errors) |
158 | + |
159 | + def test_check_expected_changes_with_found_errors(self): |
160 | + new_agents = make_agents_data('trusty', 'amd64', ['1.20.7', '1.20.8']) |
161 | + errors = check_expected_changes( |
162 | + new_agents, added=None, removed='1.20.8') |
163 | + self.assertEqual( |
164 | + ["1.20.8 agents were not removed: ['1.20.8-trusty-amd64']"], |
165 | + errors) |
166 | + |
167 | + def test_check_expected_changes_with_missing_errors(self): |
168 | + new_agents = make_agents_data('trusty', 'amd64', ['1.20.7']) |
169 | + errors = check_expected_changes( |
170 | + new_agents, added='1.20.8', removed=None) |
171 | + self.assertEqual(['1.20.8 agents were not added'], errors) |
172 | + |
173 | + def test_check_expected_unchanged_without_changes(self): |
174 | + old_agents = make_agents_data('trusty', 'amd64', ['1.20.7', '1.20.8']) |
175 | + new_agents = make_agents_data('trusty', 'amd64', ['1.20.7', '1.20.8']) |
176 | + errors = check_expected_unchanged( |
177 | + old_agents, new_agents, added=None, removed=None) |
178 | + self.assertEqual([], errors) |
179 | + |
180 | + def test_check_expected_unchanged_without_changes_and_added_removed(self): |
181 | + old_agents = make_agents_data('trusty', 'amd64', ['1.20.7', '1.20.8']) |
182 | + new_agents = make_agents_data('trusty', 'amd64', ['1.20.7', '1.20.9']) |
183 | + errors = check_expected_unchanged( |
184 | + old_agents, new_agents, added='1.20.9', removed='1.20.8') |
185 | + self.assertEqual([], errors) |
186 | + |
187 | + def test_check_expected_unchanged_with_missing_errors(self): |
188 | + old_agents = make_agents_data('trusty', 'amd64', ['1.20.7', '1.20.8']) |
189 | + new_agents = make_agents_data('trusty', 'amd64', ['1.20.7', '1.20.9']) |
190 | + errors = check_expected_unchanged( |
191 | + old_agents, new_agents, added='1.20.9', removed=None) |
192 | + self.assertEqual( |
193 | + ["These agents are missing: ['1.20.8-trusty-amd64']"], |
194 | + errors) |
195 | + |
196 | + def test_check_expected_unchanged_with_found_errors(self): |
197 | + old_agents = make_agents_data('trusty', 'amd64', ['1.20.7']) |
198 | + new_agents = make_agents_data('trusty', 'amd64', ['1.20.7', '1.20.9']) |
199 | + errors = check_expected_unchanged( |
200 | + old_agents, new_agents, added=None, removed=None) |
201 | + self.assertEqual( |
202 | + ["These unknown agents were found: ['1.20.9-trusty-amd64']"], |
203 | + errors) |
204 | + |
205 | + def test_compare_agents_changed_tool(self): |
206 | + old_agents = make_agents_data('trusty', 'amd64', ['1.20.7', '1.20.8']) |
207 | + new_agents = make_agents_data( |
208 | + 'trusty', 'amd64', ['1.20.7', '1.20.8', '1.20.9']) |
209 | + new_agents['1.20.7-trusty-amd64']['sha256'] = 'bad_sum' |
210 | + errors = compare_agents( |
211 | + old_agents, new_agents, 'proposed', '1.20.9', removed=None) |
212 | + self.assertEqual( |
213 | + ['Tool 1.20.7-trusty-amd64 sha256 changed from ' |
214 | + 'valid_sum to bad_sum'], |
215 | + errors) |
216 | + |
217 | + def test_compare_agents_called_check_expected_agents_called(self): |
218 | + old_agents = make_agents_data('trusty', 'amd64', ['1.20.7']) |
219 | + new_agents = make_agents_data('trusty', 'amd64', ['1.20.7', '1.20.8']) |
220 | + with patch("validate_streams.check_expected_changes", |
221 | + return_value=(['foo'])) as cec_mock: |
222 | + with patch("validate_streams.check_expected_unchanged", |
223 | + return_value=(['bar'])) as ceu_mock: |
224 | + errors = compare_agents( |
225 | + old_agents, new_agents, 'proposed', '1.20.9', removed=None) |
226 | + cec_mock.assert_called_with(new_agents, '1.20.9', None) |
227 | + ceu_mock.assert_called_with( |
228 | + old_agents, new_agents, '1.20.9', None) |
229 | + self.assertEqual(['foo', 'bar'], errors) |
230 | + |
231 | + def test_compare_agents_added_devel_version(self): |
232 | + # devel agents cannot ever got to proposed and release. |
233 | + old_agents = make_agents_data('trusty', 'amd64', ['1.20.7', '1.20.8']) |
234 | + new_agents = make_agents_data( |
235 | + 'trusty', 'amd64', ['1.20.7', '1.20.8', '1.21-alpha1']) |
236 | + # Devel versions can go to devel |
237 | + message = compare_agents( |
238 | + old_agents, new_agents, 'devel', '1.21-alpha1', removed=None) |
239 | + self.assertIs(None, message) |
240 | + # Devel versions cannot be proposed. |
241 | + errors = compare_agents( |
242 | + old_agents, new_agents, 'proposed', '1.21-alpha1', removed=None) |
243 | + expected = ( |
244 | + "Devel versions in proposed stream: ['1.21-alpha1-trusty-amd64']") |
245 | + self.assertEqual([expected], errors) |
246 | + # Devel versions cannot be release. |
247 | + errors = compare_agents( |
248 | + old_agents, new_agents, 'release', '1.21-alpha1', removed=None) |
249 | + expected = ( |
250 | + "Devel versions in release stream: ['1.21-alpha1-trusty-amd64']") |
251 | + self.assertEqual([expected], errors) |
252 | |
253 | === added file 'validate_streams.py' |
254 | --- validate_streams.py 1970-01-01 00:00:00 +0000 |
255 | +++ validate_streams.py 2014-10-22 21:03:40 +0000 |
256 | @@ -0,0 +1,203 @@ |
257 | +#!/usr/bin/python |
258 | + |
259 | +from __future__ import print_function |
260 | + |
261 | +from argparse import ArgumentParser |
262 | +import json |
263 | +import re |
264 | +import sys |
265 | +import traceback |
266 | + |
267 | + |
268 | +RELEASE = 'release' |
269 | +PROPOSED = 'proposed' |
270 | +DEVEL = 'devel' |
271 | +TESTING = 'testing' |
272 | +PURPOSES = (RELEASE, PROPOSED, DEVEL, TESTING) |
273 | + |
274 | + |
275 | +def find_agents(file_path): |
276 | + with open(file_path) as f: |
277 | + stream = json.load(f) |
278 | + agents = {} |
279 | + for name, product in stream['products'].items(): |
280 | + versions = product['versions'] |
281 | + for version in versions.values(): |
282 | + if isinstance(version, dict): |
283 | + items = version['items'] |
284 | + agents.update(items) |
285 | + return agents |
286 | + |
287 | + |
288 | +def check_devel_not_stable(old_agents, new_agents, purpose): |
289 | + """Return a list of errors if the version can be included in the stream. |
290 | + |
291 | + Devel versions cannot be proposed or release because the letters in |
292 | + the version break older jujus. |
293 | + |
294 | + :param old_agents: the dict of all the products/versions/*/items |
295 | + in the old json. |
296 | + :param new_agents: the dict of all the products/versions/*/items |
297 | + in the new json. |
298 | + :param purpose: either release, proposed, devel, or testing. |
299 | + :return: a list of errors, which will be empty when there are none. |
300 | + """ |
301 | + if purpose in (TESTING, DEVEL): |
302 | + return [] |
303 | + stable_pattern = re.compile(r'\d+\.\d+\.\d+-*') |
304 | + devel_versions = [ |
305 | + v for v in new_agents.keys() if not stable_pattern.match(v)] |
306 | + errors = [] |
307 | + if devel_versions: |
308 | + errors.append( |
309 | + 'Devel versions in {} stream: {}'.format(purpose, devel_versions)) |
310 | + return errors |
311 | + |
312 | + |
313 | +def check_expected_changes(new_agents, added=None, removed=None): |
314 | + """Return an list of errors if the expected changes are not present. |
315 | + |
316 | + :param new_agents: the dict of all the products/versions/*/items |
317 | + in the new json. |
318 | + :param added: the version added to the new json, eg '1.20.9'. |
319 | + :param removed: the version removed from the new json, eg '1.20.8'. |
320 | + :return: a list of errors, which will be empty when there are none. |
321 | + """ |
322 | + found = [] |
323 | + seen = False |
324 | + for n, t in new_agents.items(): |
325 | + if removed and t['version'] == removed: |
326 | + found.append(n) |
327 | + elif added and t['version'] == added: |
328 | + seen = True |
329 | + errors = [] |
330 | + if added and not seen: |
331 | + errors.append('{} agents were not added'.format(added)) |
332 | + if found: |
333 | + errors.append('{} agents were not removed: {}'.format(removed, found)) |
334 | + return errors |
335 | + |
336 | + |
337 | +def check_expected_unchanged(old_agents, new_agents, added=None, removed=None): |
338 | + """Return a list of errors if the expected unchanged versions do not match. |
339 | + |
340 | + :param old_agents: the dict of all the products/versions/*/items |
341 | + in the old json. |
342 | + :param new_agents: the dict of all the products/versions/*/items |
343 | + in the new json. |
344 | + :param added: the version added to the new json, eg '1.20.9'. |
345 | + :param removed: the version removed from the new json, eg '1.20.8'. |
346 | + :return: a list of errors, which will be empty when there are none. |
347 | + """ |
348 | + old_versions = set(k for (k, v) in old_agents.items() |
349 | + if v['version'] != removed) |
350 | + new_versions = set(k for (k, v) in new_agents.items() |
351 | + if v['version'] != added) |
352 | + missing_errors = old_versions - new_versions |
353 | + errors = [] |
354 | + if missing_errors: |
355 | + missing_errors = list(missing_errors) |
356 | + errors.append('These agents are missing: {}'.format(missing_errors)) |
357 | + found_errors = new_versions - old_versions |
358 | + if found_errors: |
359 | + found_errors = list(found_errors) |
360 | + errors.append('These unknown agents were found: {}'.format( |
361 | + found_errors)) |
362 | + return errors |
363 | + |
364 | + |
365 | +def check_agents_content(old_agents, new_agents): |
366 | + """Return a list of error messages if agents content changes. |
367 | + |
368 | + Are the old versions identical to the new versions? |
369 | + We care about change values, not new keys in the new tool. |
370 | + |
371 | + :param old_agents: the dict of all the products/versions/*/items |
372 | + in the old json. |
373 | + :param new_agents: the dict of all the products/versions/*/items |
374 | + in the new json. |
375 | + :return: a list of errors, which will be empty when there are none. |
376 | + """ |
377 | + if not new_agents: |
378 | + return None |
379 | + errors = [] |
380 | + for name, old_tool in old_agents.items(): |
381 | + try: |
382 | + new_tool = new_agents[name] |
383 | + except KeyError: |
384 | + # This is a missing version case reported by check_expected_agents. |
385 | + continue |
386 | + for old_key, old_val in old_tool.items(): |
387 | + new_val = new_tool[old_key] |
388 | + if old_val != new_val: |
389 | + errors.append( |
390 | + 'Tool {} {} changed from {} to {}'.format( |
391 | + name, old_key, old_val, new_val)) |
392 | + return errors |
393 | + |
394 | + |
395 | +def compare_agents(old_agents, new_agents, purpose, added=None, removed=None): |
396 | + """Return a list of error messages from all the validation checks. |
397 | + |
398 | + :param old_agents: the dict of all the products/versions/*/items |
399 | + in the old json. |
400 | + :param new_agents: the dict of all the products/versions/*/items |
401 | + in the new json. |
402 | + :return: a list of errors, which will be empty when there are none. |
403 | + """ |
404 | + errors = [] |
405 | + errors.extend( |
406 | + check_devel_not_stable(old_agents, new_agents, purpose)) |
407 | + errors.extend( |
408 | + check_expected_changes(new_agents, added, removed)) |
409 | + errors.extend( |
410 | + check_expected_unchanged(old_agents, new_agents, added, removed)) |
411 | + errors.extend( |
412 | + check_agents_content(old_agents, new_agents)) |
413 | + return errors or None |
414 | + |
415 | + |
416 | +def parse_args(args=None): |
417 | + """Return the argument parser for this program.""" |
418 | + parser = ArgumentParser("Compare old and new stream data.") |
419 | + parser.add_argument( |
420 | + '-v', '--verbose', action="store_true", default=False, |
421 | + help='Increse verbosity.') |
422 | + parser.add_argument( |
423 | + '-r', '--removed', default=None, help='The release version removed') |
424 | + parser.add_argument( |
425 | + '-a', '--added', default=None, help="The release version added") |
426 | + parser.add_argument('purpose', help="<{}>".format(' | '.join(PURPOSES))) |
427 | + parser.add_argument('old_json', help="The old simple streams data file") |
428 | + parser.add_argument('new_json', help="The new simple streams data file") |
429 | + return parser.parse_args(args) |
430 | + |
431 | + |
432 | +def main(argv): |
433 | + """Verify that the new json has all the expected changes. |
434 | + |
435 | + An exit code of 1 will have a list of strings explaining the problems. |
436 | + An exit code of 0 is a pass and the explanation is None. |
437 | + """ |
438 | + args = parse_args(argv[1:]) |
439 | + try: |
440 | + old_agents = find_agents(args.old_json) |
441 | + new_agents = find_agents(args.new_json) |
442 | + errors = compare_agents( |
443 | + old_agents, new_agents, args.purpose, args.version, |
444 | + retracted=args.retracted) |
445 | + if errors: |
446 | + print('\n'.join(errors)) |
447 | + return 1 |
448 | + except Exception as e: |
449 | + print(e) |
450 | + if args.verbose: |
451 | + traceback.print_tb(sys.exc_info()[2]) |
452 | + return 2 |
453 | + if args.verbose: |
454 | + print("All changes are correct.") |
455 | + return 0 |
456 | + |
457 | + |
458 | +if __name__ == '__main__': |
459 | + sys.exit(main(sys.argv)) |
I am finding it very hard to understand check_expected_ tools. I don't know why it will accept version and then basically ignore it if retracted is set. It seems like retracted is really a mode, not a parameter. I would like to see a test like this:
old_tools = make_tools_ data('trusty' , 'amd64', ['1.20.7', '1.20.9']) data('trusty' , 'amd64', ['1.20.7', '1.20.8']) tools(old_ tools, new_tools, '1.20.8', '1.20.9')
new_tools = make_tools_
check_expected_
Notice that 1.20.8 has been added and 1.20.9 has been removed. I don't know what the expected behaviour is. I think the actual behaviour is that it will complain that 1.20.8 is unexpected.