Merge lp:~sseman/juju-ci-tools/model-change-watcher-py3-2 into lp:juju-ci-tools

Proposed by Seman on 2016-12-07
Status: Needs review
Proposed branch: lp:~sseman/juju-ci-tools/model-change-watcher-py3-2
Merge into: lp:juju-ci-tools
Prerequisite: lp:~sseman/juju-ci-tools/model-change-watcher-py3
Diff against target: 288 lines (+244/-11)
3 files modified
Makefile (+6/-3)
jujupy.py (+0/-8)
tests_py3/test_assess_model_change_watcher.py (+238/-0)
To merge this branch: bzr merge lp:~sseman/juju-ci-tools/model-change-watcher-py3-2
Reviewer Review Type Date Requested Status
Nicholas Skaggs (community) Approve on 2016-12-09
Curtis Hovey (community) code 2016-12-07 Needs Information on 2016-12-09
Review via email: mp+312755@code.launchpad.net

Description of the change

This is the second branch that adds unit tests for the assess_model_change_watcher.py script.

It also updates the Makefile so that the right version of Python is used to run unittest and flake8. "unittest" doesn't make it simple to exclude some files, example excluding Python 3 files when running Python 2 tests, therefore I created a different directory for Python 3 test scripts.

First branch:
https://code.launchpad.net/~sseman/juju-ci-tools/model-change-watcher-py3/+merge/312523

To post a comment you must log in.
1796. By Seman on 2016-12-08

Removed event.

Nicholas Skaggs (nskaggs) wrote :

I'll wait until the pre-req is landed. However, 1 comment below.

Curtis Hovey (sinzui) wrote :

Thank you. I think the Makefile changes are incomplete.

review: Needs Information (code)
Seman (sseman) wrote :

I don't know if it works with all OSes you mentioned but I do see your point.

Instead of breaking "make test", I can add a different rule for Python 3 unit tests "test_python3" and only run tests under "tests_py3" directory.

Instead of using the find, grep, xargs commands for lint test, I will exclude the files explicitly and add a different rule for Python 3.
 py3="assess_model_change_watcher.py test_assess_model_change_watcher.py"
 lint:
    flake8 $$(find -name '*.py') --builtins xrange,basestring --exclude $(py3)
 lint_python3:
    flake8 $(py3)

Does the above seem a reasonable solution?

Nicholas Skaggs (nskaggs) wrote :

Thank you Seman.

review: Approve

Unmerged revisions

1796. By Seman on 2016-12-08

Removed event.

1795. By Seman on 2016-12-07

Updated Makefile.

1794. By Seman on 2016-12-07

Added unit tests.

1793. By Seman on 2016-12-06

Merged.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'Makefile'
2--- Makefile 2016-12-08 03:12:58 +0000
3+++ Makefile 2016-12-08 03:12:58 +0000
4@@ -1,10 +1,13 @@
5 p=test*.py
6-py3="assess_model_change_watcher.py"
7+python_3_compatible_tests := $$(find -name '*.py' | xargs grep -xl '\#!/usr/bin/env python3')
8+python_2_compatible_tests := $$(find -name '*.py' | xargs grep -xL '\#!/usr/bin/env python3')
9+
10 test:
11+ TMPDIR=/tmp python3 -m unittest discover -vv ./tests_py3 -p "$(p)"
12 TMPDIR=/tmp python -m unittest discover -vv ./tests -p "$(p)"
13 lint:
14- python3 -m flake8 --builtins xrange,basestring $(py3)
15- flake8 $$(find -name '*.py') --builtins xrange,basestring --exclude $(py3)
16+ python3 -m flake8 $(python_3_compatible_tests)
17+ flake8 $(python_2_compatible_tests) --builtins xrange,basestring
18 cover:
19 python -m coverage run --source="./" --omit "./tests/*" -m unittest discover -vv ./tests
20 python -m coverage report
21
22=== modified file 'jujupy.py'
23--- jujupy.py 2016-12-08 03:12:58 +0000
24+++ jujupy.py 2016-12-08 03:12:58 +0000
25@@ -1272,19 +1272,11 @@
26 enforced.
27 """
28 version = EnvJujuClient.get_version(juju_path)
29-<<<<<<< TREE
30- client_class = get_client_class(version)
31- if config is None:
32- env = client_class.config_class('', {})
33- else:
34- env = client_class.config_class.from_config(config)
35-=======
36 client_class = get_client_class(str(version))
37 if config is None:
38 env = client_class.config_class('', {})
39 else:
40 env = client_class.config_class.from_config(config)
41->>>>>>> MERGE-SOURCE
42 if juju_path is None:
43 full_path = EnvJujuClient.get_full_path()
44 else:
45
46=== added directory 'tests_py3'
47=== added file 'tests_py3/test_assess_model_change_watcher.py'
48--- tests_py3/test_assess_model_change_watcher.py 1970-01-01 00:00:00 +0000
49+++ tests_py3/test_assess_model_change_watcher.py 2016-12-08 03:12:58 +0000
50@@ -0,0 +1,238 @@
51+#!/usr/bin/env python3
52+
53+import asyncio
54+import logging
55+from io import BytesIO, StringIO, TextIOWrapper
56+import sys
57+
58+
59+from mock import (
60+ Mock,
61+ patch,
62+ )
63+
64+from assess_model_change_watcher import (
65+ assess_model_change_watcher,
66+ _get_message,
67+ is_config_change_in_event,
68+ listen_to_watcher,
69+ main,
70+ parse_args,
71+ run_listener,
72+ )
73+from fakejuju import fake_juju_client
74+from tests import (
75+ parse_error,
76+ TestCase,
77+ )
78+
79+
80+class TestParseArgs(TestCase):
81+ def test_common_args(self):
82+ with patch('utility.warnings'):
83+ args = parse_args(
84+ ["an-env", "/bin/juju", "/tmpp/logs", "an-env-mod"])
85+ self.assertEqual("an-env", args.env)
86+ self.assertEqual("/bin/juju", args.juju_bin)
87+ self.assertEqual("/tmpp/logs", args.logs)
88+ self.assertEqual("an-env-mod", args.temp_env_name)
89+ self.assertEqual(False, args.debug)
90+
91+ def test_help(self):
92+ if isinstance(sys.stdout, TextIOWrapper):
93+ fake_stdout = StringIO()
94+ else:
95+ fake_stdout = BytesIO()
96+ with parse_error(self) as fake_stderr:
97+ with patch("sys.stdout", fake_stdout):
98+ parse_args(["--help"])
99+ self.assertEqual("", fake_stderr.getvalue())
100+
101+
102+class TestMain(TestCase):
103+
104+ def test_main(self):
105+ argv = ["an-env", "/bin/juju", "/tmp/logs", "an-env-mod", "--verbose"]
106+ client = Mock(spec=["is_jes_enabled"])
107+ with patch("assess_model_change_watcher.configure_logging",
108+ autospec=True) as mock_cl:
109+ with patch("assess_model_change_watcher.BootstrapManager."
110+ "booted_context",
111+ autospec=True) as mock_bc:
112+ with patch('deploy_stack.client_from_config',
113+ return_value=client) as mock_cfc:
114+ with patch("assess_model_change_watcher."
115+ "assess_model_change_watcher",
116+ autospec=True) as mock_amcw:
117+ with patch('utility.warnings'):
118+ main(argv)
119+ mock_cl.assert_called_once_with(logging.DEBUG)
120+ mock_cfc.assert_called_once_with('an-env', "/bin/juju", debug=False,
121+ soft_deadline=None)
122+ self.assertEqual(mock_bc.call_count, 1)
123+ mock_amcw.assert_called_once_with(client, None, '/bin/juju')
124+
125+
126+class TestAssess(TestCase):
127+
128+ watcher = [
129+ "application",
130+ "change",
131+ {
132+ "owner-tag": "",
133+ "status": {
134+ "version": "",
135+ "message": "Token is bar",
136+ "current": "active",
137+ "since": "2016-12-01T02:38:03.078457034Z"
138+ },
139+ "min-units": 0,
140+ "life": "alive",
141+ "charm-url": "local:trusty/dummy-source-0",
142+ "exposed": False,
143+ "name": "dummy-source",
144+ "model-uuid": "0b9a4bd8-4980-43cd-8764-cc3ead6ac364",
145+ "constraints": {},
146+ "subordinate": False,
147+ "config": {
148+ "token": "1234asdf"
149+ }
150+ }
151+ ]
152+
153+ watcher_no_config = [
154+ "application",
155+ "change",
156+ {
157+ "owner-tag": "",
158+ "status": {
159+ "version": "",
160+ "message": "Token is bar",
161+ "current": "active",
162+ "since": "2016-12-01T02:38:03.078457034Z"
163+ },
164+ "min-units": 0,
165+ "life": "alive",
166+ "charm-url": "local:trusty/dummy-source-0",
167+ "exposed": False,
168+ "name": "dummy-source",
169+ "model-uuid": "0b9a4bd8-4980-43cd-8764-cc3ead6ac364",
170+ "constraints": {},
171+ "subordinate": False,
172+ }
173+ ]
174+
175+ def test_assess_model_change_watcher(self):
176+ fake_client = Mock(wraps=fake_juju_client())
177+ fake_client.bootstrap()
178+ fake_loop = Mock(spec=['run_until_complete', 'close'])
179+ fake_future = Mock(spec=['result'])
180+ fake_future.result.return_value = True
181+ with patch('assess_model_change_watcher.run_listener',
182+ autospec=True, return_value=(fake_loop, fake_future)
183+ ) as mock_rl:
184+ assess_model_change_watcher(fake_client, 'angsty', 'bin')
185+ fake_client.deploy.assert_called_once_with('dummy-source')
186+ fake_client.wait_for_started.assert_called_once_with()
187+ self.assertEqual(
188+ 1, fake_client.get_status().get_service_unit_count('dummy-source'))
189+ mock_rl.assert_called_once_with(
190+ fake_client, is_config_change_in_event, 'bin')
191+ fake_loop.run_until_complete.assert_called_once_with(fake_future)
192+ fake_future.result.assert_called_once_with()
193+
194+ def test_get_message(self):
195+ event = _get_message(self.watcher)
196+ self.assertEqual(event, self.watcher[2])
197+
198+ def test_is_config_in_event(self):
199+ self.assertIsTrue(is_config_change_in_event(self.watcher))
200+
201+ def test_is_config_in_event_false(self):
202+ watcher = ["application", "change", {"foo": "bar"}]
203+ self.assertIsFalse(is_config_change_in_event(watcher))
204+
205+ def test_run_listener(self):
206+ fake_asyncio = Mock(spec=['get_event_loop', 'Future'])
207+ fake_client = Mock(env=Mock(juju_home='/tmp'), spec=[])
208+ with patch('assess_model_change_watcher.asyncio',
209+ autospec=True, return_value=fake_asyncio) as mock_async:
210+ with patch("assess_model_change_watcher.Connection", autospec=True,
211+ ) as mock_c:
212+ with patch("assess_model_change_watcher.listen_to_watcher",
213+ autospec=True, ) as mock_ltw:
214+ loop, future = run_listener(fake_client, None, 'foo')
215+ mock_async.get_event_loop.assert_called_once_with()
216+ mock_async.Future.assert_called_once_with()
217+ mock_loop = mock_async.get_event_loop.return_value
218+ mock_loop.run_until_complete.assert_called_once_with(
219+ mock_c.connect_current.return_value)
220+ mock_async.ensure_future.assert_called_once_with(mock_ltw(
221+ None, mock_loop.run_until_complete.return_value,
222+ mock_async.Future.return_value))
223+ self.assertEqual(loop, mock_loop)
224+ self.assertEqual(future, mock_async.Future.return_value)
225+
226+ def test_listen_to_watcher(self):
227+ fake_future = Mock(spec=['set_result'])
228+ loop = asyncio.new_event_loop()
229+ asyncio.set_event_loop(None)
230+ with patch("assess_model_change_watcher.watcher.AllWatcher",
231+ autospec=True, return_value=FakeWatcher()) as w_mock:
232+ loop.run_until_complete(listen_to_watcher(
233+ is_config_change_in_event, FakeConn(), fake_future))
234+ w_mock.assert_called_once_with()
235+ fake_future.set_result.assert_called_once_with(True)
236+ loop.close()
237+
238+ def test_listen_to_watcher_no_config(self):
239+ fake_future = Mock(spec=['set_result'])
240+ loop = asyncio.new_event_loop()
241+ asyncio.set_event_loop(None)
242+ with patch("assess_model_change_watcher.watcher.AllWatcher",
243+ autospec=True, return_value=FakeWatcherNoConfig()
244+ ) as w_mock:
245+ with patch("assess_model_change_watcher.until_timeout",
246+ autospec=True, return_value=[1]) as mock_ut:
247+ loop.run_until_complete(listen_to_watcher(
248+ is_config_change_in_event, FakeConn(), fake_future))
249+ w_mock.assert_called_once_with()
250+ fake_future.set_result.assert_called_once_with(False)
251+ mock_ut.assert_called_once_with(120)
252+ loop.close()
253+
254+
255+class FakeConn:
256+
257+ async def close(self):
258+ return
259+
260+
261+class FakeWatcher:
262+
263+ def connect(self, conn):
264+ pass
265+
266+ async def Stop(self):
267+ return
268+
269+ async def Next(self):
270+ class FakeDelta:
271+ deltas = TestAssess.watcher
272+
273+ class FakeChange:
274+ deltas = [FakeDelta()]
275+
276+ return FakeChange()
277+
278+
279+class FakeWatcherNoConfig(FakeWatcher):
280+
281+ async def Next(self):
282+ class FakeDelta:
283+ deltas = TestAssess.watcher_no_config
284+
285+ class FakeChange:
286+ deltas = [FakeDelta()]
287+
288+ return FakeChange()

Subscribers

People subscribed via source and target branches