Code review comment for lp:~frankban/juju-quickstart/ftest-fixes

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

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