Merge lp:~sinzui/juju-release-tools/validate-streams into lp:juju-release-tools

Proposed by Curtis Hovey
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
Reviewer Review Type Date Requested Status
Aaron Bentley (community) Approve
Review via email: mp+237975@code.launchpad.net

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-release-tools/validate_streams.py proposed 1.20.10 old/com.ubuntu.juju\:released\:tools.json new/com.ubuntu.juju\:released\:tools.json

Comparing old with itself, but expecting 1.20.10 change is an error because the tools are missing
    ./juju-release-tools/validate_streams.py proposed 1.20.10 old/com.ubuntu.juju\:released\:tools.json old/com.ubuntu.juju\:released\:tools.json

I will use this script in assemble-streams in the future.

To post a comment you must log in.
Revision history for this message
Aaron Bentley (abentley) wrote :

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'])
new_tools = make_tools_data('trusty', 'amd64', ['1.20.7', '1.20.8'])
check_expected_tools(old_tools, new_tools, '1.20.8', '1.20.9')

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.

review: Needs Information
Revision history for this message
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_data('trusty', 'amd64', ['1.20.7', '1.20.8'])
new_tools = make_tools_data('trusty', 'amd64', ['1.20.7', '1.20.9'])
check_expected_tools(old_tools, new_tools, '1.20.9', '1.20.8')

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.

Revision history for this message
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()
                     if t[1][version] != new_version)
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()
                           if t[1][version] != retracted)

Aaron
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1

iQEcBAEBAgAGBQJUQWVfAAoJEK84cMOcf+9hk5QIAIk9rRS3YnrPjwSrhsRHB8n9
FXgZPXTi/Z61AbG3nrbESbpHE8R8FVtM5dQL7chVbhUSi52oFl2CVVS4wBTfX+/r
TYViz/bJQOTuAAgOwtRIMZ/ylHSdgjgVcbI0AaOguP5KevuERWxR3mXH6pusdX5+
xeaYtCTkkgK2jfcBNdPW54o97T/MnCh57iRKOFmgWCm5QMktizGkyPAvgpCBwhmx
vaPwjqnFjZUIuN3zWLFL7Gn6YS9QHWWbDAvjKp2XqyoDAuKVk3x+QH5RhFkuQlfB
luf56rgHhj/YoCbkBrj4dB6gTcv2pmFW1mHdontpKJvn2bgcOXK5tKIpYVqQBuw=
=Sd2G
-----END PGP SIGNATURE-----

105. By Curtis Hovey

added is optional. use --added to clearly state a version was added.

Revision history for this message
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_expected_tools(old_tools, new_tools, added=None, retracted=None)

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

Revision history for this message
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.

Revision history for this message
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_tools can be tricked out of warning that "added" is missing.

1. Do a successful retract. This means that expected_differences will be non-empty from the retract clause, and so the added clause cannot set missing_errors.

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.

review: Needs Fixing
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.

Revision history for this message
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.canonical.com's behaviour.

Revision history for this message
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/versions/*/items. I could change the
> 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.items(): + if t['version'] == retracted:
>> + expected_differences.update([(n, t)])
>
> 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_differences? You don't use it anywhere, except the
"added" clause. And in the "added" clause, it can only cause a bug,
by falsely preventing the "missing.append(added)".

> So expected_differences was just a way to remove what we know to
> be different to prove nothing else is changed.

No, it doesn't do anything like that. All it does is prevent
"missing.append(added)", so that this version is not considered missing.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1

iQEcBAEBAgAGBQJURnGpAAoJEK84cMOcf+9hwDkIAJmJOGytVsStF5De6MijLpV5
4G6YNYWhzp+ShM38KIhT39CZ83pJV4DY7yO3ajIi03/359j1rufRug4CxukxPGwq
ByQO+/SuyvKZYpHFRVqmpsCe1kaKX4WC8gInLm2JLgFoMr/zXDWAL9jnZ8TMmEa7
V0a9T/jNit/3q9t4hMEoaNohaAaWj3vxbNWdm/gasmtApMiqtLltHD1SNEm8czau
py1qmtidjNd4R4q+TTjROmo5uJqOR0jPBOe1MoYdzzUY4F/1QgqWlCEo6TEVTseU
rSuaU1aVG0ks63Et+S8/GcBuqGOjT5DIjHDIt1LYVPa4xZYuBkHuP6loJmEyOZ4=
=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.

Revision history for this message
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.

Revision history for this message
Aaron Bentley (abentley) wrote :

This looks clear and correct. Thanks!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
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))

Subscribers

People subscribed via source and target branches