Merge into trunk : new-test-and-server-deps : Trusty Tahr (14.04) : Code : juju-gui package : Juju Charms Collection

Status:	Merged
Merged at revision:	199
Proposed branch:	lp://staging/~frankban/charms/trusty/juju-gui/new-test-and-server-deps
Merge into:	lp://staging/~juju-gui/charms/trusty/juju-gui/trunk
Diff against target:	277 lines (+58/-41) 5 files modified server-requirements.pip (+5/-3) test-requirements.pip (+1/-1) tests/20-functional.test (+17/-13) tests/helpers.py (+17/-7) tests/test_helpers.py (+18/-17)
To merge this branch:	bzr merge lp://staging/~frankban/charms/trusty/juju-gui/new-test-and-server-deps
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
Juju GUI Hackers		2014-09-12	Pending
Review via email: mp+234469@code.staging.launchpad.net

Description of the change

Update dependencies + other fixes.

Update the GUI server dependencies:
websocket-client, jujuclient, juju=deployer.
Also included in the dependencies repo
the new websocket-client requirements.

Update selenium to the latest version
(2.43.0) so that the functional tests
pass on Firefox 32. The download cache
has been updated accordingly.

Fix how the suite detects if it is running
on a local environment: do not use the
environment name (unreliable), but retrieve
the environment type instead.

Fix the WebSocket client used when running
functional tests to reflect changes in the
new websocket-client.

QA:
run the test suite on precise and trusty:
assuming your ec2 trusty environment is
called ec2-trusty and your ec2 precise one
is called ec2-precise, run the following:

make test JUJU_ENV=ec2-trusty
make test JUJU_ENV=ec2-precise

As you already know, the functional tests
take a while.

https://codereview.appspot.com/136570043/

Revision history for this message

Francesco Banconi (frankban) wrote on 2014-09-12:

#

Download full text (12.7 KiB)

Reviewers: mp+234469_code.launchpad.net,

Message:
Please take a look.

Description:
Update dependencies + other fixes.

Update the GUI server dependencies:
websocket-client, jujuclient, juju=deployer.
Also included in the dependencies repo
the new websocket-client requirements.

Update selenium to the latest version
(2.43.0) so that the functional tests
pass on Firefox 32. The download cache
has been updated accordingly.

Fix how the suite detects if it is running
on a local environment: do not use the
environment name (unreliable), but retrieve
the environment type instead.

Fix the WebSocket client used when running
functional tests to reflect changes in the
new websocket-client.

QA:
run the test suite on precise and trusty:
assuming your ec2 trusty environment is
called ec2-trusty and your ec2 precise one
is called ec2-precise, run the following:

make test JUJU_ENV=ec2-trusty
make test JUJU_ENV=ec2-precise

As you already know, the functional tests
take a while.

https://code.launchpad.net/~frankban/charms/trusty/juju-gui/new-test-and-server-deps/+merge/234469

(do not edit description out of merge proposal)

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

Affected files (+52, -33 lines):
   A [revision details]
   A deps/backports.ssl_match_hostname-3.4.0.2.tar.gz
   D deps/juju-deployer-0.3.9.tar.gz
   A deps/juju-deployer-0.4.0.tar.gz
   D deps/jujuclient-0.17.5.tar.gz
   A deps/jujuclient-0.18.4.tar.gz
   A deps/six-1.8.0.tar.gz
   D deps/websocket-client-0.12.0.tar.gz
   A deps/websocket-client-0.18.0.tar.gz
   M server-requirements.pip
   M test-requirements.pip
   M tests/20-functional.test
   M tests/helpers.py
   M tests/test_helpers.py

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision:
<email address hidden>
+New revision:
<email address hidden>

Index: deps/backports.ssl_match_hostname-3.4.0.2.tar.gz
=== added file 'deps/backports.ssl_match_hostname-3.4.0.2.tar.gz'
Binary files deps/backports.ssl_match_hostname-3.4.0.2.tar.gz 1970-01-01
00:00:00 +0000 and deps/backports.ssl_match_hostname-3.4.0.2.tar.gz
2014-09-12 11:00:01 +0000 differ

Index: deps/juju-deployer-0.3.9.tar.gz
=== removed file 'deps/juju-deployer-0.3.9.tar.gz'
Binary files deps/juju-deployer-0.3.9.tar.gz 2014-08-28 20:23:34 +0000 and
deps/juju-deployer-0.3.9.tar.gz 1970-01-01 00:00:00 +0000 differ

Index: deps/juju-deployer-0.4.0.tar.gz
=== added file 'deps/juju-deployer-0.4.0.tar.gz'
Binary files deps/juju-deployer-0.4.0.tar.gz 1970-01-01 00:00:00 +0000 and
deps/juju-deployer-0.4.0.tar.gz 2014-09-12 11:00:01 +0000 differ

Index: deps/jujuclient-0.17.5.tar.gz
=== removed file 'deps/jujuclient-0.17.5.tar.gz'
Binary files deps/jujuclient-0.17.5.tar.gz 2014-03-17 11:16:15 +0000 and
deps/jujuclient-0.17.5.tar.gz 1970-01-01 00:00:00 +0000 differ

Index: deps/jujuclient-0.18.4.tar.gz
=== added file 'deps/jujuclient-0.18.4.tar.gz'
Binary files deps/jujuclient-0.18.4.tar.gz 1970-01-01 00:00:00 +0000 and
de...

Reviewers: mp+234469_code.launchpad.net,

Message:
Please take a look.

Description:
Update dependencies + other fixes.

Update the GUI server dependencies:
websocket-client, jujuclient, juju=deployer.
Also included in the dependencies repo
the new websocket-client requirements.

Update selenium to the latest version
(2.43.0) so that the functional tests
pass on Firefox 32. The download cache
has been updated accordingly.

Fix how the suite detects if it is running
on a local environment: do not use the
environment name (unreliable), but retrieve
the environment type instead.

Fix the WebSocket client used when running
functional tests to reflect changes in the
new websocket-client.

QA:
run the test suite on precise and trusty:
assuming your ec2 trusty environment is
called ec2-trusty and your ec2 precise one
is called ec2-precise, run the following:

make test JUJU_ENV=ec2-trusty
     make test JUJU_ENV=ec2-precise

As you already know, the functional tests
take a while.

https://code.launchpad.net/~frankban/charms/trusty/juju-gui/new-test-and-server-deps/+merge/234469

(do not edit description out of merge proposal)

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

Affected files (+52, -33 lines):
   A [revision details]
   A deps/backports.ssl_match_hostname-3.4.0.2.tar.gz
   D deps/juju-deployer-0.3.9.tar.gz
   A deps/juju-deployer-0.4.0.tar.gz
   D deps/jujuclient-0.17.5.tar.gz
   A deps/jujuclient-0.18.4.tar.gz
   A deps/six-1.8.0.tar.gz
   D deps/websocket-client-0.12.0.tar.gz
   A deps/websocket-client-0.18.0.tar.gz
   M server-requirements.pip
   M test-requirements.pip
   M tests/20-functional.test
   M tests/helpers.py
   M tests/test_helpers.py

Index: [revision details]
=== added file '[revision details]'
--- [revision details]	2012-01-01 00:00:00 +0000
+++ [revision details]	2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision:  
francesco.banconi@canonical.com-20140909082542-16336j16rxj286t9
+New revision:  
francesco.banconi@canonical.com-20140912112344-pa49ian1xvfihd66

Index: deps/backports.ssl_match_hostname-3.4.0.2.tar.gz
=== added file 'deps/backports.ssl_match_hostname-3.4.0.2.tar.gz'
Binary files deps/backports.ssl_match_hostname-3.4.0.2.tar.gz	1970-01-01  
00:00:00 +0000 and deps/backports.ssl_match_hostname-3.4.0.2.tar.gz	 
2014-09-12 11:00:01 +0000 differ

Index: deps/juju-deployer-0.3.9.tar.gz
=== removed file 'deps/juju-deployer-0.3.9.tar.gz'
Binary files deps/juju-deployer-0.3.9.tar.gz	2014-08-28 20:23:34 +0000 and  
deps/juju-deployer-0.3.9.tar.gz	1970-01-01 00:00:00 +0000 differ

Index: deps/juju-deployer-0.4.0.tar.gz
=== added file 'deps/juju-deployer-0.4.0.tar.gz'
Binary files deps/juju-deployer-0.4.0.tar.gz	1970-01-01 00:00:00 +0000 and  
deps/juju-deployer-0.4.0.tar.gz	2014-09-12 11:00:01 +0000 differ

Index: deps/jujuclient-0.17.5.tar.gz
=== removed file 'deps/jujuclient-0.17.5.tar.gz'
Binary files deps/jujuclient-0.17.5.tar.gz	2014-03-17 11:16:15 +0000 and  
deps/jujuclient-0.17.5.tar.gz	1970-01-01 00:00:00 +0000 differ

Index: deps/jujuclient-0.18.4.tar.gz
=== added file 'deps/jujuclient-0.18.4.tar.gz'
Binary files deps/jujuclient-0.18.4.tar.gz	1970-01-01 00:00:00 +0000 and  
deps/jujuclient-0.18.4.tar.gz	2014-09-12 11:00:01 +0000 differ

Index: deps/six-1.8.0.tar.gz
=== added file 'deps/six-1.8.0.tar.gz'
Binary files deps/six-1.8.0.tar.gz	1970-01-01 00:00:00 +0000 and  
deps/six-1.8.0.tar.gz	2014-09-12 11:00:01 +0000 differ

Index: deps/websocket-client-0.12.0.tar.gz
=== removed file 'deps/websocket-client-0.12.0.tar.gz'
Binary files deps/websocket-client-0.12.0.tar.gz	2013-10-04 12:53:53 +0000  
and deps/websocket-client-0.12.0.tar.gz	1970-01-01 00:00:00 +0000 differ

Index: server-requirements.pip
=== modified file 'server-requirements.pip'
--- server-requirements.pip	2014-08-28 20:23:34 +0000
+++ server-requirements.pip	2014-09-12 10:58:48 +0000
@@ -25,7 +25,9 @@

futures==2.1.4
  tornado==3.1.1
-websocket-client==0.12.0
-jujuclient==0.17.5
-juju-deployer==0.3.9
+six==1.8.0
+backports.ssl_match_hostname==3.4.0.2
+websocket-client==0.18.0
+jujuclient==0.18.4
+juju-deployer==0.4.0
  charmworldlib==0.3.0

Index: test-requirements.pip
=== modified file 'test-requirements.pip'
--- test-requirements.pip	2014-01-09 13:51:29 +0000
+++ test-requirements.pip	2014-09-12 10:58:48 +0000
@@ -43,7 +43,7 @@
  mock==1.0.1

# Charm tests.
-selenium==2.39.0
+selenium==2.43.0
  xvfbwrapper==0.2.2

Index: deps/websocket-client-0.18.0.tar.gz
=== added file 'deps/websocket-client-0.18.0.tar.gz'
Binary files deps/websocket-client-0.18.0.tar.gz	1970-01-01 00:00:00 +0000  
and deps/websocket-client-0.18.0.tar.gz	2014-09-12 11:00:01 +0000 differ

Index: tests/20-functional.test
=== modified file 'tests/20-functional.test'
--- tests/20-functional.test	2014-07-10 18:59:23 +0000
+++ tests/20-functional.test	2014-09-12 11:23:44 +0000
@@ -34,7 +34,7 @@
  # DeployTestMixin.juju_deploy instead.  See comment in the method.
  from deploy import juju_deploy
  from helpers import (
-    get_admin_secret,
+    get_env_attr,
      juju_destroy_service,
      juju_status,
      make_service_name,
@@ -45,22 +45,26 @@
  JUJU_GUI_TEST_BRANCH = 'https://github.com/juju/juju-gui.git master'

try:
-    admin_secret = get_admin_secret()
+    admin_secret = get_env_attr('admin-secret')
  except ValueError as err:
      admin_secret = None
      print(err)
+
+# If the suite is using a local environment, then the charm cannot be
+# co-located to machine 0, and a new machine must be created for each charm
+# deployment.
+try:
+    env_type = get_env_attr('type')
+except ValueError as err:
+    # This should never happen, but if it does, defensively assume a new
+    # machine is always required.
+    force_machine = None
+    print(err)
+else:
+    force_machine = None if env_type == 'local' else 0
+
  bootstrap_node_series = juju_status()['machines']['0']['series']

-"""
-If JUJU_ENV is defined to local, then the --to is not
-used as it is not needed in the local env. Otherwise, the
-default placing of the deployed GUI charm is on machine 0
-with a --to 0
-"""
-forced_machine = 0
-env_par = os.getenv('JUJU_ENV')
-if env_par == "local":
-    forced_machine = None

def juju_deploy_gui(options=None):
      """Deploy the Juju GUI charm with the given options.
@@ -73,7 +77,7 @@
      service_name = make_service_name(prefix='juju-gui-')
      unit_info = juju_deploy(
          'juju-gui', service_name=service_name, options=options,
-        force_machine=forced_machine, series=bootstrap_node_series)
+        force_machine=force_machine, series=bootstrap_node_series)
      return service_name, unit_info

Index: tests/helpers.py
=== modified file 'tests/helpers.py'
--- tests/helpers.py	2014-01-14 23:57:37 +0000
+++ tests/helpers.py	2014-09-12 11:23:44 +0000
@@ -23,6 +23,7 @@
  import random
  import string
  import subprocess
+import ssl
  import time

import websocket
@@ -106,10 +107,13 @@
      return decorator

-def get_admin_secret():
-    """Return the admin secret for the current environment.
+def get_env_attr(attr):
+    """Return the requested attribute for the current environment.

+    The attr argument is the key included in the current environment  
section
+    in ~/.juju/environments.yaml.
      The environment name must be present in the JUJU_ENV env variable.
+
      Raise a ValueError if the environment is not found in the context or  
the
      given environment name is not included in ~/.juju/environments.yaml.
      """
@@ -134,10 +138,10 @@
          raise ValueError('Invalid YAML contents: {}'.format(environments))
      if environment is None:
          raise ValueError('Environment {} not found'.format(env))
-    admin_secret = environment.get('admin-secret')
-    if admin_secret is None:
-        raise ValueError('Admin secret not found')
-    return admin_secret
+    value = environment.get(attr)
+    if value is None:
+        raise ValueError('Attribute {} not found'.format(attr))
+    return value

@retry(ProcessError)
@@ -210,6 +214,12 @@
              return unit

+SSLOPT = {
+    'cert_reqs': ssl.CERT_NONE,
+    'ssl_version': ssl.PROTOCOL_TLSv1,
+}
+
+
  class WebSocketClient(object):
      """A simple blocking WebSocket client used in functional tests."""

@@ -219,7 +229,7 @@

def connect(self):
          """Connect to the WebSocket server."""
-        self._conn = websocket.create_connection(self._url)
+        self._conn = websocket.create_connection(self._url, sslopt=SSLOPT)

def send(self, request):
          """Send the given WebSocket request.

Index: tests/test_helpers.py
=== modified file 'tests/test_helpers.py'
--- tests/test_helpers.py	2014-01-14 23:57:37 +0000
+++ tests/test_helpers.py	2014-09-12 11:23:44 +0000
@@ -28,13 +28,14 @@

from helpers import (
      command,
-    get_admin_secret,
+    get_env_attr,
      juju,
      juju_destroy_service,
      juju_env,
      juju_status,
      ProcessError,
      retry,
+    SSLOPT,
      stop_services,
      wait_for_unit,
      WebSocketClient,
@@ -270,11 +271,11 @@
          self.assertGreater(mock_callable.call_count, 1)

-class TestGetAdminSecret(unittest.TestCase):
+class TestGetEnvAttr(unittest.TestCase):

def mock_environment_file(self, contents, juju_env=None):
          """Create a mock environment file containing the given contents."""
-        # Create a temporary home that will be used by get_admin_secret().
+        # Create a temporary home that will be used by get_env_attr().
          home = tempfile.mkdtemp()
          self.addCleanup(shutil.rmtree, home)
          # Create a juju home.
@@ -308,7 +309,7 @@
          expected = 'Unable to retrieve the current environment name.'
          with self.mock_environment_file(''):
              with self.assert_error(expected):
-                get_admin_secret()
+                get_env_attr('attr-name')

def test_no_file(self):
          # A ValueError is raised if the environments file is not found.
@@ -317,42 +318,42 @@
          expected = 'Unable to open environments file: [Errno 2] No such  
file'
          with mock.patch('os.environ', {'HOME': home, 'JUJU_ENV': 'ec2'}):
              with self.assert_error(expected):
-                get_admin_secret()
+                get_env_attr('attr-name')

def test_invalid_yaml(self):
          # A ValueError is raised if the environments file is not well  
formed.
          with self.mock_environment_file(':', juju_env='ec2'):
              with self.assert_error('Unable to parse environments file:'):
-                get_admin_secret()
+                get_env_attr('attr-name')

def test_invalid_yaml_contents(self):
          # A ValueError is raised if the environments file is not well  
formed.
          with self.mock_environment_file('a-string', juju_env='ec2'):
              with self.assert_error('Invalid YAML contents: a-string'):
-                get_admin_secret()
+                get_env_attr('attr-name')

def test_no_env(self):
          # A ValueError is raised if the environment is not found in the  
YAML.
          contents = yaml.safe_dump({'environments': {'local': {}}})
          with self.mock_environment_file(contents, juju_env='ec2'):
              with self.assert_error('Environment ec2 not found'):
-                get_admin_secret()
+                get_env_attr('attr-name')

-    def test_no_admin_secret(self):
-        # A ValueError is raised if the admin secret is not included in the
-        # environment info.
+    def test_attribute_not_found(self):
+        # A ValueError is raised if the requested attribute is not  
included in
+        # the environment info.
          contents = yaml.safe_dump({'environments': {'ec2': {}}})
          with self.mock_environment_file(contents, juju_env='ec2'):
-            with self.assert_error('Admin secret not found'):
-                get_admin_secret()
+            with self.assert_error('Attribute attr-name not found'):
+                get_env_attr('attr-name')

def test_ok(self):
-        # The environment admin secret is correctly returned.
+        # The environment attribute value is correctly returned.
          contents = yaml.safe_dump({
-            'environments': {'ec2': {'admin-secret': 'Secret!'}},
+            'environments': {'ec2': {'attr-name': 'Value!'}},
          })
          with self.mock_environment_file(contents, juju_env='ec2'):
-            self.assertEqual('Secret!', get_admin_secret())
+            self.assertEqual('Value!', get_env_attr('attr-name'))

@mock.patch('helpers.ssh')
@@ -468,7 +469,7 @@
      def test_connect(self, mock_create_connection):
          # The WebSocket connection is correctly established.
          self.client.connect()
-        mock_create_connection.assert_called_once_with(self.url)
+        mock_create_connection.assert_called_once_with(self.url,  
sslopt=SSLOPT)

def test_send(self, mock_create_connection):
          # A request is correctly sent, and a response is returned.

Revision history for this message

Richard Harding (rharding) wrote on 2014-09-12:

#

code looks good, starting up a QA/test run on trusty if the other
reviewer could do precise that'd be great.

https://codereview.appspot.com/136570043/

Revision history for this message

Richard Harding (rharding) wrote on 2014-09-12:

#

Ran 12 tests in 1809.282s

QA good on trusty LGTM

https://codereview.appspot.com/136570043/

Revision history for this message

Madison Scott-Clary (makyo) wrote on 2014-09-12:

#

LGTM, QA okay in precise.

https://codereview.appspot.com/136570043/

Revision history for this message

Francesco Banconi (frankban) wrote on 2014-09-12:

#

*** Submitted:

Update dependencies + other fixes.

Update the GUI server dependencies:
websocket-client, jujuclient, juju=deployer.
Also included in the dependencies repo
the new websocket-client requirements.

Update selenium to the latest version
(2.43.0) so that the functional tests
pass on Firefox 32. The download cache
has been updated accordingly.

Fix how the suite detects if it is running
on a local environment: do not use the
environment name (unreliable), but retrieve
the environment type instead.

Fix the WebSocket client used when running
functional tests to reflect changes in the
new websocket-client.

QA:
run the test suite on precise and trusty:
assuming your ec2 trusty environment is
called ec2-trusty and your ec2 precise one
is called ec2-precise, run the following:

make test JUJU_ENV=ec2-trusty
make test JUJU_ENV=ec2-precise

As you already know, the functional tests
take a while.

R=rharding, matthew.scott
CC=
https://codereview.appspot.com/136570043

https://codereview.appspot.com/136570043/

Revision history for this message

Francesco Banconi (frankban) wrote on 2014-09-12:

#

Thank you both for the reviews!

https://codereview.appspot.com/136570043/

Juju Charms Collection
juju-gui package

Merge lp://staging/~frankban/charms/trusty/juju-gui/new-test-and-server-deps into lp://staging/~juju-gui/charms/trusty/juju-gui/trunk

Commit message

Description of the change

Preview Diff

Subscribers

Juju Charms Collectionjuju-gui package

Merge lp://staging/~frankban/charms/trusty/juju-gui/new-test-and-server-deps into lp://staging/~juju-gui/charms/trusty/juju-gui/trunk

Commit message

Description of the change

Preview Diff

Subscribers

Juju Charms Collection
juju-gui package