Merge lp:~frankban/juju-quickstart/views-params into lp:juju-quickstart

[Francesco Banconi]

Refactor view parameters.

Implement a Params object used to store
common view parameters. This way it will
be easier to add parameters in the future
(e.g. one callable to remove stale jenv files).

New code include the params module and a fix
to the code handling the listing of jenv files
in the index view: now the header message is
only displayed if jenv files actually exist.

The rest of the diff is mechanical: i.e. replacing
the single view arguments with the params named
tuple.

Tests: `make check`.

QA:
Use the quickstart interactive session and
check everything works ok.

https://codereview.appspot.com/194030043/

[Francesco Banconi (frankban)]

Reviewers: mp+245753_code.launchpad.net,

Message:
Please take a look.

Description:
Refactor view parameters.

Implement a Params object used to store
common view parameters. This way it will
be easier to add parameters in the future
(e.g. one callable to remove stale jenv files).

New code include the params module and a fix
to the code handling the listing of jenv files
in the index view: now the header message is
only displayed if jenv files actually exist.

The rest of the diff is mechanical: i.e. replacing
the single view arguments with the params named
tuple.

Tests: `make check`.

QA:
Use the quickstart interactive session and
check everything works ok.

https://code.launchpad.net/~frankban/juju-quickstart/views-params/+merge/245753

(do not edit description out of merge proposal)

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

Affected files (+303, -172 lines):
   A [revision details]
   A quickstart/cli/params.py
   M quickstart/cli/views.py
   M quickstart/manage.py
   A quickstart/tests/cli/test_params.py
   M quickstart/tests/cli/test_views.py
   M quickstart/tests/test_manage.py

[Brad Crittenden (bac)]

LGTM. No QA yet.

https://codereview.appspot.com/194030043/diff/1/quickstart/cli/views.py
File quickstart/cli/views.py (right):

https://codereview.appspot.com/194030043/diff/1/quickstart/cli/views.py#newcode314
quickstart/cli/views.py:314: # and supposed to be working/active. The
user has the ability to select
Maybe 'assumed' would be clearer than 'supposed'?

https://codereview.appspot.com/194030043/diff/1/quickstart/tests/cli/test_params.py
File quickstart/tests/cli/test_params.py (right):

https://codereview.appspot.com/194030043/diff/1/quickstart/tests/cli/test_params.py#newcode87
quickstart/tests/cli/test_params.py:87: self.assertNotEqual(self.params,
params)
Such a nice set of tests! Thanks for the clarity.

https://codereview.appspot.com/194030043/

[Martin Hilton (martin-hilton)]

LGTM

https://codereview.appspot.com/194030043/

[Brad Crittenden (bac)]

QA-OK. I simply ran quickstart and launched a local provider.
Everything came up as expected.

https://codereview.appspot.com/194030043/

[lp:~frankban/juju-quickstart/views-params]

114. By Francesco Banconi on 2015-01-07

Improve view comment.

[Francesco Banconi (frankban)]

*** Submitted:

Refactor view parameters.

Implement a Params object used to store
common view parameters. This way it will
be easier to add parameters in the future
(e.g. one callable to remove stale jenv files).

New code include the params module and a fix
to the code handling the listing of jenv files
in the index view: now the header message is
only displayed if jenv files actually exist.

The rest of the diff is mechanical: i.e. replacing
the single view arguments with the params named
tuple.

Tests: `make check`.

QA:
Use the quickstart interactive session and
check everything works ok.

R=bac, martin.hilton
CC=
https://codereview.appspot.com/194030043

https://codereview.appspot.com/194030043/diff/1/quickstart/cli/views.py
File quickstart/cli/views.py (right):

https://codereview.appspot.com/194030043/diff/1/quickstart/cli/views.py#newcode314
quickstart/cli/views.py:314: # and supposed to be working/active. The
user has the ability to select
On 2015/01/07 15:58:26, bac wrote:
> Maybe 'assumed' would be clearer than 'supposed'?

Done.

https://codereview.appspot.com/194030043/

[Francesco Banconi (frankban)]

Thanks for the reviews!

https://codereview.appspot.com/194030043/

=== added file 'quickstart/cli/params.py'
--- quickstart/cli/params.py	1970-01-01 00:00:00 +0000
+++ quickstart/cli/params.py	2015-01-07 16:46:42 +0000
@@ -0,0 +1,51 @@
+# 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 view parameters implementation.
+
+This module contains an implementation of a composite parameter object to be
+used in views.
+
+The Params named tuple is intended to both help grouping view parameters
+together and avoid undesired mutations, by allowing explicit copies of the
+whole parameters object (see the Params.copy instance method).
+"""
+
+from __future__ import unicode_literals
+
+from collections import namedtuple
+import copy
+
+
+_PARAMS = ('env_type_db', 'env_db', 'jenv_db', 'save_callable')
+
+
+class Params(namedtuple('Params', _PARAMS)):
+    """View parameters as a named tuple."""
+
+    # Define empty slots to keep memory requirements low by preventing the
+    # creation of instance dictionaries.
+    # See https://docs.python.org/2/library/collections.html
+    __slots__ = ()
+
+    def copy(self):
+        """Return a deep copy of the stored parameters."""
+        return self.__class__(
+            env_type_db=self.env_type_db,
+            env_db=copy.deepcopy(self.env_db),
+            jenv_db=copy.deepcopy(self.jenv_db),
+            save_callable=self.save_callable,
+        )

=== modified file 'quickstart/cli/views.py'
--- quickstart/cli/views.py	2014-12-17 11:34:06 +0000
+++ quickstart/cli/views.py	2015-01-07 16:46:42 +0000
@@ -155,36 +155,33 @@
     raise ui.AppExit((env_db, env_data))
 
 
-def env_index(app, env_type_db, env_db, jenv_db, save_callable):
+def env_index(app, params):
     """Show the Juju environments list.
 
     The env_detail view is displayed when the user clicks on an environment.
     From here it is also possible to switch to the edit view in order to create
     a new environment.
 
-    Receives:
+    Receives a params namedtuple-like object including the following fields:
         - env_type_db: the environments meta information;
         - env_db: the environments database;
         - jenv_db: the jenv files database;
         - save_callable: a function called to save a new environment database.
+    See quickstart.cli.params.Params.
     """
     # XXX frankban 16/12/2014: this function is too long, subdivide it.
-    env_db = copy.deepcopy(env_db)
-    jenv_db = copy.deepcopy(jenv_db)
+    params = params.copy()
     # All the environment views return a tuple (new_env_db, env_data).
     # Set the env_data to None in the case the user quits the application
     # without selecting an environment to use.
-    app.set_return_value_on_exit((env_db, None))
-    detail_view = functools.partial(
-        env_detail, app, env_type_db, env_db, jenv_db, save_callable)
-    jenv_view = functools.partial(
-        jenv_detail, app, env_type_db, env_db, jenv_db, save_callable)
-    edit_view = functools.partial(
-        env_edit, app, env_type_db, env_db, jenv_db, save_callable)
+    app.set_return_value_on_exit((params.env_db, None))
+    detail_view = functools.partial(env_detail, app, params)
+    jenv_view = functools.partial(jenv_detail, app, params)
+    edit_view = functools.partial(env_edit, app, params)
     # Alphabetically sort the existing environments.
     environments = sorted([
-        envs.get_env_data(env_db, env_name)
-        for env_name in env_db['environments']
+        envs.get_env_data(params.env_db, env_name)
+        for env_name in params.env_db['environments']
     ], key=operator.itemgetter('name'))
 
     def create_and_start_local_env():
@@ -193,8 +190,8 @@
         # database. For this reason, the new environment is set as default.
         # Exit the interactive session selecting the newly created environment.
         env_data = envs.create_local_env_data(
-            env_type_db, 'local', is_default=True)
-        _save_and_exit(env_db, env_data, save_callable)
+            params.env_type_db, 'local', is_default=True)
+        _save_and_exit(params.env_db, env_data, params.save_callable)
 
     def create_and_start_maas_env(name, server, api_key):
         # Automatically create and use a MAAS environment.
@@ -202,8 +199,8 @@
         # database. For this reason, the new environment is set as default.
         # Exit the interactive session selecting the newly created environment.
         env_data = envs.create_maas_env_data(
-            env_type_db, name, server, api_key, is_default=True)
-        _save_and_exit(env_db, env_data, save_callable)
+            params.env_type_db, name, server, api_key, is_default=True)
+        _save_and_exit(params.env_db, env_data, params.save_callable)
 
     platform = platform_support.get_platform()
     supports_local = platform_support.supports_local(platform)
@@ -279,7 +276,7 @@
     focus_position = None
     active_found = default_found = errors_found = False
     existing_widgets_num = len(widgets)
-    remaining_jenv_db = copy.deepcopy(jenv_db)
+    remaining_jenv_db = copy.deepcopy(params.jenv_db)
     for position, env_data in enumerate(environments):
         bullet = '\N{BULLET}'
         # Is this environment the default one?
@@ -294,7 +291,7 @@
             bullet = ('active', bullet)
         else:
             # Check if this environment is valid.
-            env_metadata = envs.get_env_metadata(env_type_db, env_data)
+            env_metadata = envs.get_env_metadata(params.env_type_db, env_data)
             errors = envs.validate(env_metadata, env_data)
             if errors:
                 errors_found = True
@@ -306,25 +303,28 @@
 
     # Alphabetically sort the remaining environments not included in the
     # environments.yaml file.
-    environments = sorted([
+    environments = list(sorted([
         envs.get_env_data(remaining_jenv_db, env_name)
         for env_name in remaining_jenv_db['environments']
-    ], key=operator.itemgetter('name'))
+    ], key=operator.itemgetter('name')))
 
-    # List the remaining active environments. Those environments are not
-    # included in the environments.yaml file: they are probably imported and
-    # supposed to be working/active. The user has the ability to select them.
-    widgets.extend([
-        urwid.Divider(),
-        urwid.Text(('highlight', 'Other active environments')),
-        urwid.Text('(imported/not included in your environments.yaml file):'),
-        urwid.Divider(),
-    ])
-    bullet = ('active', '\N{BULLET}')
-    for env_data in environments:
-        env_short_description = jenv.get_env_short_description(env_data)
-        text = [bullet, ' {}'.format(env_short_description)]
-        widgets.append(ui.MenuButton(text, ui.thunk(jenv_view, env_data)))
+    if environments:
+        # List the remaining active environments. Those environments are not
+        # included in the environments.yaml file: they are probably imported
+        # and assumed to be working/active. The user has the ability to select
+        # them.
+        widgets.extend([
+            urwid.Divider(),
+            urwid.Text(('highlight', 'Other active environments')),
+            urwid.Text(
+                '(imported/not included in your environments.yaml file):'),
+            urwid.Divider(),
+        ])
+        bullet = ('active', '\N{BULLET}')
+        for env_data in environments:
+            env_short_description = jenv.get_env_short_description(env_data)
+            text = [bullet, ' {}'.format(env_short_description)]
+            widgets.append(ui.MenuButton(text, ui.thunk(jenv_view, env_data)))
 
     # Set up the "create a new environment" section.
     widgets.extend([
@@ -342,7 +342,7 @@
     if not supports_local:
         filter_function = lambda env_type, _: env_type != 'local'
     supported_env_types = envs.get_supported_env_types(
-        env_type_db, filter_function=filter_function)
+        params.env_type_db, filter_function=filter_function)
     # Add the buttons used to create new environments.
     widgets.extend([
         ui.MenuButton(
@@ -369,42 +369,40 @@
     app.set_contents(contents)
 
 
-def env_detail(app, env_type_db, env_db, jenv_db, save_callable, env_data):
+def env_detail(app, params, env_data):
     """Show details on a Juju environment.
 
     From this view it is possible to start the environment, set it as default,
     edit/remove the environment.
 
-    Receives:
+    Receives a params namedtuple-like object including the following fields:
         - env_type_db: the environments meta information;
         - env_db: the environments database;
         - jenv_db: the jenv files database;
-        - save_callable: a function called to save a new environment database;
-        - env_data: the environment data.
+        - save_callable: a function called to save a new environment database.
+    See quickstart.cli.params.Params.
+    Also receives the current environment data env_data.
     """
-    env_db = copy.deepcopy(env_db)
-    jenv_db = copy.deepcopy(jenv_db)
+    params = params.copy()
     # All the environment views return a tuple (new_env_db, env_data).
     # Set the env_data to None in the case the user quits the application
     # without selecting an environment to use.
-    app.set_return_value_on_exit((env_db, None))
-    index_view = functools.partial(
-        env_index, app, env_type_db, env_db, jenv_db, save_callable)
-    edit_view = functools.partial(
-        env_edit, app, env_type_db, env_db, jenv_db, save_callable, env_data)
+    app.set_return_value_on_exit((params.env_db, None))
+    index_view = functools.partial(env_index, app, params)
+    edit_view = functools.partial(env_edit, app, params, env_data)
 
     def use(env_data):
         # Quit the interactive session returning the (possibly modified)
         # environment database and the environment data corresponding to the
         # selected environment.
-        raise ui.AppExit((env_db, env_data))
+        raise ui.AppExit((params.env_db, env_data))
 
     def set_default(env_data):
         # Set this environment as the default one, save the env_db and return
         # to the index view.
         env_name = env_data['name']
-        env_db['default'] = env_name
-        save_callable(env_db)
+        params.env_db['default'] = env_name
+        params.save_callable(params.env_db)
         app.set_message('{} successfully set as default'.format(env_name))
         index_view()
 
@@ -412,8 +410,8 @@
         # The environment deletion is confirmed: remove the environment from
         # the database, save the new env_db and return to the index view.
         env_name = env_data['name']
-        envs.remove_env(env_db, env_name)
-        save_callable(env_db)
+        envs.remove_env(params.env_db, env_name)
+        params.save_callable(params.env_db)
         app.set_message('{} successfully removed'.format(env_name))
         index_view()
 
@@ -427,7 +425,7 @@
             ],
         )
 
-    env_metadata = envs.get_env_metadata(env_type_db, env_data)
+    env_metadata = envs.get_env_metadata(params.env_type_db, env_data)
     app.set_title(envs.get_env_short_description(env_data))
     # Validate the environment.
     errors = envs.validate(env_metadata, env_data)
@@ -464,34 +462,33 @@
     app.set_contents(listbox)
 
 
-def jenv_detail(app, env_type_db, env_db, jenv_db, save_callable, env_data):
+def jenv_detail(app, params, env_data):
     """Show details on a Juju imported environment.
 
     The environment is not included in the environments.yaml file, but just
     found in the jenv database.
     From this view it is possible to start the environment.
 
-    Receives:
+    Receives a params namedtuple-like object including the following fields:
         - env_type_db: the environments meta information;
         - env_db: the environments database;
         - jenv_db: the jenv files database;
         - save_callable: a function called to save a new environment database;
-        - env_data: the environment data.
+    See quickstart.cli.params.Params.
+    Also receives the current environment data env_data.
     """
-    env_db = copy.deepcopy(env_db)
-    jenv_db = copy.deepcopy(jenv_db)
+    params = params.copy()
     # All the environment views return a tuple (new_env_db, env_data).
     # Set the env_data to None in the case the user quits the application
     # without selecting an environment to use.
-    app.set_return_value_on_exit((env_db, None))
-    index_view = functools.partial(
-        env_index, app, env_type_db, env_db, jenv_db, save_callable)
+    app.set_return_value_on_exit((params.env_db, None))
+    index_view = functools.partial(env_index, app, params)
 
     def use(env_data):
         # Quit the interactive session returning the (possibly modified)
         # environment database and the environment data corresponding to the
         # selected environment.
-        raise ui.AppExit((env_db, env_data))
+        raise ui.AppExit((params.env_db, env_data))
 
     app.set_title(jenv.get_env_short_description(env_data))
     widgets = []
@@ -519,36 +516,34 @@
     app.set_status([' \N{RIGHTWARDS ARROW OVER LEFTWARDS ARROW} navigate '])
 
 
-def env_edit(app, env_type_db, env_db, jenv_db, save_callable, env_data):
+def env_edit(app, params, env_data):
     """Create or modify a Juju environment.
 
     This view displays an edit form allowing for environment
     creation/modification. Saving the form redirects to the environment detail
     view if the values are valid.
 
-    Receives:
+    Receives a params namedtuple-like object including the following fields:
         - env_type_db: the environments meta information;
         - env_db: the environments database;
         - jenv_db: the jenv files database;
         - save_callable: a function called to save a new environment database;
-        - env_data: the environment data.
+    See quickstart.cli.params.Params.
+    Also receives the current environment data env_data.
 
     The last value (env_data) indicates whether this view is used to create a
     new environment or to change an existing one. In the former case, env_data
     does not include the "name" key. If instead the environment already exists,
     env_data includes the "name" key and all the other environment info.
     """
-    env_db = copy.deepcopy(env_db)
-    jenv_db = copy.deepcopy(jenv_db)
+    params = params.copy()
     # All the environment views return a tuple (new_env_db, env_data).
     # Set the env_data to None in the case the user quits the application
     # without selecting an environment to use.
-    app.set_return_value_on_exit((env_db, None))
-    env_metadata = envs.get_env_metadata(env_type_db, env_data)
-    index_view = functools.partial(
-        env_index, app, env_type_db, env_db, jenv_db, save_callable)
-    detail_view = functools.partial(
-        env_detail, app, env_type_db, env_db, jenv_db, save_callable)
+    app.set_return_value_on_exit((params.env_db, None))
+    env_metadata = envs.get_env_metadata(params.env_type_db, env_data)
+    index_view = functools.partial(env_index, app, params)
+    detail_view = functools.partial(env_detail, app, params)
     if 'name' in env_data:
         exists = True
         title = 'Edit the {} environment'
@@ -580,6 +575,7 @@
         errors = envs.validate(env_metadata, new_env_data)
         new_name = new_env_data['name']
         initial_name = env_data.get('name')
+        env_db = params.env_db
         if (new_name != initial_name) and new_name in env_db['environments']:
             errors['name'] = 'an environment with this name already exists'
         # If errors are found, re-render the form passing the errors. This way
@@ -594,7 +590,7 @@
         if not env_db['environments']:
             env_data['is-default'] = True
         envs.set_env_data(env_db, initial_name, env_data)
-        save_callable(env_db)
+        params.save_callable(env_db)
         verb = 'modified' if exists else 'created'
         app.set_message('{} successfully {}'.format(new_name, verb))
         return detail_view(env_data)

=== modified file 'quickstart/manage.py'
--- quickstart/manage.py	2014-12-16 16:52:49 +0000
+++ quickstart/manage.py	2015-01-07 16:46:42 +0000
@@ -38,7 +38,10 @@
     settings,
     utils,
 )
-from quickstart.cli import views
+from quickstart.cli import (
+    params,
+    views,
+)
 from quickstart.models import (
     charms,
     envs,
@@ -210,9 +213,13 @@
     Exit the application if the user exits the interactive session without
     selecting an environment to start.
     """
-    save_callable = _create_save_callable(parser, env_file)
-    new_env_db, env_data = views.show(
-        views.env_index, env_type_db, env_db, jenv_db, save_callable)
+    parameters = params.Params(
+        env_type_db=env_type_db,
+        env_db=env_db,
+        jenv_db=jenv_db,
+        save_callable=_create_save_callable(parser, env_file),
+    )
+    new_env_db, env_data = views.show(views.env_index, parameters)
     if new_env_db != env_db:
         print('changes to the environments file have been saved')
     if env_data is None:

=== added file 'quickstart/tests/cli/test_params.py'
--- quickstart/tests/cli/test_params.py	1970-01-01 00:00:00 +0000
+++ quickstart/tests/cli/test_params.py	2015-01-07 16:46:42 +0000
@@ -0,0 +1,87 @@
+# 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/>.
+
+"""Tests for the Juju Quickstart CLI parameters implementation."""
+
+from __future__ import unicode_literals
+
+import unittest
+
+from quickstart.cli import params
+from quickstart.models import envs
+from quickstart.tests import helpers
+
+
+class TestParams(unittest.TestCase):
+
+    def setUp(self):
+        # Store parameters.
+        self.env_type_db = envs.get_env_type_db()
+        self.env_db = helpers.make_env_db()
+        self.jenv_db = helpers.make_jenv_db()
+        self.save_callable = lambda env_db: None
+        # Set up a params object used in tests.
+        self.params = params.Params(
+            env_type_db=self.env_type_db,
+            env_db=self.env_db,
+            jenv_db=self.jenv_db,
+            save_callable=self.save_callable,
+        )
+
+    def test_tuple(self):
+        # The params object can be used as a tuple.
+        env_type_db, env_db, jenv_db, save_callable = self.params
+        self.assertIs(self.env_type_db, env_type_db)
+        self.assertIs(self.env_db, env_db)
+        self.assertIs(self.jenv_db, jenv_db)
+        self.assertIs(self.save_callable, save_callable)
+
+    def test_attributes(self):
+        # Parameters can be accessed as attributes.
+        self.assertIs(self.env_type_db, self.params.env_type_db)
+        self.assertIs(self.env_db, self.params.env_db)
+        self.assertIs(self.jenv_db, self.params.jenv_db)
+        self.assertIs(self.save_callable, self.params.save_callable)
+
+    def test_immutable(self):
+        # It is not possible to replace a stored parameter.
+        with self.assertRaises(AttributeError):
+            self.params.env_db = {}
+
+    def test_copy(self):
+        # Params can be copied.
+        params = self.params.copy()
+        # The original object is not mutated by the copy.
+        self.assertIs(self.env_type_db, self.params.env_type_db)
+        self.assertIs(self.env_db, self.params.env_db)
+        self.assertIs(self.jenv_db, self.params.jenv_db)
+        self.assertIs(self.save_callable, self.params.save_callable)
+        # The new params object stores the same data.
+        self.assertEqual(self.params, params)
+        # But they do not refer to the same object.
+        self.assertIsNot(self.params, params)
+
+    def test_copy_mutations(self):
+        # Internal mutable objects can be still mutated.
+        params = self.params.copy()
+        # Mutate the copy.
+        params.env_db['environments']['lxc'] = {}
+        # The mutation took effect.
+        self.assertNotEqual(self.env_db, params.env_db)
+        # The original object is still preserved.
+        self.assertIs(self.env_db, self.params.env_db)
+        # The two now store different data.
+        self.assertNotEqual(self.params, params)

=== modified file 'quickstart/tests/cli/test_views.py'
--- quickstart/tests/cli/test_views.py	2014-12-17 11:47:43 +0000
+++ quickstart/tests/cli/test_views.py	2015-01-07 16:46:42 +0000
@@ -31,6 +31,7 @@
 from quickstart.cli import (
     base,
     forms,
+    params,
     ui,
     views,
 )
@@ -166,6 +167,15 @@
         """
         return lambda arg: isinstance(arg, cls)
 
+    def make_params(self, env_db, jenv_db):
+        """Create and return view parameters using the given env databases."""
+        return params.Params(
+            env_type_db=self.env_type_db,
+            env_db=env_db,
+            jenv_db=jenv_db,
+            save_callable=self.save_callable,
+        )
+
 
 class TestEnvIndex(EnvViewTestsMixin, unittest.TestCase):
 
@@ -175,6 +185,7 @@
     create_maas_caption = (
         '\N{BULLET} automatically create and bootstrap the {} MAAS '
         'environment'.format(MAAS_NAME))
+    active_environments_message = 'Other active environments'
 
     def test_view_default_return_value_on_exit(self):
         # The view configures the app so that the return value on user exit is
@@ -182,8 +193,7 @@
         # meaning no environment has been selected.
         env_db = helpers.make_env_db()
         jenv_db = helpers.make_jenv_db()
-        views.env_index(
-            self.app, self.env_type_db, env_db, jenv_db, self.save_callable)
+        views.env_index(self.app, self.make_params(env_db, jenv_db))
         new_env_db, env_data = self.get_on_exit_return_value(self.loop)
         self.assertEqual(env_db, new_env_db)
         self.assertIsNot(env_db, new_env_db)
@@ -193,8 +203,7 @@
         # The application title is correctly set up.
         env_db = helpers.make_env_db()
         jenv_db = helpers.make_jenv_db()
-        views.env_index(
-            self.app, self.env_type_db, env_db, jenv_db, self.save_callable)
+        views.env_index(self.app, self.make_params(env_db, jenv_db))
         self.assertEqual(
             'Select an existing Juju environment or create a new one',
             self.app.get_title())
@@ -203,8 +212,7 @@
         # The application title changes if the env_db has no environments.
         env_db = {'environments': {}}
         jenv_db = helpers.make_jenv_db()
-        views.env_index(
-            self.app, self.env_type_db, env_db, jenv_db, self.save_callable)
+        views.env_index(self.app, self.make_params(env_db, jenv_db))
         self.assertEqual(
             'No Juju environments already set up: please create one',
             self.app.get_title())
@@ -215,9 +223,7 @@
         env_db = helpers.make_env_db()
         jenv_db = {'environments': {}}
         with local_envs_supported(True):
-            views.env_index(
-                self.app, self.env_type_db, env_db, jenv_db,
-                self.save_callable)
+            views.env_index(self.app, self.make_params(env_db, jenv_db))
         buttons = self.get_widgets_in_contents(
             filter_function=self.is_a(ui.MenuButton))
         # A button is created for each existing environment (see details) and
@@ -232,9 +238,7 @@
         env_db = {'environments': {}}
         jenv_db = helpers.make_jenv_db()
         with local_envs_supported(True):
-            views.env_index(
-                self.app, self.env_type_db, env_db, jenv_db,
-                self.save_callable)
+            views.env_index(self.app, self.make_params(env_db, jenv_db))
         buttons = self.get_widgets_in_contents(
             filter_function=self.is_a(ui.MenuButton))
         # A button is created for each existing environment (see details) and
@@ -249,6 +253,20 @@
             1
         )
         self.assertEqual(expected_buttons_number, len(buttons))
+        # A text widget is displayed as header for the imported environments.
+        widgets = self.get_widgets_in_contents(
+            filter_function=self.is_a(urwid.Text))
+        self.assertEqual(self.active_environments_message, widgets[2].text)
+
+    def test_view_contents_without_imported_envs(self):
+        # If there are no active imported environments the corresponding
+        # header text is not displayed.
+        env_db = jenv_db = {'environments': {}}
+        views.env_index(self.app, self.make_params(env_db, jenv_db))
+        widgets = self.get_widgets_in_contents(
+            filter_function=self.is_a(urwid.Text))
+        texts = [widget.text for widget in widgets]
+        self.assertNotIn(self.active_environments_message, texts)
 
     def test_new_local_environment_disabled(self):
         # The option to create a new local environment is not present if they
@@ -256,9 +274,7 @@
         env_db = helpers.make_env_db()
         jenv_db = helpers.make_jenv_db()
         with local_envs_supported(False):
-            views.env_index(
-                self.app, self.env_type_db, env_db, jenv_db,
-                self.save_callable)
+            views.env_index(self.app, self.make_params(env_db, jenv_db))
         buttons = self.get_widgets_in_contents(
             filter_function=self.is_a(ui.MenuButton))
         captions = map(cli_helpers.get_button_caption, buttons)
@@ -273,8 +289,8 @@
         # The environment detail view is called when clicking an environment.
         env_db = helpers.make_env_db()
         jenv_db = {'environments': {}}
-        views.env_index(
-            self.app, self.env_type_db, env_db, jenv_db, self.save_callable)
+        params = self.make_params(env_db, jenv_db)
+        views.env_index(self.app, params)
         buttons = self.get_widgets_in_contents(
             filter_function=self.is_a(ui.MenuButton))
         # The environments are listed in alphabetical order.
@@ -288,9 +304,7 @@
             # When the button is clicked, the detail view is called passing the
             # corresponding environment data.
             cli_helpers.emit(button)
-            mock_env_detail.assert_called_once_with(
-                self.app, self.env_type_db, env_db, jenv_db,
-                self.save_callable, env_data)
+            mock_env_detail.assert_called_once_with(self.app, params, env_data)
             # Reset the mock so that it does not include any calls on the next
             # loop cycle.
             mock_env_detail.reset_mock()
@@ -300,10 +314,9 @@
         # The jenv detail view is called when clicking an imported environment.
         env_db = {'environments': {}}
         jenv_db = helpers.make_jenv_db()
+        params = self.make_params(env_db, jenv_db)
         with local_envs_supported(False):
-            views.env_index(
-                self.app, self.env_type_db, env_db, jenv_db,
-                self.save_callable)
+            views.env_index(self.app, params)
         buttons = self.get_widgets_in_contents(
             filter_function=self.is_a(ui.MenuButton))
         # The environments are listed in alphabetical order.
@@ -318,8 +331,7 @@
             # passing the corresponding environment data.
             cli_helpers.emit(button)
             mock_jenv_detail.assert_called_once_with(
-                self.app, self.env_type_db, env_db, jenv_db,
-                self.save_callable, env_data)
+                self.app, params, env_data)
             # Reset the mock so that it does not include any calls on the next
             # loop cycle.
             mock_jenv_detail.reset_mock()
@@ -330,10 +342,9 @@
         # environment.
         env_db = helpers.make_env_db()
         jenv_db = {'environments': {}}
+        params = self.make_params(env_db, jenv_db)
         with local_envs_supported(True):
-            views.env_index(
-                self.app, self.env_type_db, env_db, jenv_db,
-                self.save_callable)
+            views.env_index(self.app, params)
         buttons = self.get_widgets_in_contents(
             filter_function=self.is_a(ui.MenuButton))
         env_types = envs.get_supported_env_types(self.env_type_db)
@@ -347,8 +358,7 @@
             # corresponding environment data.
             cli_helpers.emit(button)
             mock_env_edit.assert_called_once_with(
-                self.app, self.env_type_db, env_db, jenv_db,
-                self.save_callable, {
+                self.app, params, {
                     'type': env_type,
                     'default-series': settings.JUJU_GUI_SUPPORTED_SERIES[-1],
                 })
@@ -365,9 +375,7 @@
         jenv_db = helpers.make_jenv_db()
         with maas_env_detected(False):
             with local_envs_supported(True):
-                views.env_index(
-                    self.app, self.env_type_db, env_db, jenv_db,
-                    self.save_callable)
+                views.env_index(self.app, self.make_params(env_db, jenv_db))
         buttons = self.get_widgets_in_contents(
             filter_function=self.is_a(ui.MenuButton))
         # The "create and bootstrap" button is the first one in the contents.
@@ -390,9 +398,7 @@
         env_db = envs.create_empty_env_db()
         jenv_db = helpers.make_jenv_db()
         with local_envs_supported(False):
-            views.env_index(
-                self.app, self.env_type_db, env_db, jenv_db,
-                self.save_callable)
+            views.env_index(self.app, self.make_params(env_db, jenv_db))
         buttons = self.get_widgets_in_contents(
             filter_function=self.is_a(ui.MenuButton))
         # No "create and bootstrap local" buttons are present.
@@ -408,9 +414,7 @@
         env_db = envs.create_empty_env_db()
         jenv_db = helpers.make_jenv_db()
         with maas_env_detected(True):
-            views.env_index(
-                self.app, self.env_type_db, env_db, jenv_db,
-                self.save_callable)
+            views.env_index(self.app, self.make_params(env_db, jenv_db))
         buttons = self.get_widgets_in_contents(
             filter_function=self.is_a(ui.MenuButton))
         # The "create and bootstrap" button is the first one in the contents.
@@ -433,9 +437,7 @@
         env_db = envs.create_empty_env_db()
         jenv_db = helpers.make_jenv_db()
         with maas_env_detected(False):
-            views.env_index(
-                self.app, self.env_type_db, env_db, jenv_db,
-                self.save_callable)
+            views.env_index(self.app, self.make_params(env_db, jenv_db))
         buttons = self.get_widgets_in_contents(
             filter_function=self.is_a(ui.MenuButton))
         # No "create and bootstrap MAAS" buttons are present.
@@ -446,8 +448,7 @@
         # The default environment is already selected in the list.
         env_db = helpers.make_env_db(default='lxc')
         jenv_db = helpers.make_jenv_db()
-        views.env_index(
-            self.app, self.env_type_db, env_db, jenv_db, self.save_callable)
+        views.env_index(self.app, self.make_params(env_db, jenv_db))
         env_data = envs.get_env_data(env_db, 'lxc')
         env_description = envs.get_env_short_description(env_data)
         contents = self.app.get_contents()
@@ -460,8 +461,7 @@
         # The status message explains how errors are displayed.
         env_db = helpers.make_env_db()
         jenv_db = {'environments': {}}
-        views.env_index(
-            self.app, self.env_type_db, env_db, jenv_db, self.save_callable)
+        views.env_index(self.app, self.make_params(env_db, jenv_db))
         status = self.app.get_status()
         self.assertEqual(self.base_status + ' \N{BULLET} has errors ', status)
 
@@ -469,8 +469,7 @@
         # The status message explains how default environment is represented.
         env_db = helpers.make_env_db(default='lxc', exclude_invalid=True)
         jenv_db = {'environments': {}}
-        views.env_index(
-            self.app, self.env_type_db, env_db, jenv_db, self.save_callable)
+        views.env_index(self.app, self.make_params(env_db, jenv_db))
         status = self.app.get_status()
         self.assertEqual(self.base_status + ' \N{CHECK MARK} default ', status)
 
@@ -478,8 +477,7 @@
         # The status message explains how active environments are displayed.
         env_db = helpers.make_env_db(exclude_invalid=True)
         jenv_db = helpers.make_jenv_db()
-        views.env_index(
-            self.app, self.env_type_db, env_db, jenv_db, self.save_callable)
+        views.env_index(self.app, self.make_params(env_db, jenv_db))
         status = self.app.get_status()
         self.assertEqual(self.base_status + ' \N{BULLET} active ', status)
 
@@ -487,8 +485,7 @@
         # The status message includes default, active and errors explanations.
         env_db = helpers.make_env_db(default='lxc')
         jenv_db = helpers.make_jenv_db()
-        views.env_index(
-            self.app, self.env_type_db, env_db, jenv_db, self.save_callable)
+        views.env_index(self.app, self.make_params(env_db, jenv_db))
         status = self.app.get_status()
         self.assertEqual(
             self.base_status +
@@ -501,8 +498,7 @@
         # The status only includes navigation info if there are no errors.
         env_db = helpers.make_env_db(exclude_invalid=True)
         jenv_db = {'environments': {}}
-        views.env_index(
-            self.app, self.env_type_db, env_db, jenv_db, self.save_callable)
+        views.env_index(self.app, self.make_params(env_db, jenv_db))
         status = self.app.get_status()
         self.assertEqual(self.base_status, status)
 
@@ -516,9 +512,8 @@
     def call_view(self, env_name='lxc'):
         """Call the view passing the env_data corresponding to env_name."""
         self.env_data = envs.get_env_data(self.env_db, env_name)
-        return views.env_detail(
-            self.app, self.env_type_db, self.env_db, self.jenv_db,
-            self.save_callable, self.env_data)
+        self.params = self.make_params(self.env_db, self.jenv_db)
+        return views.env_detail(self.app, self.params, self.env_data)
 
     def test_view_default_return_value_on_exit(self):
         # The view configures the app so that the return value on user exit is
@@ -584,9 +579,7 @@
         # The "back" button is the first one.
         back_button = self.get_control_buttons()[0]
         cli_helpers.emit(back_button)
-        mock_env_index.assert_called_once_with(
-            self.app, self.env_type_db, self.env_db, self.jenv_db,
-            self.save_callable)
+        mock_env_index.assert_called_once_with(self.app, self.params)
 
     def test_use_button(self):
         # The application exits if the "use" button is clicked.
@@ -608,14 +601,13 @@
         # The "set default" button is the third one.
         set_default_button = self.get_control_buttons()[2]
         cli_helpers.emit(set_default_button)
-        # The index view has been called passing the modified env_db as third
-        # argument.
+        # The index view has been called passing the modified env_db in params.
         self.assertTrue(mock_env_index.called)
-        new_env_db = mock_env_index.call_args[0][2]
+        params = mock_env_index.call_args[0][1]
         # The new env_db has a new default.
-        self.assertEqual(new_env_db['default'], 'ec2-west')
+        self.assertEqual(params.env_db['default'], 'ec2-west')
         # The new env_db has been saved.
-        self.save_callable.assert_called_once_with(new_env_db)
+        self.save_callable.assert_called_once_with(params.env_db)
 
     @mock.patch('quickstart.cli.views.env_edit')
     def test_edit_button(self, mock_env_edit):
@@ -625,8 +617,7 @@
         edit_button = self.get_control_buttons()[3]
         cli_helpers.emit(edit_button)
         mock_env_edit.assert_called_once_with(
-            self.app, self.env_type_db, self.env_db, self.jenv_db,
-            self.save_callable, self.env_data)
+            self.app, self.params, self.env_data)
 
     def test_remove_button(self):
         # A confirmation dialog is displayed if the "remove" button is clicked.
@@ -677,14 +668,13 @@
         # The "confirm" button is the second one in the dialog.
         confirm_button = buttons[1]
         cli_helpers.emit(confirm_button)
-        # The index view has been called passing the modified env_db as third
-        # argument.
+        # The index view has been called passing the modified env_db in params.
         self.assertTrue(mock_env_index.called)
-        new_env_db = mock_env_index.call_args[0][2]
+        params = mock_env_index.call_args[0][1]
         # The new env_db no longer includes the "ec2-west" environment.
-        self.assertNotIn('ec2-west', new_env_db['environments'])
+        self.assertNotIn('ec2-west', params.env_db['environments'])
         # The new env_db has been saved.
-        self.save_callable.assert_called_once_with(new_env_db)
+        self.save_callable.assert_called_once_with(params.env_db)
 
     def test_status_with_errors(self):
         # The status message explains how field errors are displayed.
@@ -709,9 +699,8 @@
     def call_view(self, env_name='lxc'):
         """Call the view passing the env_data corresponding to env_name."""
         self.env_data = envs.get_env_data(self.jenv_db, env_name)
-        return views.jenv_detail(
-            self.app, self.env_type_db, self.env_db, self.jenv_db,
-            self.save_callable, self.env_data)
+        self.params = self.make_params(self.env_db, self.jenv_db)
+        return views.jenv_detail(self.app, self.params, self.env_data)
 
     def test_view_default_return_value_on_exit(self):
         # The view configures the app so that the return value on user exit is
@@ -756,9 +745,7 @@
         # The "back" button is the first one.
         back_button = self.get_control_buttons()[0]
         cli_helpers.emit(back_button)
-        mock_env_index.assert_called_once_with(
-            self.app, self.env_type_db, self.env_db, self.jenv_db,
-            self.save_callable)
+        mock_env_index.assert_called_once_with(self.app, self.params)
 
     def test_use_button(self):
         # The application exits if the "use" button is clicked.
@@ -788,9 +775,8 @@
             self.env_data = envs.get_env_data(self.env_db, env_name)
         else:
             self.env_data = {'type': env_type}
-        return views.env_edit(
-            self.app, self.env_type_db, self.env_db, self.jenv_db,
-            self.save_callable, self.env_data)
+        self.params = self.make_params(self.env_db, self.jenv_db)
+        return views.env_edit(self.app, self.params, self.env_data)
 
     def get_form_contents(self):
         """Return the form contents included in the app page.
@@ -925,8 +911,7 @@
             'ec2-west successfully modified', self.app.get_message())
         # The application displays the environment detail view.
         mock_env_detail.assert_called_once_with(
-            self.app, self.env_type_db, self.env_db, self.jenv_db,
-            self.save_callable, new_env_data)
+            self.app, self.params, new_env_data)
 
     @mock.patch('quickstart.cli.views.env_detail')
     def test_save_empty_db(self, mock_env_detail):
@@ -945,8 +930,7 @@
         expected_new_env_data.update({'type': 'local', 'is-default': True})
         envs.set_env_data(self.env_db, None, expected_new_env_data)
         mock_env_detail.assert_called_once_with(
-            self.app, self.env_type_db, self.env_db, self.jenv_db,
-            self.save_callable, expected_new_env_data)
+            self.app, self.params, expected_new_env_data)
 
     def test_save_invalid_form_data(self):
         # Errors are displayed if the user tries to save invalid data.
@@ -990,9 +974,7 @@
         # The "cancel" button is the second one.
         cancel_button = self.get_control_buttons()[1]
         cli_helpers.emit(cancel_button)
-        mock_env_index.assert_called_once_with(
-            self.app, self.env_type_db, self.env_db, self.jenv_db,
-            self.save_callable)
+        mock_env_index.assert_called_once_with(self.app, self.params)
 
     @mock.patch('quickstart.cli.views.env_detail')
     def test_modification_view_cancel_button(self, mock_env_detail):
@@ -1003,5 +985,4 @@
         cancel_button = self.get_control_buttons()[1]
         cli_helpers.emit(cancel_button)
         mock_env_detail.assert_called_once_with(
-            self.app, self.env_type_db, self.env_db, self.jenv_db,
-            self.save_callable, self.env_data)
+            self.app, self.params, self.env_data)

=== modified file 'quickstart/tests/test_manage.py'
--- quickstart/tests/test_manage.py	2014-12-16 16:52:49 +0000
+++ quickstart/tests/test_manage.py	2015-01-07 16:46:42 +0000
@@ -35,7 +35,10 @@
     manage,
     settings,
 )
-from quickstart.cli import views
+from quickstart.cli import (
+    params,
+    views,
+)
 from quickstart.models import (
     envs,
     jenv,
@@ -391,9 +394,13 @@
             with mock.patch('quickstart.manage.views.show', mock_show):
                 yield
         mock_save_callable.assert_called_once_with(self.parser, self.env_file)
-        mock_show.assert_called_once_with(
-            views.env_index, self.env_type_db, env_db, jenv_db,
-            mock_save_callable())
+        expected_params = params.Params(
+            env_type_db=self.env_type_db,
+            env_db=env_db,
+            jenv_db=jenv_db,
+            save_callable=mock_save_callable(),
+        )
+        mock_show.assert_called_once_with(views.env_index, expected_params)
 
     def test_resulting_env_data(self):
         # The interactive session can be used to select an environment, in