Merge lp:~frankban/juju-gui/bug-1117554-gui-tests-in-browser into lp:juju-gui/experimental

[Francesco Banconi]

Added GUI unit tests in the browser test suite.

Juju GUI unit tests are now included in the CI process.
Also added a test exercising the environment connection.

Updated the charm deployment process: "juju set" is no
longer used, and the charm configuration is now provided
by a temporary configuration file.

To run browser tests:

sudo apt-get install python-shelltoolbox python-selenium python-yaml juju
bin/test-charm lp:~frankban/juju-gui/bug-1117554-gui-tests-in-browser

https://codereview.appspot.com/7308068/

[Francesco Banconi (frankban)]

Reviewers: mp+147356_code.launchpad.net,

Message:
Please take a look.

Description:
Added GUI unit tests in the browser test suite.

Juju GUI unit tests are now included in the CI process.
Also added a test exercising the environment connection.

Updated the charm deployment process: "juju set" is no
longer used, and the charm configuration is now provided
by a temporary configuration file.

To run browser tests:

sudo apt-get install python-shelltoolbox python-selenium python-yaml
juju
bin/test-charm lp:~frankban/juju-gui/bug-1117554-gui-tests-in-browser

https://code.launchpad.net/~frankban/juju-gui/bug-1117554-gui-tests-in-browser/+merge/147356

(do not edit description out of merge proposal)

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

Affected files:
   A [revision details]
   M bin/test-charm
   M docs/browser-testing.rst
   M lib/deploy_charm_for_testing.py
   M test/browser.py
   M test/index.html
   M test/test_charm_running.py
   M test/test_deploy_charm_for_testing.py

[Gary Poster (gary)]

Land as is. Really nice. Worked great for me. Thank you.

Gary

https://codereview.appspot.com/7308068/

[Nicola Larosa (teknico)]

Land as is.

Nice changes. Having difficulties running tests, for some reason. I'll
keep trying.

https://codereview.appspot.com/7308068/diff/1/test/test_charm_running.py
File test/test_charm_running.py (right):

https://codereview.appspot.com/7308068/diff/1/test/test_charm_running.py#newcode41
test/test_charm_running.py:41: msg = '{} failure(s) running {}
tests.'.format(failures, total)
Nice, you can omit the field numbers in 2.7, I didn't know.

https://codereview.appspot.com/7308068/

[Francesco Banconi (frankban)]

*** Submitted:

Added GUI unit tests in the browser test suite.

Juju GUI unit tests are now included in the CI process.
Also added a test exercising the environment connection.

Updated the charm deployment process: "juju set" is no
longer used, and the charm configuration is now provided
by a temporary configuration file.

To run browser tests:

sudo apt-get install python-shelltoolbox python-selenium python-yaml
juju
bin/test-charm lp:~frankban/juju-gui/bug-1117554-gui-tests-in-browser

R=gary.poster, teknico
CC=
https://codereview.appspot.com/7308068

https://codereview.appspot.com/7308068/

[Francesco Banconi (frankban)]

Thanks for the reviews Gary and Nicola.

https://codereview.appspot.com/7308068/

=== modified file 'bin/test-charm'
--- bin/test-charm	2013-02-06 02:22:34 +0000
+++ bin/test-charm	2013-02-08 12:00:36 +0000
@@ -1,12 +1,12 @@
 #!/bin/sh
 # This script expects that the system has python-shelltoolbox, python-
-# selenium, and juju installed:
-# sudo apt-get install python-shelltoolbox python-selenium juju
+# selenium, python-yaml, and juju installed:
+# sudo apt-get install python-shelltoolbox python-selenium python-yaml juju
 # Deploy the charm, using a branch if given.
 python lib/deploy_charm_for_testing.py "$@"
 # Figure out the URL of the APP.
-export APP_URL=http://`juju status -e juju-gui-testing | grep public-address: | cut -d: -f2 | cut -c2-`
+export APP_URL=https://`juju status -e juju-gui-testing | grep public-address: | cut -d: -f2 | cut -c2-`
 # Run the browser tests against the app.
-python test/test_charm_running.py
+python test/test_charm_running.py -v
 # Destroy the environment, releasing the resources.
 yes | juju destroy-environment -e juju-gui-testing

=== modified file 'docs/browser-testing.rst'
--- docs/browser-testing.rst	2013-02-06 02:22:34 +0000
+++ docs/browser-testing.rst	2013-02-08 12:00:36 +0000
@@ -23,7 +23,7 @@
 
 The test script has a few system dependencies::
 
-    sudo apt-get install python-shelltoolbox python-selenium juju
+    sudo apt-get install python-shelltoolbox python-selenium python-yaml juju
 
 It does not require that the Makefile be run.
 

=== modified file 'lib/deploy_charm_for_testing.py'
--- lib/deploy_charm_for_testing.py	2013-02-06 12:43:42 +0000
+++ lib/deploy_charm_for_testing.py	2013-02-08 12:00:36 +0000
@@ -4,6 +4,8 @@
 import shelltoolbox
 import sys
 import time
+import tempfile
+import yaml
 
 
 juju_command = shelltoolbox.command('juju')
@@ -34,6 +36,21 @@
     return unit['agent-state']
 
 
+def make_config_file(options):
+    """Create a Juju GUI charm config file. Return the config file object.
+
+    This function can also be used as a context manager.
+    """
+    config = {'juju-gui': options}
+    config_file = tempfile.NamedTemporaryFile()
+    config_file.write(yaml.dump(config))
+    config_file.flush()
+    # The NamedTemporaryFile instance is returned instead of just the name
+    # because we want to take advantage of garbage collection-triggered
+    # deletion of the temp file when it goes out of scope in the caller.
+    return config_file
+
+
 def wait_for_service(get_state=get_state, sleep=time.sleep):
     """Wait for the service to start or for it to enter an error state."""
     while True:
@@ -45,17 +62,20 @@
         sleep(10)
 
 
-def main(argv, print=print, juju=juju, wait_for_service=wait_for_service):
+def main(argv, print=print, juju=juju, wait_for_service=wait_for_service,
+         make_config_file=make_config_file):
     """Deploy the Juju GUI service and wait for it to become available."""
     branch = get_branch_url(argv)
     print('Bootstrapping...')
     juju('bootstrap --environment juju-gui-testing')
     print('Deploying service...')
-    juju('deploy juju-gui --environment juju-gui-testing')
+    options = {'serve-tests': True, 'staging': True}
     if branch is not None:
         print('Setting branch for charm to deploy...')
-        juju('set juju-gui juju-gui-source={} --environment juju-gui-testing'
-            .format(branch))
+        options['juju-gui-source'] = branch
+    with make_config_file(options) as config_file:
+        juju('deploy --environment juju-gui-testing --config {} '
+             'cs:~juju-gui/precise/juju-gui'.format(config_file.name))
     print('Waiting for service to start...')
     wait_for_service()
     print('Exposing the service...')

=== modified file 'test/browser.py'
--- test/browser.py	2013-02-06 21:05:31 +0000
+++ test/browser.py	2013-02-08 12:00:36 +0000
@@ -8,7 +8,9 @@
 import os
 import selenium
 import selenium.webdriver
+from selenium.webdriver.support import ui
 import unittest
+import urlparse
 
 
 ie = dict(selenium.webdriver.DesiredCapabilities.INTERNETEXPLORER)
@@ -58,7 +60,7 @@
 
     @classmethod
     def setUpClass(cls):
-        global driver # We only want one because they are expensive to set up.
+        global driver  # We only want one because they are expensive to set up.
         if driver is None:
             # We sometimes run the tests under different browsers, if none is
             # specified, use Chrome.
@@ -76,6 +78,7 @@
             atexit.register(driver.quit)
 
     def setUp(self):
+        self.app_url = os.environ['APP_URL']
         self.driver = driver
 
     def run(self, result=None):
@@ -85,3 +88,18 @@
     def tearDown(self):
         successful = self.last_result.wasSuccessful()
         set_test_result(driver.session_id, successful)
+
+    def load(self, path='/'):
+        """Load a page using the current Selenium driver."""
+        url = urlparse.urljoin(self.app_url, path)
+        self.driver.get(url)
+
+    def wait_for(self, condition, error=None, timeout=10):
+        """Wait for condition to be True.
+
+        The argument condition is a callable accepting a driver object.
+        Fail printing the provided error if timeout is exceeded.
+        Otherwise, return the value returned by the condition call.
+        """
+        wait = ui.WebDriverWait(self.driver, timeout)
+        return wait.until(condition, error)

=== modified file 'test/index.html'
--- test/index.html	2013-01-24 17:43:57 +0000
+++ test/index.html	2013-02-08 12:00:36 +0000
@@ -68,8 +68,11 @@
   YUI().use(['node', 'event'], function(Y) {
       // Run the tests.
       if (window.mochaPhantomJS) {
-          mochaPhantomJS.run(); }
-      else { mocha.run(); }
+          mochaPhantomJS.run();
+      } else {
+          // The global variable testRunner is required by browser tests.
+          testRunner = mocha.run();
+      }
   });
   </script>
 

=== modified file 'test/test_charm_running.py'
--- test/test_charm_running.py	2013-02-01 21:02:30 +0000
+++ test/test_charm_running.py	2013-02-08 12:00:36 +0000
@@ -1,23 +1,46 @@
 import browser
-import os
 import unittest
 
 
 class TestBasics(browser.TestCase):
 
-    def setUp(self):
-        super(TestBasics, self).setUp()
-        self.app_url = os.environ['APP_URL']
-
     def test_title(self):
-        self.driver.get(self.app_url)
+        self.load()
         self.assertTrue('Juju Admin' in self.driver.title)
 
     def test_environment_name(self):
-        self.driver.get(self.app_url)
+        self.load()
         body = self.driver.find_element_by_xpath('//body')
         self.assertTrue('Environment on ' in body.text)
 
+    def test_environment_connection(self):
+        # The GUI connects to the API backend.
+        self.load()
+
+        def connected(driver):
+            return driver.execute_script('return app.env.get("connected");')
+        self.wait_for(connected, 'Environment not connected.')
+
+    def test_gui_unit_tests(self):
+        # Ensure Juju GUI unit tests pass.
+        self.load('/test/')
+        script = """
+            var stats = testRunner.stats;
+            return [testRunner.total, stats.tests, stats.failures];
+        """
+
+        def tests_completed(driver):
+            total, done, failures = driver.execute_script(script)
+            # Return when tests completed or a failure occurred.
+            if (done == total) or failures:
+                return total, failures
+
+        total, failures = self.wait_for(
+            tests_completed, 'Unable to complete test run.', timeout=30)
+        if failures:
+            msg = '{} failure(s) running {} tests.'.format(failures, total)
+            self.fail(msg)
+
 
 if __name__ == '__main__':
     unittest.main()

=== modified file 'test/test_deploy_charm_for_testing.py'
--- test/test_deploy_charm_for_testing.py	2013-02-06 12:43:42 +0000
+++ test/test_deploy_charm_for_testing.py	2013-02-08 12:00:36 +0000
@@ -87,6 +87,19 @@
         self.assertEqual(get_state(get_status), 'started')
 
 
+class MakeConfigFile(object):
+    """Simulate creating a charm configuration file."""
+
+    name = 'my-config-file.yaml'
+    options = None
+    # This class is used as a context manager.
+    __enter__ = lambda self: self
+    __exit__ = noop
+
+    def __init__(self, options):
+        self.__class__.options = options
+
+
 class TestScript(unittest.TestCase):
     """The main() function is the entry point when run as a script."""
 
@@ -101,16 +114,27 @@
              'Waiting for service to start...',
              'Exposing the service...'])
 
+    def test_config_file(self):
+        # A charm config file is correctly created.
+        juju_commands = []
+
+        def juju(s):
+            juju_commands.append(s)
+        main(argv=[], print=noop, juju=juju, wait_for_service=noop,
+             make_config_file=MakeConfigFile)
+        options = MakeConfigFile.options
+        deploy_command = juju_commands[1]
+        self.assertIn('--config my-config-file.yaml', deploy_command)
+        self.assertDictEqual({'serve-tests': True, 'staging': True}, options)
+
     def test_providing_a_branch(self):
         # If the user provides a branch name on the command line, it will be
         # passed to the charm.
         printed = []
-        juju_commands = []
 
-        def juju(s):
-            juju_commands.append(s)
-        main(argv=['', 'lp:foo'], print=printed.append, juju=juju,
-            wait_for_service=noop)
+        main(argv=['', 'lp:foo'], print=printed.append, juju=noop,
+             wait_for_service=noop, make_config_file=MakeConfigFile)
+        options = MakeConfigFile.options
         self.assertSequenceEqual(
             printed,
             ['Bootstrapping...',
@@ -118,10 +142,8 @@
              'Setting branch for charm to deploy...',
              'Waiting for service to start...',
              'Exposing the service...'])
-        self.assertIn(
-            ('set juju-gui juju-gui-source=lp:foo --environment '
-             'juju-gui-testing'),
-            juju_commands)
+        self.assertIn('juju-gui-source', options)
+        self.assertEqual('lp:foo', options['juju-gui-source'])
 
 
 if __name__ == '__main__':