Merge into trunk : ftest-fixes : Code : juju-quickstart

Status:	Merged
Merged at revision:	118
Proposed branch:	lp://qastaging/~frankban/juju-quickstart/ftest-fixes
Merge into:	lp://qastaging/juju-quickstart
Diff against target:	116 lines (+46/-11) 2 files modified HACKING.rst (+2/-1) quickstart/tests/functional/test_functional.py (+44/-10)
To merge this branch:	bzr merge lp://qastaging/~frankban/juju-quickstart/ftest-fixes
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
Juju GUI Hackers		2015-02-05	Pending
Review via email: mp+248755@code.qastaging.launchpad.net

Description of the change

Make ftests fail if Juju env is not ready.

The functional tests are no longer silently skipped
if the default Juju environment cannot be found.

Also fail if the environment is running, as this
would cause it to be destroyed at the end of the
suite.

QA:
- bootstrap your default environment;
- run `make ftest`/`make fcheck` and ensure
  the functional tests fail
  (environment already bootstrapped);
- destroy your environment;
- run `make fcheck` again and ensure all the
  suites complete successfully.

https://codereview.appspot.com/195690043/

Revision history for this message

Francesco Banconi (frankban) wrote on 2015-02-05:

#

Download full text (5.7 KiB)

Reviewers: mp+248755_code.launchpad.net,

Message:
Please take a look.

Description:
Make ftests fail if Juju env is not ready.

The functional tests are no longer silently skipped
if the default Juju environment cannot be found.

Also fail if the environment is running, as this
would cause it to be destroyed at the end of the
suite.

QA:
- bootstrap your default environment;
- run `make ftest`/`make fcheck` and ensure
   the functional tests fail
   (environment already bootstrapped);
- destroy your environment;
- run `make fcheck` again and ensure all the
   suites complete successfully.

https://code.launchpad.net/~frankban/juju-quickstart/ftest-fixes/+merge/248755

(do not edit description out of merge proposal)

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

Affected files (+48, -11 lines):
   M HACKING.rst
   A [revision details]
   M quickstart/tests/functional/test_functional.py

Index: HACKING.rst
=== modified file 'HACKING.rst'
--- HACKING.rst 2015-01-30 18:06:46 +0000
+++ HACKING.rst 2015-02-05 13:35:53 +0000
@@ -85,6 +85,7 @@
    bootstrapped in the process;
  - a Juju home correctly set up with at least one environment defined in the
    ``environments.yaml`` file;
+- the selected Juju environment not to be in use;
  - SSH keys already generated for the user running the tests;
  - a working Internet connection.

@@ -94,7 +95,7 @@
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**.
+complete. The tests fail the selected environment is already in use.

To run the test using a customized build of Juju, pass the ``JUJU``
environment
variable to ``make``, e.g.::

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: quickstart/tests/functional/test_functional.py
=== modified file 'quickstart/tests/functional/test_functional.py'
--- quickstart/tests/functional/test_functional.py 2015-02-03 14:13:09 +0000
+++ quickstart/tests/functional/test_functional.py 2015-02-05 12:19:48 +0000
@@ -36,27 +36,55 @@
FTEST_ENV_VAR = 'JUJU_QUICKSTART_FTESTS'

-def skip_if_disbled(func):
- """Decorate a test method so that it is only run when
required/possible."""
+def skip_or_fail_if_environment_not_ready(func):
+ """Decorate a test method so that it is only run when asked.
+
+ Also fail if the Juju environment is already running, or if the default
+ environment cannot be found.
+ """
      # 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')

@fun...

Reviewers: mp+248755_code.launchpad.net,

Message:
Please take a look.

Description:
Make ftests fail if Juju env is not ready.

The functional tests are no longer silently skipped
if the default Juju environment cannot be found.

Also fail if the environment is running, as this
would cause it to be destroyed at the end of the
suite.

QA:
- bootstrap your default environment;
- run `make ftest`/`make fcheck` and ensure
   the functional tests fail
   (environment already bootstrapped);
- destroy your environment;
- run `make fcheck` again and ensure all the
   suites complete successfully.

https://code.launchpad.net/~frankban/juju-quickstart/ftest-fixes/+merge/248755

(do not edit description out of merge proposal)

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

Affected files (+48, -11 lines):
   M HACKING.rst
   A [revision details]
   M quickstart/tests/functional/test_functional.py

Index: HACKING.rst
=== modified file 'HACKING.rst'
--- HACKING.rst	2015-01-30 18:06:46 +0000
+++ HACKING.rst	2015-02-05 13:35:53 +0000
@@ -85,6 +85,7 @@
    bootstrapped in the process;
  - a Juju home correctly set up with at least one environment defined in the
    ``environments.yaml`` file;
+- the selected Juju environment not to be in use;
  - SSH keys already generated for the user running the tests;
  - a working Internet connection.

@@ -94,7 +95,7 @@
      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**.
+complete. The tests fail the selected environment is already in use.

To run the test using a customized build of Juju, pass the ``JUJU``  
environment
  variable to ``make``, e.g.::

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-20150203163949-653xoiks4ju949cr
+New revision:  
francesco.banconi@canonical.com-20150205133553-sq2gkmm0qol673n2

Index: quickstart/tests/functional/test_functional.py
=== modified file 'quickstart/tests/functional/test_functional.py'
--- quickstart/tests/functional/test_functional.py	2015-02-03 14:13:09 +0000
+++ quickstart/tests/functional/test_functional.py	2015-02-05 12:19:48 +0000
@@ -36,27 +36,55 @@
  FTEST_ENV_VAR = 'JUJU_QUICKSTART_FTESTS'

-def skip_if_disbled(func):
-    """Decorate a test method so that it is only run when  
required/possible."""
+def skip_or_fail_if_environment_not_ready(func):
+    """Decorate a test method so that it is only run when asked.
+
+    Also fail if the Juju environment is already running, or if the default
+    environment cannot be found.
+    """
      # 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.
+        # If the environment cannot be found, or it is already running,
+        # do not proceed with setting up the test case class, and make the
+        # tests fail by setting env_error on the class or instance.
+        cls_or_self.env_error = None
+        # Check that a Juju environment can be found.
+        env_name = envs.get_default_env_name()
+        if env_name is None:
+            cls_or_self.env_error = 'cannot find a configured Juju  
environment'
+            return
+        # Check that the Juju environment is not running.
+        retcode, _, _ = run_juju('status', '-e', env_name)
+        if not retcode:
+            cls_or_self.env_error = (
+                'the {} Juju environment is already  
running'.format(env_name))
+            return
+        # Update the received class or instance setting the env_name  
attribute.
          cls_or_self.env_name = env_name
          return func(cls_or_self)

return decorated

+def skip_if_environment_not_set(func):
+    """Decorate a test method so that it is not run without an environment.
+
+    The presence of the "env_name" attribute in the TestCase indicates  
whether
+    or not the tests can be run.
+    """
+    @functools.wraps(func)
+    def decorated(cls_or_self):
+        if hasattr(cls_or_self, 'env_name'):
+            return func(cls_or_self)
+
+    return decorated
+
+
  def run_juju(*args):
      """Run the juju command with the given args.

@@ -81,7 +109,7 @@
  class TestFunctional(unittest.TestCase):

@classmethod
-    @skip_if_disbled
+    @skip_or_fail_if_environment_not_ready
      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
@@ -89,11 +117,17 @@
          cls.retcode, cls.output, cls.error = run_quickstart(cls.env_name)

@classmethod
-    @skip_if_disbled
+    @skip_if_environment_not_set
      def tearDownClass(cls):
          # Destroy the environment.
          run_juju('destroy-environment', cls.env_name, '-y', '--force')

+    def setUp(self):
+        # Let all the tests fail if an environment error has been detected
+        # in setUpClass.
+        if self.env_error is not None:
+            self.fail(self.env_error)
+
      def test_executed(self):
          # The application successfully completed its execution.
          self.assertEqual(0, self.retcode)

Revision history for this message

Jeff Pihach (hatch) wrote on 2015-02-05:

#

LGTM Thanks for this follow-up!

https://codereview.appspot.com/195690043/

Revision history for this message

Kyle Adams (kadams54) wrote on 2015-02-05:

#

LGTM as well.

https://codereview.appspot.com/195690043/

Revision history for this message

Francesco Banconi (frankban) wrote on 2015-02-05:

#

*** Submitted:

Make ftests fail if Juju env is not ready.

The functional tests are no longer silently skipped
if the default Juju environment cannot be found.

Also fail if the environment is running, as this
would cause it to be destroyed at the end of the
suite.

QA:
- bootstrap your default environment;
- run `make ftest`/`make fcheck` and ensure
   the functional tests fail
   (environment already bootstrapped);
- destroy your environment;
- run `make fcheck` again and ensure all the
   suites complete successfully.

R=jeff.pihach, kadams54
CC=
https://codereview.appspot.com/195690043

https://codereview.appspot.com/195690043/

Revision history for this message

Francesco Banconi (frankban) wrote on 2015-02-05:

#

Thanks for the reviews!

https://codereview.appspot.com/195690043/

juju-quickstart

Merge lp://qastaging/~frankban/juju-quickstart/ftest-fixes into lp://qastaging/juju-quickstart

Commit message

Description of the change

Preview Diff

Subscribers