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

[Seman]

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

[lp:~sseman/juju-ci-tools/model-change-watcher-py3-2]

1796. By Seman on 2016-12-08

Removed event.

[Nicholas Skaggs (nskaggs)]

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

[Curtis Hovey (sinzui)]

Thank you. I think the Makefile changes are incomplete.

review: Needs Information (code)

[Seman (sseman)]

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)]

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.

=== modified file 'Makefile'
--- Makefile	2016-12-08 03:12:58 +0000
+++ Makefile	2016-12-08 03:12:58 +0000
@@ -1,10 +1,13 @@
 p=test*.py
-py3="assess_model_change_watcher.py"
+python_3_compatible_tests := $$(find -name '*.py' | xargs grep -xl '\#!/usr/bin/env python3')
+python_2_compatible_tests := $$(find -name '*.py' | xargs grep -xL '\#!/usr/bin/env python3')
+
 test:
+	TMPDIR=/tmp python3 -m unittest discover -vv ./tests_py3 -p "$(p)"
 	TMPDIR=/tmp python -m unittest discover -vv ./tests -p "$(p)"
 lint:
-	python3 -m flake8 --builtins xrange,basestring $(py3)
-	flake8 $$(find -name '*.py') --builtins xrange,basestring --exclude $(py3)
+	python3 -m flake8 $(python_3_compatible_tests)
+	flake8 $(python_2_compatible_tests) --builtins xrange,basestring
 cover:
 	python -m coverage run --source="./" --omit "./tests/*" -m unittest discover -vv ./tests
 	python -m coverage report
 
=== modified file 'jujupy.py'
--- jujupy.py	2016-12-08 03:12:58 +0000
+++ jujupy.py	2016-12-08 03:12:58 +0000
@@ -1272,19 +1272,11 @@
         enforced.
     """
     version = EnvJujuClient.get_version(juju_path)
-<<<<<<< TREE
-    client_class = get_client_class(version)
-    if config is None:
-        env = client_class.config_class('', {})
-    else:
-        env = client_class.config_class.from_config(config)
-=======
     client_class = get_client_class(str(version))
     if config is None:
         env = client_class.config_class('', {})
     else:
         env = client_class.config_class.from_config(config)
->>>>>>> MERGE-SOURCE
     if juju_path is None:
         full_path = EnvJujuClient.get_full_path()
     else:
 
=== added directory 'tests_py3'
=== added file 'tests_py3/test_assess_model_change_watcher.py'
--- tests_py3/test_assess_model_change_watcher.py	1970-01-01 00:00:00 +0000
+++ tests_py3/test_assess_model_change_watcher.py	2016-12-08 03:12:58 +0000
@@ -0,0 +1,238 @@
+#!/usr/bin/env python3
+
+import asyncio
+import logging
+from io import BytesIO, StringIO, TextIOWrapper
+import sys
+
+
+from mock import (
+    Mock,
+    patch,
+    )
+
+from assess_model_change_watcher import (
+    assess_model_change_watcher,
+    _get_message,
+    is_config_change_in_event,
+    listen_to_watcher,
+    main,
+    parse_args,
+    run_listener,
+    )
+from fakejuju import fake_juju_client
+from tests import (
+    parse_error,
+    TestCase,
+    )
+
+
+class TestParseArgs(TestCase):
+    def test_common_args(self):
+        with patch('utility.warnings'):
+            args = parse_args(
+                ["an-env", "/bin/juju", "/tmpp/logs", "an-env-mod"])
+        self.assertEqual("an-env", args.env)
+        self.assertEqual("/bin/juju", args.juju_bin)
+        self.assertEqual("/tmpp/logs", args.logs)
+        self.assertEqual("an-env-mod", args.temp_env_name)
+        self.assertEqual(False, args.debug)
+
+    def test_help(self):
+        if isinstance(sys.stdout, TextIOWrapper):
+            fake_stdout = StringIO()
+        else:
+            fake_stdout = BytesIO()
+        with parse_error(self) as fake_stderr:
+            with patch("sys.stdout", fake_stdout):
+                parse_args(["--help"])
+        self.assertEqual("", fake_stderr.getvalue())
+
+
+class TestMain(TestCase):
+
+    def test_main(self):
+        argv = ["an-env", "/bin/juju", "/tmp/logs", "an-env-mod", "--verbose"]
+        client = Mock(spec=["is_jes_enabled"])
+        with patch("assess_model_change_watcher.configure_logging",
+                   autospec=True) as mock_cl:
+            with patch("assess_model_change_watcher.BootstrapManager."
+                       "booted_context",
+                       autospec=True) as mock_bc:
+                with patch('deploy_stack.client_from_config',
+                           return_value=client) as mock_cfc:
+                    with patch("assess_model_change_watcher."
+                               "assess_model_change_watcher",
+                               autospec=True) as mock_amcw:
+                        with patch('utility.warnings'):
+                            main(argv)
+        mock_cl.assert_called_once_with(logging.DEBUG)
+        mock_cfc.assert_called_once_with('an-env', "/bin/juju", debug=False,
+                                         soft_deadline=None)
+        self.assertEqual(mock_bc.call_count, 1)
+        mock_amcw.assert_called_once_with(client, None, '/bin/juju')
+
+
+class TestAssess(TestCase):
+
+    watcher = [
+        "application",
+        "change",
+        {
+            "owner-tag": "",
+            "status": {
+                "version": "",
+                "message": "Token is bar",
+                "current": "active",
+                "since": "2016-12-01T02:38:03.078457034Z"
+            },
+            "min-units": 0,
+            "life": "alive",
+            "charm-url": "local:trusty/dummy-source-0",
+            "exposed": False,
+            "name": "dummy-source",
+            "model-uuid": "0b9a4bd8-4980-43cd-8764-cc3ead6ac364",
+            "constraints": {},
+            "subordinate": False,
+            "config": {
+                "token": "1234asdf"
+            }
+        }
+    ]
+
+    watcher_no_config = [
+        "application",
+        "change",
+        {
+            "owner-tag": "",
+            "status": {
+                "version": "",
+                "message": "Token is bar",
+                "current": "active",
+                "since": "2016-12-01T02:38:03.078457034Z"
+            },
+            "min-units": 0,
+            "life": "alive",
+            "charm-url": "local:trusty/dummy-source-0",
+            "exposed": False,
+            "name": "dummy-source",
+            "model-uuid": "0b9a4bd8-4980-43cd-8764-cc3ead6ac364",
+            "constraints": {},
+            "subordinate": False,
+        }
+    ]
+
+    def test_assess_model_change_watcher(self):
+        fake_client = Mock(wraps=fake_juju_client())
+        fake_client.bootstrap()
+        fake_loop = Mock(spec=['run_until_complete', 'close'])
+        fake_future = Mock(spec=['result'])
+        fake_future.result.return_value = True
+        with patch('assess_model_change_watcher.run_listener',
+                   autospec=True, return_value=(fake_loop, fake_future)
+                   ) as mock_rl:
+            assess_model_change_watcher(fake_client, 'angsty', 'bin')
+        fake_client.deploy.assert_called_once_with('dummy-source')
+        fake_client.wait_for_started.assert_called_once_with()
+        self.assertEqual(
+            1, fake_client.get_status().get_service_unit_count('dummy-source'))
+        mock_rl.assert_called_once_with(
+            fake_client, is_config_change_in_event, 'bin')
+        fake_loop.run_until_complete.assert_called_once_with(fake_future)
+        fake_future.result.assert_called_once_with()
+
+    def test_get_message(self):
+        event = _get_message(self.watcher)
+        self.assertEqual(event, self.watcher[2])
+
+    def test_is_config_in_event(self):
+        self.assertIsTrue(is_config_change_in_event(self.watcher))
+
+    def test_is_config_in_event_false(self):
+        watcher = ["application", "change", {"foo": "bar"}]
+        self.assertIsFalse(is_config_change_in_event(watcher))
+
+    def test_run_listener(self):
+        fake_asyncio = Mock(spec=['get_event_loop', 'Future'])
+        fake_client = Mock(env=Mock(juju_home='/tmp'), spec=[])
+        with patch('assess_model_change_watcher.asyncio',
+                   autospec=True, return_value=fake_asyncio) as mock_async:
+            with patch("assess_model_change_watcher.Connection", autospec=True,
+                       ) as mock_c:
+                with patch("assess_model_change_watcher.listen_to_watcher",
+                           autospec=True, ) as mock_ltw:
+                    loop, future = run_listener(fake_client, None, 'foo')
+        mock_async.get_event_loop.assert_called_once_with()
+        mock_async.Future.assert_called_once_with()
+        mock_loop = mock_async.get_event_loop.return_value
+        mock_loop.run_until_complete.assert_called_once_with(
+            mock_c.connect_current.return_value)
+        mock_async.ensure_future.assert_called_once_with(mock_ltw(
+            None, mock_loop.run_until_complete.return_value,
+            mock_async.Future.return_value))
+        self.assertEqual(loop, mock_loop)
+        self.assertEqual(future, mock_async.Future.return_value)
+
+    def test_listen_to_watcher(self):
+        fake_future = Mock(spec=['set_result'])
+        loop = asyncio.new_event_loop()
+        asyncio.set_event_loop(None)
+        with patch("assess_model_change_watcher.watcher.AllWatcher",
+                   autospec=True, return_value=FakeWatcher()) as w_mock:
+            loop.run_until_complete(listen_to_watcher(
+                is_config_change_in_event, FakeConn(), fake_future))
+        w_mock.assert_called_once_with()
+        fake_future.set_result.assert_called_once_with(True)
+        loop.close()
+
+    def test_listen_to_watcher_no_config(self):
+        fake_future = Mock(spec=['set_result'])
+        loop = asyncio.new_event_loop()
+        asyncio.set_event_loop(None)
+        with patch("assess_model_change_watcher.watcher.AllWatcher",
+                   autospec=True, return_value=FakeWatcherNoConfig()
+                   ) as w_mock:
+            with patch("assess_model_change_watcher.until_timeout",
+                       autospec=True, return_value=[1]) as mock_ut:
+                loop.run_until_complete(listen_to_watcher(
+                    is_config_change_in_event, FakeConn(), fake_future))
+        w_mock.assert_called_once_with()
+        fake_future.set_result.assert_called_once_with(False)
+        mock_ut.assert_called_once_with(120)
+        loop.close()
+
+
+class FakeConn:
+
+    async def close(self):
+        return
+
+
+class FakeWatcher:
+
+    def connect(self, conn):
+        pass
+
+    async def Stop(self):
+        return
+
+    async def Next(self):
+        class FakeDelta:
+            deltas = TestAssess.watcher
+
+        class FakeChange:
+            deltas = [FakeDelta()]
+
+        return FakeChange()
+
+
+class FakeWatcherNoConfig(FakeWatcher):
+
+    async def Next(self):
+        class FakeDelta:
+            deltas = TestAssess.watcher_no_config
+
+        class FakeChange:
+            deltas = [FakeDelta()]
+
+        return FakeChange()