Merge lp:~frankban/juju-quickstart/support-old-series into lp:juju-quickstart

[Francesco Banconi]

Enable tests for trusty and utopic.

Fix the unit test suite so that tests pass
when run in trusty and utopic scenarios.

Also add some simple functional tests. In those
tests, Quickstart is run against a real Juju env.

See the changes in the HACKING file for instructions
about running functional tests.

To QA this, run `make check` and `make fcheck`: the
latter can take some time.

https://codereview.appspot.com/197240043/

[Francesco Banconi (frankban)]

Reviewers: mp+248150_code.launchpad.net,

Message:
Please take a look.

Description:
Enable tests for trusty and utopic.

Fix the unit test suite so that tests pass
when run in trusty and utopic scenarios.

Also add some simple functional tests. In those
tests, Quickstart is run against a real Juju env.

See the changes in the HACKING file for instructions
about running functional tests.

To QA this, run `make check` and `make fcheck`: the
latter can take some time.

https://code.launchpad.net/~frankban/juju-quickstart/support-old-series/+merge/248150

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/197240043/

Affected files (+265, -35 lines):
   M HACKING.rst
   M Makefile
   A [revision details]
   M quickstart/juju.py
   M quickstart/manage.py
   M quickstart/platform_support.py
   A quickstart/tests/functional/__init__.py
   A quickstart/tests/functional/test_functional.py
   M quickstart/tests/test_juju.py
   M setup.cfg
   M tox.ini

[Brad Crittenden (bac)]

LGTM. No QA.

https://codereview.appspot.com/197240043/diff/1/quickstart/tests/functional/test_functional.py
File quickstart/tests/functional/test_functional.py (right):

https://codereview.appspot.com/197240043/diff/1/quickstart/tests/functional/test_functional.py#newcode87
quickstart/tests/functional/test_functional.py:87: cls.retcode,
cls.output, cls.error = run_quickstart(cls.env_name)
Thanks for the thoughtful test implementation.

It may be worth noting that run_quickstart only happens once per suite
not once per test as it isn't super obvious since we don't use
setUpClass all that much.

https://codereview.appspot.com/197240043/

[lp:~frankban/juju-quickstart/support-old-series]

138. By Francesco Banconi on 2015-02-03

Changes as per review.

[Francesco Banconi (frankban)]

Please take a look.

https://codereview.appspot.com/197240043/

[Jeff Pihach (hatch)]

LGTM Thanks for getting this working! I'll QA once we chat about my
comment.

https://codereview.appspot.com/197240043/diff/20001/quickstart/tests/functional/test_functional.py
File quickstart/tests/functional/test_functional.py (right):

https://codereview.appspot.com/197240043/diff/20001/quickstart/tests/functional/test_functional.py#newcode46
quickstart/tests/functional/test_functional.py:46: env_name =
envs.get_default_env_name()
As a precautionary measure I would prefer if there was a default
environment name for these tests. That way there would be no possible
way someone would accidentally run these tests on a live environment.

https://codereview.appspot.com/197240043/

[Jeff Pihach (hatch)]

QA OK Thanks for this branch!

https://codereview.appspot.com/197240043/

[Francesco Banconi (frankban)]

*** Submitted:

Enable tests for trusty and utopic.

Fix the unit test suite so that tests pass
when run in trusty and utopic scenarios.

Also add some simple functional tests. In those
tests, Quickstart is run against a real Juju env.

See the changes in the HACKING file for instructions
about running functional tests.

To QA this, run `make check` and `make fcheck`: the
latter can take some time.

R=bac, jeff.pihach
CC=
https://codereview.appspot.com/197240043

https://codereview.appspot.com/197240043/

[Francesco Banconi (frankban)]

Thanks for the reviews!

https://codereview.appspot.com/197240043/

=== modified file 'HACKING.rst'
--- HACKING.rst	2015-01-30 09:28:38 +0000
+++ HACKING.rst	2015-02-03 14:13:23 +0000
@@ -67,6 +67,43 @@
 
 Run ``make help`` for more information about all the available make targets.
 
+Running functional tests
+~~~~~~~~~~~~~~~~~~~~~~~~
+
+The Juju Quickstart test suite includes functional tests exercising the
+application execution against a real Juju environment. Functional tests are
+not run by default (i.e. when using ``make test`` or ``make check``), but they
+can be activated by setting the ``JUJU_QUICKSTART_FTESTS`` environment variable
+to ``1``, or, in a simpler way, by running ``make ftest`` or ``make fcheck``.
+
+Note that functional tests are not intended to be run in the day-by-day
+development process. Their goal is to automate part of the release QA.
+
+Functional tests require:
+
+- some time to complete their execution: a real Juju environment is
+  bootstrapped in the process;
+- a Juju home correctly set up with at least one environment defined in the
+  ``environments.yaml`` file;
+- SSH keys already generated for the user running the tests;
+- a working Internet connection.
+
+By default, the current default environment is used to run those tests.
+To change the environment, use the JUJU_ENV environment variable, e.g.::
+
+    make fcheck JUJU_ENV=myenv
+
+The environment used by the functional suite is destroyed after the tests
+complete. For this reason, **ensure the selected environment is not in use**.
+
+To run the test using a customized build of Juju, pass the ``JUJU`` environment
+variable to ``make``, e.g.::
+
+  make fcheck JUJU=$GOPATH/bin/juju
+
+Also note that when running functional tests against a local environment, the
+password for sudo privileges may be asked while the suite is run.
+
 Requirements
 ~~~~~~~~~~~~
 
@@ -157,6 +194,10 @@
 Pre-release QA
 ~~~~~~~~~~~~~~
 
+Run the Quickstart functional tests on all supported platforms::
+
+  make fcheck
+
 The general steps for manual QA (until we get a continuous integration set up
 with functional tests) should be run on trusty, utopic and vivid.
 

=== modified file 'Makefile'
--- Makefile	2015-01-30 16:34:50 +0000
+++ Makefile	2015-02-03 14:13:23 +0000
@@ -66,6 +66,14 @@
 	# Remove the virtualenv used in previous configurations.
 	rm -rfv .venv/
 
+.PHONY: fcheck
+fcheck: setup
+	$(MAKE) check JUJU_QUICKSTART_FTESTS=1
+
+.PHONY: ftest
+ftest: setup
+	$(MAKE) test JUJU_QUICKSTART_FTESTS=1
+
 .PHONY: help
 help:
 	@echo -e 'Juju Quickstart - list of make targets:\n'
@@ -73,6 +81,8 @@
 	@echo 'make test - Run tests.'
 	@echo 'make lint - Run linter and pep8.'
 	@echo 'make check - Run all the tests and lint in all supported scenarios.'
+	@echo 'make ftest - Run tests (including functional tests).'
+	@echo 'make fcheck - Run functional tests and lint in all scenarios.'
 	@echo 'make source - Create source package.'
 	@echo 'make install - Install on local system.'
 	@echo 'make clean - Get rid of bytecode files, build and dist dirs, venvs.'

=== modified file 'quickstart/juju.py'
--- quickstart/juju.py	2014-12-16 11:10:10 +0000
+++ quickstart/juju.py	2015-02-03 14:13:23 +0000
@@ -148,7 +148,7 @@
         Overridden to add logging in the case the payload is text.
         """
         if opcode == OPCODE_TEXT:
-            message = payload.decode('utf-8')
+            message = payload.decode('utf-8', 'ignore')
             logging.debug('API message: --> {}'.format(message))
         return super(WebSocketConnection, self).send(payload, opcode=opcode)
 
@@ -159,6 +159,6 @@
         """
         data = super(WebSocketConnection, self).recv()
         if isinstance(data, bytes):
-            message = data.decode('utf-8')
+            message = data.decode('utf-8', 'ignore')
             logging.debug('API message: <-- {}'.format(message))
         return data

=== modified file 'quickstart/manage.py'
--- quickstart/manage.py	2015-01-12 12:10:38 +0000
+++ quickstart/manage.py	2015-02-03 14:13:23 +0000
@@ -386,7 +386,6 @@
 
     Exit with an error if the provided arguments are not valid.
     """
-
     # Determine the host platform.  This needs to be done early as it affects
     # the options we present.
     platform = platform_support.get_platform()

=== modified file 'quickstart/platform_support.py'
--- quickstart/platform_support.py	2014-08-25 16:57:41 +0000
+++ quickstart/platform_support.py	2015-02-03 14:13:23 +0000
@@ -55,10 +55,7 @@
 
 
 def get_platform():
-    """Return the platform of the host.
-
-    Raises UnsupportedOS if a platform we don't support is detected.
-    """
+    """Return the platform of the host."""
     system = platform.system()
     if system == 'Darwin':
         return settings.OSX

=== added directory 'quickstart/tests/functional'
=== added file 'quickstart/tests/functional/__init__.py'
--- quickstart/tests/functional/__init__.py	1970-01-01 00:00:00 +0000
+++ quickstart/tests/functional/__init__.py	2015-02-03 14:13:23 +0000
@@ -0,0 +1,41 @@
+# This file is part of the Juju Quickstart Plugin, which lets users set up a
+# Juju environment in very few steps (https://launchpad.net/juju-quickstart).
+# Copyright (C) 2015 Canonical Ltd.
+#
+# This program is free software: you can redistribute it and/or modify it under
+# the terms of the GNU Affero General Public License version 3, as published by
+# the Free Software Foundation.
+#
+# This program is distributed in the hope that it will be useful, but WITHOUT
+# ANY WARRANTY; without even the implied warranties of MERCHANTABILITY,
+# SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# Affero General Public License for more details.
+#
+# You should have received a copy of the GNU Affero General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+"""Juju Quickstart functional tests package.
+
+This package includes functional tests for Juju Quickstart.
+In those tests, Juju Quickstart is run to bootstrap and deploy the Juju GUI
+in a real Juju environment. For this reason, completing the suite takes a
+while. Therefore tests are disabled by default, and are intended to be only run
+before releasing a new version of the application.
+
+To run the tests, set the JUJU_QUICKSTART_FTESTS environment variable to "1",
+or use "make ftest/fcheck".
+
+Functional tests require:
+- a Juju home correctly set up with at least one environment defined in the
+  environments.yaml file;
+- SSH keys already generated for the user running the tests;
+- a working Internet connection.
+
+By default, the current default environment is used to run the application.
+To change the environment, set the JUJU_ENV environment variable, e.g.:
+
+    make fcheck JUJU_ENV=myenv
+
+Also note that when running functional tests against a local environment, the
+password for sudo privileges may be asked while the suite is run.
+"""

=== added file 'quickstart/tests/functional/test_functional.py'
--- quickstart/tests/functional/test_functional.py	1970-01-01 00:00:00 +0000
+++ quickstart/tests/functional/test_functional.py	2015-02-03 14:13:23 +0000
@@ -0,0 +1,139 @@
+# This file is part of the Juju Quickstart Plugin, which lets users set up a
+# Juju environment in very few steps (https://launchpad.net/juju-quickstart).
+# Copyright (C) 2015 Canonical Ltd.
+#
+# This program is free software: you can redistribute it and/or modify it under
+# the terms of the GNU Affero General Public License version 3, as published by
+# the Free Software Foundation.
+#
+# This program is distributed in the hope that it will be useful, but WITHOUT
+# ANY WARRANTY; without even the implied warranties of MERCHANTABILITY,
+# SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# Affero General Public License for more details.
+#
+# You should have received a copy of the GNU Affero General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+"""Functional tests for Juju Quickstart."""
+
+from __future__ import unicode_literals
+
+import functools
+import json
+import os
+import unittest
+
+import quickstart
+from quickstart import (
+    platform_support,
+    settings,
+    utils,
+)
+from quickstart.models import envs
+
+
+# Define the name of the environment variable used to run the functional tests.
+FTEST_ENV_VAR = 'JUJU_QUICKSTART_FTESTS'
+
+
+def skip_if_disbled(func):
+    """Decorate a test method so that it is only run when required/possible."""
+    # Check that functional tests are enabled.
+    if os.getenv(FTEST_ENV_VAR) != '1':
+        return unittest.skip(
+            'to run functional tests, set {} to "1"'.format(FTEST_ENV_VAR))
+    # Check that a Juju environment can be found.
+    env_name = envs.get_default_env_name()
+    if env_name is None:
+        return unittest.skip('cannot find a configured Juju environment')
+
+    @functools.wraps(func)
+    def decorated(cls_or_self):
+        # Decorate the function to update the received class or instance with
+        # the Juju env name.
+        cls_or_self.env_name = env_name
+        return func(cls_or_self)
+
+    return decorated
+
+
+def run_juju(*args):
+    """Run the juju command with the given args.
+
+    Return a tuple including the command exit code, its output and error.
+    """
+    platform = platform_support.get_platform()
+    cmd, _ = platform_support.get_juju_command(platform)
+    return utils.call(cmd, *args)
+
+
+def run_quickstart(env_name, *args):
+    """Run the Juju Quickstart command.
+
+    Return a tuple including the command exit code, its output and error.
+    """
+    package_dir = os.path.dirname(quickstart.__file__)
+    cmd = os.path.abspath(os.path.join(package_dir, '..', 'juju-quickstart'))
+    return utils.call(
+        cmd, '-e', env_name, '--distro-only', '--no-browser', *args)
+
+
+class TestFunctional(unittest.TestCase):
+
+    @classmethod
+    @skip_if_disbled
+    def setUpClass(cls):
+        # Run Juju Quickstart to bootstrap Juju and deploy the Juju GUI.
+        # Note that this is done once per suite. The resulting environment is
+        # then destroyed when the suite completes.
+        cls.retcode, cls.output, cls.error = run_quickstart(cls.env_name)
+
+    @classmethod
+    @skip_if_disbled
+    def tearDownClass(cls):
+        # Destroy the environment.
+        run_juju('destroy-environment', cls.env_name, '-y', '--force')
+
+    def test_executed(self):
+        # The application successfully completed its execution.
+        self.assertEqual(0, self.retcode)
+        self.assertIn('done!', self.output)
+        self.assertEqual('', self.error)
+
+    def test_gui_started(self):
+        # At the end of the process, the Juju GUI unit is started.
+        retcode, output, _ = run_juju(
+            'status', '-e', self.env_name, '--format', 'json')
+        self.assertEqual(0, retcode)
+        status = json.loads(output)
+        # The Juju GUI service is exposed.
+        service = status['services'][settings.JUJU_GUI_SERVICE_NAME]
+        self.assertTrue(service['exposed'])
+        # The Juju GUI unit is started.
+        unit = service['units']['{}/0'.format(settings.JUJU_GUI_SERVICE_NAME)]
+        self.assertEqual('started', unit['agent-state'])
+
+    def test_idempotent(self):
+        # The application can be run again on an already bootstrapped
+        # environment.
+        retcode, output, error = run_quickstart(self.env_name)
+        self.assertEqual(0, retcode)
+        msg = 'reusing the already bootstrapped {} environment'
+        self.assertIn(msg.format(self.env_name), output)
+        self.assertEqual('', error)
+
+    def test_version(self):
+        # The application correctly prints out its own version.
+        retcode, output, error = run_quickstart(self.env_name, '--version')
+        self.assertEqual(0, retcode)
+        self.assertEqual('', output)
+        self.assertEqual(
+            'juju-quickstart {}\n'.format(quickstart.get_version()), error)
+
+    def test_bundle_deployment(self):
+        # The application can be used to deploy bundles.
+        retcode, output, error = run_quickstart(
+            self.env_name, 'bundle:mediawiki/single')
+        self.assertEqual(0, retcode)
+        self.assertIn('bundle deployment request accepted', output)
+        self.assertEqual('', error)

=== modified file 'quickstart/tests/test_juju.py'
--- quickstart/tests/test_juju.py	2014-12-16 15:53:25 +0000
+++ quickstart/tests/test_juju.py	2015-02-03 14:13:23 +0000
@@ -63,8 +63,13 @@
         api_url = self.api_url
         with mock.patch('websocket.create_connection') as mock_connect:
             self.env = juju.Environment(api_url)
-        mock_connect.assert_called_once_with(
-            api_url, origin=api_url, sslopt=juju.SSLOPT)
+        # In old versions of jujuclient the SSL options are not passed as
+        # kwargs to create_connection.
+        if len(mock_connect.call_args[1]) == 1:
+            mock_connect.assert_called_once_with(api_url, origin=api_url)
+        else:
+            mock_connect.assert_called_once_with(
+                api_url, origin=api_url, sslopt=juju.SSLOPT)
         # Keep track of watcher changes in the changesets list.
         self.changesets = []
 
@@ -345,49 +350,52 @@
 
     def setUp(self):
         self.conn = juju.WebSocketConnection()
-        # The send method calls the send_frame one.
-        self.conn.send_frame = self.mock_send = mock.Mock()
-        # The recv method calls the recv_data one.
-        self.conn.recv_data = self.mock_recv = mock.Mock()
 
     def test_send(self):
         # Outgoing messages are properly logged.
         with helpers.assert_logs(['API message: --> my message'], 'debug'):
-            self.conn.send('my message')
-        self.assertTrue(self.mock_send.called)
+            with mock.patch('websocket.WebSocket.send') as mock_send:
+                self.conn.send('my message')
+        mock_send.assert_called_once_with(
+            'my message', opcode=juju.OPCODE_TEXT)
 
     def test_send_unicode(self):
         # Outgoing unicode messages are properly logged.
         expected = 'API message: --> {}'.format(self.snowman)
         with helpers.assert_logs([expected], 'debug'):
-            self.conn.send(self.snowman.encode('utf-8'))
-        self.assertTrue(self.mock_send.called)
+            with mock.patch('websocket.WebSocket.send') as mock_send:
+                self.conn.send(self.snowman.encode('utf-8'))
+        mock_send.assert_called_once_with(
+            self.snowman.encode('utf-8'), opcode=juju.OPCODE_TEXT)
 
     def test_send_not_text(self):
         # Outgoing non-textual messages are not logged.
         with helpers.assert_logs([], 'debug'):
-            self.conn.send(0x0, opcode=websocket.ABNF.OPCODE_BINARY)
-        self.assertTrue(self.mock_send.called)
+            with mock.patch('websocket.WebSocket.send') as mock_send:
+                self.conn.send(0x0, opcode=websocket.ABNF.OPCODE_BINARY)
+        mock_send.assert_called_once_with(
+            0x0, opcode=websocket.ABNF.OPCODE_BINARY)
 
     def test_recv(self):
         # Incoming messages are properly logged.
-        self.mock_recv.return_value = (juju.OPCODE_TEXT, b'my message')
         with helpers.assert_logs(['API message: <-- my message'], 'debug'):
-            self.conn.recv()
-        self.mock_recv.assert_called_once_with()
+            with mock.patch('websocket.WebSocket.recv') as mock_recv:
+                mock_recv.return_value = (b'my message')
+                message = self.conn.recv()
+        self.assertEqual('my message', message)
 
     def test_recv_unicode(self):
         # Incoming unicode messages are properly logged.
-        self.mock_recv.return_value = (
-            juju.OPCODE_TEXT, self.snowman.encode('utf-8'))
         expected = 'API message: <-- {}'.format(self.snowman)
         with helpers.assert_logs([expected], 'debug'):
-            self.conn.recv()
-        self.mock_recv.assert_called_once_with()
+            with mock.patch('websocket.WebSocket.recv') as mock_recv:
+                mock_recv.return_value = self.snowman.encode('utf-8')
+                message = self.conn.recv()
+        self.assertEqual(self.snowman.encode('utf-8'), message)
 
     def test_recv_not_text(self):
         # Incoming non-textual messages are not logged.
-        self.mock_recv.return_value = (websocket.ABNF.OPCODE_BINARY, 0x0)
         with helpers.assert_logs([], 'debug'):
-            self.conn.recv()
-        self.mock_recv.assert_called_once_with()
+            with mock.patch('websocket.WebSocket.recv') as mock_recv:
+                mock_recv.return_value = 0x0
+                self.conn.recv()

=== modified file 'setup.cfg'
--- setup.cfg	2015-01-16 12:32:38 +0000
+++ setup.cfg	2015-02-03 14:13:23 +0000
@@ -15,6 +15,5 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 [nosetests]
-detailed-errors=1
 nocapture=1
 verbosity=2

=== modified file 'tox.ini'
--- tox.ini	2015-01-30 09:28:38 +0000
+++ tox.ini	2015-02-03 14:13:23 +0000
@@ -18,11 +18,7 @@
 # The envlist option must only include platform scenarios. In essence, those
 # are used to test Juju Quickstart with the specific dependencies present in
 # each supported OS platform.
-envlist = pypi,ppa,vivid
-# XXX frankban: currently tests do not pass on trusty and utopic scenarios.
-# Fix the tests, then remove the line above and uncomment the line below.
-# envlist = pypi,ppa,trusty,utopic,vivid
-
+envlist = pypi,ppa,trusty,utopic,vivid
 
 # Define the base requirements and commands to use when testing the program.