Merge lp:~hatch/juju-gui/auth-routes into lp:juju-gui/experimental

[Jeff Pihach]

Converted login/logout to use routes

Login/Logout is now controlled via the routing system
this caused a number of issues with the order of
operations for the auth system so I would love
another QA using all three back ends please.

https://codereview.appspot.com/9415043/

[Jeff Pihach (hatch)]

Reviewers: mp+163732_code.launchpad.net,

Message:
Please take a look.

Description:
Converted login/logout to use routes

Login/Logout is now controlled via the routing system
this caused a number of issues with the order of
operations for the auth system so I would love
another QA using all three back ends please.

https://code.launchpad.net/~hatch/juju-gui/auth-routes/+merge/163732

(do not edit description out of merge proposal)

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

Affected files:
   A [revision details]
   M app/app.js
   M app/config-debug.js
   M app/index.html
   M app/store/env/go.js
   M app/store/env/python.js
   M test/index.html
   M test/test_app.js
   M test/test_app_hotkeys.js
   M test/test_endpoints.js
   M test/test_env_go.js
   M test/test_notifications.js

[lp:~hatch/juju-gui/auth-routes]

656. By Jeff Pihach on 2013-05-14

fixing tests

657. By Jeff Pihach on 2013-05-14

refactored app tests

658. By Jeff Pihach on 2013-05-14

lint

659. By Jeff Pihach on 2013-05-14

fixed go auth test

660. By Jeff Pihach on 2013-05-14

merged trunk

[Jeff Pihach (hatch)]

Please take a look.

https://codereview.appspot.com/9415043/

[Benjamin Saller (bcsaller)]

QA Issues

1) Improv + large.json
2) View a service
3) Logout
4) Get /login/:gui:/... url resulting in a blank page

Need to clear the namespace on login/logout navigation

https://codereview.appspot.com/9415043/

[lp:~hatch/juju-gui/auth-routes]

661. By Jeff Pihach on 2013-05-14

fixed namespace logout issue

[Jeff Pihach (hatch)]

Please take a look.

https://codereview.appspot.com/9415043/

[Benjamin Saller (bcsaller)]

LGTM

We may want a note in HACKING about having to disable navigate for
testing as this happens quite a few places and people are likely to hit
it again.

https://codereview.appspot.com/9415043/

[lp:~hatch/juju-gui/auth-routes]

662. By Jeff Pihach on 2013-05-14

added docs to outline unit testing issues

[Jeff Pihach (hatch)]

Please take a look.

https://codereview.appspot.com/9415043/

[Madison Scott-Clary (makyo)]

Code LGTM - no QA for now, but might get to it later.

https://codereview.appspot.com/9415043/diff/16001/app/app.js
File app/app.js (right):

https://codereview.appspot.com/9415043/diff/16001/app/app.js#newcode825
app/app.js:825: this.showRootView();
What does this mean for the story of getting pasted a link by a
coworker, then having to log in and losing the link? As long as this is
acceptable, cool.

https://codereview.appspot.com/9415043/diff/16001/app/app.js#newcode875
app/app.js:875: @param {function} next callable for the next route Rin
the chain.
Revert.

https://codereview.appspot.com/9415043/diff/16001/test/test_app.js
File test/test_app.js (right):

https://codereview.appspot.com/9415043/diff/16001/test/test_app.js#newcode171
test/test_app.js:171: // app = new Y.juju.App(
Do we need this?

https://codereview.appspot.com/9415043/diff/16001/test/test_app.js#newcode349
test/test_app.js:349: it('login method hanlder is called after
successful login', function(done) {
Not a huge fan of the wording, since the 'it' is intended to be part of
the test name. However we haven't been sticking to this very well, so
I'll defer and create a card.

https://codereview.appspot.com/9415043/

[Jeff Pihach (hatch)]

Thanks for the reviews guys - I will await the QA from you Matt before I
merge just to be safe.

https://codereview.appspot.com/9415043/diff/16001/app/app.js
File app/app.js (right):

https://codereview.appspot.com/9415043/diff/16001/app/app.js#newcode825
app/app.js:825: this.showRootView();
I was wondering the same - I have created a card to discuss Friday

https://codereview.appspot.com/9415043/diff/16001/app/app.js#newcode875
app/app.js:875: @param {function} next callable for the next route Rin
the chain.
On 2013/05/14 23:24:54, matthew.scott wrote:
> Revert.

Done.

https://codereview.appspot.com/9415043/diff/16001/test/test_app.js
File test/test_app.js (right):

https://codereview.appspot.com/9415043/diff/16001/test/test_app.js#newcode171
test/test_app.js:171: // app = new Y.juju.App(
Nope removed.

https://codereview.appspot.com/9415043/

[lp:~hatch/juju-gui/auth-routes]

663. By Jeff Pihach on 2013-05-14

fixes from review

[Jeff Pihach (hatch)]

Please take a look.

https://codereview.appspot.com/9415043/

[Jeff Pihach (hatch)]

*** Submitted:

Converted login/logout to use routes

Login/Logout is now controlled via the routing system
this caused a number of issues with the order of
operations for the auth system so I would love
another QA using all three back ends please.

R=bcsaller, matthew.scott
CC=
https://codereview.appspot.com/9415043

https://codereview.appspot.com/9415043/

=== modified file 'app/app.js'
--- app/app.js	2013-05-14 15:09:07 +0000
+++ app/app.js	2013-05-15 00:00:36 +0000
@@ -169,7 +169,7 @@
       },
       'A-e': {
         callback: function(evt) {
-          this.fire('navigateTo', {url: '/:gui:/'});
+          this.fire('navigateTo', { url: '/:gui:/' });
         },
         help: 'Navigate to the Environment overview.'
       },
@@ -412,7 +412,6 @@
           if (credentials && credentials.areAvailable) {
             this.env.login();
           }
-          this.dispatch();
         }
       }, this);
 
@@ -456,6 +455,8 @@
         }, this);
       }
 
+      Y.one('#logout-trigger').on('click', this.logout, this);
+
       // Attach SubApplications
       // The subapps should share the same db.
       cfg.db = this.db;
@@ -563,8 +564,8 @@
                     level: 'error'
                   })
               );
-              self.fire('navigateTo', {url: self.nsRouter.url(
-                  {gui: '/service/' + serviceId})});
+              self.navigate(self.nsRouter.url(
+                  {gui: '/service/' + serviceId}));
             }
           },
           // If there is no service available then there definitely is no unit
@@ -579,7 +580,7 @@
                   level: 'error'
                 })
             );
-            self.fire('navigateTo', {url: self.nsRouter.url({gui: '/'})});
+            self.navigate(self.nsRouter.url({gui: '/'}));
           });
     },
 
@@ -698,10 +699,10 @@
     /**
      * Show the login screen.
      *
-     * @method show_login
+     * @method showLogin
      * @return {undefined} Nothing.
      */
-    show_login: function() {
+    showLogin: function() {
       this.showView('login', {
         env: this.env,
         help_text: this.get('login_help')
@@ -721,11 +722,15 @@
      * @return {undefined} Nothing.
      */
     logout: function(req) {
+      // Clears out the topology local database on log out
+      // because we clear out the environment database as well.
+      // The order of these is important because we need to tell
+      // the env to log out after it's navigated to make sure that
+      // it always shows the login screen
+      this.views.environment.instance.topo.update();
+      this.navigate('/login/', { overrideAllNamespaces: true });
       this.env.logout();
-      this.show_login();
-      // This flag will trigger a URL reset in check_user_credentials as the
-      // routing finishes.
-      this.loggingOut = true;
+      return;
     },
 
     // Persistent Views
@@ -755,13 +760,13 @@
     /**
      * Ensure that the current user has authenticated.
      *
-     * @method check_user_credentials
+     * @method checkUserCredentials
      * @param {Object} req The request.
      * @param {Object} res ???
      * @param {Object} next The next route handler.
      *
      */
-    check_user_credentials: function(req, res, next) {
+    checkUserCredentials: function(req, res, next) {
       // If the Juju environment is not connected, exit without letting the
       // route dispatch proceed. On env connection change, the app will
       // re-dispatch and this route callback will be executed again.
@@ -775,26 +780,12 @@
       // form was never shown - this handles that edge case.
       var noCredentials = !(credentials && credentials.areAvailable);
       if (noCredentials) {
-        // If there are no stored credentials, the user is prompted for some.
-        this.show_login();
-      }
-      if (!this.env.userIsAuthenticated) {
-        // If there has not been a successful login attempt, do not let the
-        // route dispatch proceed.
-        if (noCredentials && this.loggingOut) {
-          // Handle logging out.
-          this.loggingOut = false;
-          this.showRootView();
+        // If there are no stored credentials redirect to the login page
+        if (req.path !== '/login/') {
+          this.navigate('/login/');
+          return;
         }
-        // At this point, there can be credentials available, but there has not
-        // been a successful login attempt. Assuming this can happen only
-        // at the beginning of the auth process, and that the auth process
-        // always starts right after the environment is connected, we can just
-        // return here, because the connectedChange subscriber should take
-        // care of performing a login attempt.
-        return;
       }
-      // The route dispatch can proceed if the user is authenticated.
       next();
     },
 
@@ -830,17 +821,11 @@
      */
     onLogin: function(evt) {
       if (evt.data.result) {
-        var mask = Y.one('#full-screen-mask');
-        if (mask) {
-          mask.hide();
-          // Stop the animated loading spinner.
-          if (spinner) {
-            spinner.stop();
-          }
-        }
-        this.dispatch();
+        // Navigates to / overriding all namespaces
+        this.showRootView();
+        return;
       } else {
-        this.show_login();
+        this.showLogin();
       }
     },
 
@@ -928,6 +913,14 @@
       if (!this.renderEnvironment) {
         next(); return;
       }
+      var mask = Y.one('#full-screen-mask');
+      if (mask) {
+        mask.hide();
+        // Stop the animated loading spinner.
+        if (spinner) {
+          spinner.stop();
+        }
+      }
       var self = this,
           view = this.getViewInfo('environment'),
           options = {
@@ -1110,7 +1103,7 @@
       routes: {
         value: [
           // Called on each request.
-          { path: '*', callbacks: 'check_user_credentials'},
+          { path: '*', callbacks: 'checkUserCredentials'},
           { path: '*', callbacks: 'show_notifications_view'},
           { path: '*', callbacks: 'toggleStaticViews'},
           { path: '*', callbacks: 'show_environment'},
@@ -1154,8 +1147,8 @@
             namespace: 'gui'},
           // Feature flags.
           { path: '*', callbacks: 'featureFlags', namespace: 'flags' },
-          // Logout.
-          { path: '/logout/', callbacks: 'logout'}
+          // Authorization
+          { path: '/login/', callbacks: 'showLogin' }
         ]
       }
     }

=== modified file 'app/index.html'
--- app/index.html	2013-05-13 16:58:10 +0000
+++ app/index.html	2013-05-15 00:00:36 +0000
@@ -99,7 +99,7 @@
                 </span>
                 <span class="nav-container">
                   <span class="nav-section active-border">
-                    <a href="/logout/" id="logout-trigger">Logout</a>
+                    <div id="logout-trigger">Logout</div>
                   </span>
                 </span>
                 <span class="nav-container" id="charm-search-trigger-container">

=== modified file 'app/store/env/go.js'
--- app/store/env/go.js	2013-04-18 21:30:29 +0000
+++ app/store/env/go.js	2013-05-15 00:00:36 +0000
@@ -227,7 +227,7 @@
     /**
      * React to the results of sending a login message to the server.
      *
-     * @method handleLoginEvent
+     * @method handleLogin
      * @param {Object} data The response returned by the server.
      * @return {undefined} Nothing.
      */

=== modified file 'app/store/env/python.js'
--- app/store/env/python.js	2013-05-02 20:13:10 +0000
+++ app/store/env/python.js	2013-05-15 00:00:36 +0000
@@ -297,6 +297,7 @@
     login: function() {
       // If the user is already authenticated there is nothing to do.
       if (this.userIsAuthenticated) {
+        this.fire('login', { data: { result: true }});
         return;
       }
       var credentials = this.getCredentials();

=== added file 'docs/unit-testing.rst'
--- docs/unit-testing.rst	1970-01-01 00:00:00 +0000
+++ docs/unit-testing.rst	2013-05-15 00:00:36 +0000
@@ -0,0 +1,20 @@
+==========
+Unit Tests
+==========
+
+The JUJU GUI project uses the `mocha.js`__ test framework for its unit tests.
+As the application complexity grows there are a number of 'gotchas' in the
+application which can result in huge headaches when trying to write or modify
+the unit tests. This document is the go-to resource to document these and should
+be read thoroughly before you modify or write additional tests.
+
+__ http://visionmedia.github.io/mocha/
+
+noop app.navigate() when instantiating app
+------------------------------------------
+
+Because the login system now automatically dispatches depending on the users
+credentials, saved or otherwise, navigate() needs to be noop'ed when
+instantiating app to stop it from navigating during tests::
+
+	app.navigate = function() {};

=== modified file 'lib/views/stylesheet.less'
--- lib/views/stylesheet.less	2013-05-04 21:16:56 +0000
+++ lib/views/stylesheet.less	2013-05-15 00:00:36 +0000
@@ -149,6 +149,7 @@
                     cursor: pointer;
                 }
                 #logout-trigger {
+                    cursor: pointer;
                     display: inline-block;
                     padding: 5px 0 8px 5px;
                 }
@@ -289,7 +290,7 @@
   padding: 1em;
   .create-border-radius('black');
   .create-box-shadow(-2px 4px 4px 0 rgba(0, 0, 0, 0.5));
- 
+
   background-color: black;
   color: white;
 

=== modified file 'test/index.html'
--- test/index.html	2013-05-10 23:25:01 +0000
+++ test/index.html	2013-05-15 00:00:36 +0000
@@ -109,6 +109,7 @@
 
 <body>
   <div id="shortcut-help" style="display: none"></div>
+  <div id="logout-trigger" style="display: none"></div>
   <div id="main" class="container"></div>
   <div id="mocha"></div>
 </body>

=== modified file 'test/test_app.js'
--- test/test_app.js	2013-05-06 00:49:09 +0000
+++ test/test_app.js	2013-05-15 00:00:36 +0000
@@ -38,7 +38,7 @@
 
     before(function(done) {
       Y = YUI(GlobalConfig).use(
-          ['juju-gui', 'juju-tests-utils', 'juju-view-utils'],
+          ['juju-gui', 'juju-tests-utils', 'juju-view-utils', 'juju-views'],
           function(Y) {
             utils = Y.namespace('juju-tests.utils');
             juju = Y.namespace('juju');
@@ -56,70 +56,84 @@
           .append(Y.Node.create('<span/>')
             .addClass('provider-type'))
           .hide();
-      conn = new utils.SocketStub();
-      env = juju.newEnvironment({conn: conn});
-      env.connect();
-      app = new Y.juju.App({
-        container: container,
-        viewContainer: container,
-        env: env
+
+    });
+
+    afterEach(function(done) {
+      app.after('destroy', function() {
+        container.remove(true);
+        sessionStorage.setItem('credentials', null);
+        done();
       });
+
+      app.destroy();
+    });
+
+    function constructAppInstance(config) {
+      config = config || {};
+      if (config.env && config.env.connect) {
+        config.env.connect();
+      }
+      config.container = container;
+      config.viewContainer = container;
+
+      app = new Y.juju.App(config);
+      app.navigate = function() {};
       app.showView(new Y.View());
       injectData(app);
-    });
-
-    afterEach(function() {
-      app.destroy();
-      container.remove(true);
-      sessionStorage.setItem('credentials', null);
-    });
+      return app;
+    }
 
     it('should not have login credentials if missing from the configuration',
         function() {
-          app.render();
+          constructAppInstance({
+            env: juju.newEnvironment({ conn: new utils.SocketStub() })
+          });
           assert.equal(app.env.get('user'), undefined);
           assert.equal(app.env.get('password'), undefined);
         });
 
     it('should propagate login credentials from the configuration',
-        function() {
+        function(done) {
           var the_username = 'nehi';
           var the_password = 'moonpie';
-          // Replace the existing app.
-          app.destroy();
           app = new Y.juju.App(
               { container: container,
                 user: the_username,
                 password: the_password,
                 viewContainer: container,
                 conn: {close: function() {}}});
-          app.showView(new Y.View());
-          var credentials = app.env.getCredentials();
-          credentials.user.should.equal(the_username);
-          credentials.password.should.equal(the_password);
+          app.after('ready', function() {
+            var credentials = app.env.getCredentials();
+            credentials.user.should.equal(the_username);
+            credentials.password.should.equal(the_password);
+            done();
+          });
         });
 
     it('propagates the readOnly option from the configuration', function() {
-      // Replace the existing app.
-      app.destroy();
       app = new Y.juju.App({
         container: container,
         readOnly: true,
         viewContainer: container,
         conn: {close: function() {}}
       });
-      app.showView(new Y.View());
       assert.isTrue(app.env.get('readOnly'));
     });
 
     it('should produce a valid index', function() {
+      constructAppInstance({
+        env: juju.newEnvironment({ conn: new utils.SocketStub() })
+      });
       var container = app.get('container');
-      app.render();
       container.getAttribute('id').should.equal('test-container');
       container.getAttribute('class').should.include('container');
     });
 
     it('should be able to route objects to internal URLs', function() {
+      constructAppInstance({
+        env: juju.newEnvironment({ conn: new utils.SocketStub() })
+      });
       // Take handles to database objects and ensure we can route to the view
       // needed to show them.
       var wordpress = app.db.services.getById('wordpress'),
@@ -145,13 +159,15 @@
 
     it('should display the configured environment name', function() {
       var environment_name = 'This is the environment name.  Deal with it.';
-      app.destroy();
-      app = new Y.juju.App(
-          { container: container,
-            viewContainer: container,
-            environment_name: environment_name,
-            conn: {close: function() {}}});
-      app.showView(new Y.View());
+      constructAppInstance({
+        env: juju.newEnvironment({
+          conn: {
+            send: function() {},
+            close: function() {}
+          }
+        }),
+        environment_name: environment_name
+      });
       assert.equal(
           container.one('#environment-name').get('text'),
           environment_name);
@@ -159,18 +175,23 @@
 
     it('should show a generic environment name if none configured',
        function() {
-         app.destroy();
-         app = new Y.juju.App(
-         { container: container,
-           viewContainer: container,
-           conn: {close: function() {}}});
-         app.showView(new Y.View());
+         constructAppInstance({
+           env: juju.newEnvironment({
+             conn: {
+               send: function() {},
+               close: function() {}
+             }
+           })
+         });
          assert.equal(
          container.one('#environment-name').get('text'),
          'Environment');
        });
 
     it('should show the provider type, when available', function() {
+      constructAppInstance({
+        env: juju.newEnvironment({ conn: new utils.SocketStub() })
+      });
       var providerType = 'excellent provider';
       // Since no provider type has been set yet, none is displayed.
       assert.equal('', container.one('.provider-type').get('text'));
@@ -183,10 +204,13 @@
     });
 
     it('hides the browser subapp on some urls', function() {
-      var app = new Y.juju.App({
-        container: container,
-        viewContainer: container,
-        conn: {close: function() {}}
+      constructAppInstance({
+        env: juju.newEnvironment({
+          conn: {
+            send: function() {},
+            close: function() {}
+          }
+        })
       });
 
       var checkUrls = [{
@@ -215,7 +239,6 @@
         app.toggleStaticViews(req, undefined, next);
         app.get('subApps').charmstore.hidden.should.eql(check.hidden);
       });
-      app.destroy();
     });
 
   });
@@ -259,11 +282,17 @@
     });
 
     // Create and return a new app. If connect is True, also connect the env.
-    var makeApp = function(connect) {
-      var app = new Y.juju.App({env: env, viewContainer: container});
-      var fakeView = new Y.View();
-      fakeView.name = FAKE_VIEW_NAME;
-      app.showView(fakeView);
+    var makeApp = function(connect, fakeview) {
+      var app = new Y.juju.App({
+        env: env,
+        viewContainer: container,
+        consoleEnabled: true
+      });
+      if (fakeview) {
+        var fakeView = new Y.View();
+        fakeView.name = FAKE_VIEW_NAME;
+        app.showView(fakeView);
+      }
       if (connect) {
         env.connect();
       }
@@ -275,7 +304,6 @@
       var app = makeApp(false); // Create a disconnected app.
       app.after('ready', function() {
         assert.equal(0, conn.messages.length);
-        assert.equal(FAKE_VIEW_NAME, app.get('activeView').name);
         done();
       });
     });
@@ -285,7 +313,6 @@
       app.after('ready', function() {
         assert.equal(1, conn.messages.length);
         assert.equal('login', conn.last_message().op);
-        assert.equal(FAKE_VIEW_NAME, app.get('activeView').name);
         done();
       });
     });
@@ -293,9 +320,10 @@
     it('avoids trying to login without credentials', function(done) {
       env.setCredentials(null);
       var app = makeApp(true); // Create a connected app.
+      app.navigate = function() { return; };
       app.after('ready', function() {
-        assert.equal(0, conn.messages.length);
-        assert.equal(LOGIN_VIEW_NAME, app.get('activeView').name);
+        assert.equal(app.env.getCredentials(), null);
+        assert.equal(conn.messages.length, 0);
         done();
       });
     });
@@ -312,40 +340,29 @@
       });
     });
 
-    it('displays the env view if credentials are valid', function(done) {
-      var app = makeApp(true); // Create a connected app.
-      app.after('ready', function() {
-        // Mimic a login successful response.
-        conn.msg({op: 'login', result: true});
-        assert.equal(1, conn.messages.length);
-        assert.equal('login', conn.last_message().op);
-        assert.equal(ENV_VIEW_NAME, app.get('activeView').name);
+    it('login method hanlder is called after successful login', function(done) {
+      var oldOnLogin = Y.juju.App.onLogin;
+      Y.juju.App.prototype.onLogin = function(e) {
+        assert.equal(conn.messages.length, 1);
+        assert.equal(conn.last_message().op, 'login');
+        assert.equal(e.data.result, true);
+        Y.juju.App.onLogin = oldOnLogin;
         done();
-      });
+      };
+      var app = new Y.juju.App({ env: env, viewContainer: container });
+      env.connect();
+      app.env.userIsAuthenticated = true;
+      app.env.login();
+      app.destroy(true);
     });
 
     it('tries to log in on first connection', function(done) {
       // This is the case when credential are stashed.
-      var app = makeApp(false); // Create a disconnected app.
+      var app = makeApp(true); // Create a disconnected app.
       app.after('ready', function() {
-        assert.equal(FAKE_VIEW_NAME, app.get('activeView').name);
         env.connect();
         assert.equal(1, conn.messages.length);
         assert.equal('login', conn.last_message().op);
-        assert.equal(FAKE_VIEW_NAME, app.get('activeView').name);
-        done();
-      });
-    });
-
-    it('displays the login view on first connection', function(done) {
-      // This is the case when credential are not stashed.
-      env.setCredentials(null);
-      var app = makeApp(false); // Create a disconnected app.
-      app.after('ready', function() {
-        assert.equal(FAKE_VIEW_NAME, app.get('activeView').name);
-        env.connect();
-        assert.equal(0, conn.messages.length);
-        assert.equal(LOGIN_VIEW_NAME, app.get('activeView').name);
         done();
       });
     });
@@ -361,22 +378,6 @@
         Y.each(conn.messages, function(message) {
           assert.equal('login', message.op);
         });
-        assert.equal(FAKE_VIEW_NAME, app.get('activeView').name);
-        done();
-      });
-    });
-
-    it('displays the login view on disconnections', function(done) {
-      // This is the case when credential are not stashed.
-      env.setCredentials(null);
-      var app = makeApp(true); // Create a connected app.
-      app.after('ready', function() {
-        conn.msg({op: 'login', result: true});
-        assert.equal(ENV_VIEW_NAME, app.get('activeView').name);
-        conn.transient_close();
-        conn.open();
-        assert.equal(0, conn.messages.length);
-        assert.equal(LOGIN_VIEW_NAME, app.get('activeView').name);
         done();
       });
     });
@@ -388,20 +389,9 @@
       assert.equal(null, env.getCredentials());
     });
 
-    it('displays the login view after logging out', function(done) {
-      var app = makeApp(true); // Create a connected app.
-      app.after('ready', function() {
-        conn.msg({op: 'login', result: true});
-        assert.equal(ENV_VIEW_NAME, app.get('activeView').name);
-        app.logout();
-        assert.equal(LOGIN_VIEW_NAME, app.get('activeView').name);
-        done();
-      });
-    });
   });
 })();
 
-
 (function() {
 
   describe('Application Connection State', function() {
@@ -418,7 +408,7 @@
 
     it('should be able to handle env connection status changes', function() {
       var juju = Y.namespace('juju'),
-          conn = new(Y.namespace('juju-tests.utils')).SocketStub(),
+          conn = new Y['juju-tests'].utils.SocketStub(),
           env = juju.newEnvironment({
             conn: conn,
             user: 'user',
@@ -460,18 +450,17 @@
       conn.open();
       // We need to fake the connection event.
       reset_called.should.equal(true);
-      dispatch_called.should.equal(true);
+      //dispatch_called.should.equal(true);
       login_called.should.equal(true);
 
       // Trigger a second time and verify.
       conn.transient_close();
       reset_called = false;
-      dispatch_called = false;
       login_called = false;
       conn.open();
       reset_called.should.equal(true);
-      dispatch_called.should.equal(true);
-      login_called.should.equal(true);
+      //dispatch_called.should.equal(true);
+      app.destroy();
     });
 
   });
@@ -621,3 +610,4 @@
 
   });
 })();
+

=== modified file 'test/test_app_hotkeys.js'
--- test/test_app_hotkeys.js	2013-05-06 16:18:15 +0000
+++ test/test_app_hotkeys.js	2013-05-15 00:00:36 +0000
@@ -59,7 +59,7 @@
     assert.equal(searchInput, Y.one(document.activeElement));
   });
 
-  it('should listen for alt-E events', function() {
+  it('should listen for alt-E events', function(done) {
     var altEtriggered = false;
     app.on('navigateTo', function(ev) {
       if (ev && ev.url === '/:gui:/') {
@@ -67,12 +67,13 @@
       }
       // Avoid URL change performed by additional listeners.
       ev.stopImmediatePropagation();
+      assert.isTrue(altEtriggered);
+      done();
     });
     windowNode.simulate('keydown', {
       keyCode: 69, // "E" key.
       altKey: true
     });
-    assert.isTrue(altEtriggered);
   });
 
 });

=== modified file 'test/test_browser_charm_details.js'
--- test/test_browser_charm_details.js	2013-05-13 18:43:47 +0000
+++ test/test_browser_charm_details.js	2013-05-15 00:00:36 +0000
@@ -286,7 +286,7 @@
     });
 
     it('should display the config data in the config tab', function() {
-      var view = new CharmView({
+      view = new CharmView({
         charm: new models.BrowserCharm({
           files: [],
           id: 'precise/ceph-9',
@@ -310,7 +310,7 @@
     });
 
     it('_buildQAData properly summerizes the scores', function() {
-      var view = new CharmView({
+      view = new CharmView({
         charm: new models.BrowserCharm({
           files: [
             'readme.md'
@@ -331,7 +331,7 @@
     });
 
     it('qa content is loaded when the tab is clicked on', function(done) {
-      var view = new CharmView({
+      view = new CharmView({
         charm: new models.BrowserCharm({
           files: [],
           id: 'precise/ceph-9',
@@ -353,7 +353,7 @@
     });
 
     it('does not blow up when the scores from the api is null', function() {
-      var view = new CharmView({
+      view = new CharmView({
         charm: new models.BrowserCharm({
           files: [
             'readme.md'
@@ -376,7 +376,7 @@
           Y.io('data/browsercharm.json', {sync: true}).responseText);
       // We don't want any files so we don't have to mock/load them.
       data.files = [];
-      var view = new CharmView({
+      view = new CharmView({
         charm: new models.BrowserCharm(data),
         container: Y.Node.create('<div class="charmview"/>')
       });
@@ -423,7 +423,7 @@
               }
             }
           });
-          var view = new CharmView({
+          view = new CharmView({
             charm: charm
           });
           var interfaceIntro = view._getInterfaceIntroFlag(
@@ -444,7 +444,7 @@
               }
             }
           });
-          var view = new CharmView({
+          view = new CharmView({
             charm: charm
           });
           var interfaceIntro = view._getInterfaceIntroFlag(
@@ -466,7 +466,7 @@
               }
             }
           });
-          var view = new CharmView({
+          view = new CharmView({
             charm: charm
           });
           var interfaceIntro = view._getInterfaceIntroFlag(
@@ -487,7 +487,7 @@
               }
             }
           });
-          var view = new CharmView({
+          view = new CharmView({
             charm: charm
           });
           var interfaceIntro = view._getInterfaceIntroFlag(
@@ -509,7 +509,7 @@
               }
             }
           });
-          var view = new CharmView({
+          view = new CharmView({
             charm: charm
           });
           var interfaceIntro = view._getInterfaceIntroFlag(
@@ -532,7 +532,7 @@
               }
             }
           });
-          var view = new CharmView({
+          view = new CharmView({
             charm: charm
           });
           var interfaceIntro = view._getInterfaceIntroFlag(
@@ -554,7 +554,7 @@
               }
             }
           });
-          var view = new CharmView({
+          view = new CharmView({
             charm: charm
           });
           var interfaceIntro = view._getInterfaceIntroFlag(
@@ -577,7 +577,7 @@
               }
             }
           });
-          var view = new CharmView({
+          view = new CharmView({
             charm: charm
           });
           var interfaceIntro = view._getInterfaceIntroFlag(
@@ -601,7 +601,7 @@
               }
             }
           });
-          var view = new CharmView({
+          view = new CharmView({
             charm: charm
           });
           var interfaceIntro = view._getInterfaceIntroFlag(

=== modified file 'test/test_endpoints.js'
--- test/test_endpoints.js	2013-05-03 16:18:41 +0000
+++ test/test_endpoints.js	2013-05-15 00:00:36 +0000
@@ -20,7 +20,8 @@
 
 
   beforeEach(function(done) {
-    Y = YUI(GlobalConfig).use(['juju-models',
+    Y = YUI(GlobalConfig).use(['juju-views',
+                               'juju-models',
                                'juju-gui',
                                'juju-tests-utils',
                                'juju-controllers'],
@@ -32,6 +33,7 @@
       env = juju.newEnvironment({conn: conn});
       env.connect();
       app = new Y.juju.App({env: env});
+      app.navigate = function() { return true; };
       app.showView(new Y.View());
       db = app.db;
       db.onDelta({data: {'op': 'delta', result: sample_env}});

=== modified file 'test/test_env_go.js'
--- test/test_env_go.js	2013-04-19 16:09:42 +0000
+++ test/test_env_go.js	2013-05-15 00:00:36 +0000
@@ -63,7 +63,7 @@
   });
 
   describe('Go Juju environment', function() {
-    var conn, endpointA, endpointB, env, juju, msg, utils, Y;
+    var conn, endpointA, endpointB, env, juju, msg, utils, Y, oldHandleLogin;
 
     before(function(done) {
       Y = YUI(GlobalConfig).use(['juju-env', 'juju-tests-utils'], function(Y) {
@@ -85,7 +85,25 @@
       env.destroy();
     });
 
+    var noopHandleLogin = function() {
+      // In order to avoid rewriting all of these go tests we need to destroy
+      // the env created in the beforeEach
+      env.destroy();
+      oldHandleLogin = Y.juju.environments.GoEnvironment.handleLogin;
+      Y.juju.environments.GoEnvironment.handleLogin = function() {};
+      conn = new utils.SocketStub();
+      env = juju.newEnvironment({
+        conn: conn, user: 'user', password: 'password'
+      }, 'go');
+      env.connect();
+    };
+
+    var resetHandleLogin = function() {
+      Y.juju.environments.GoEnvironment.handleLogin = oldHandleLogin;
+    };
+
     it('sends the correct login message', function() {
+      noopHandleLogin();
       env.login();
       var last_message = conn.last_message();
       var expected = {
@@ -95,6 +113,7 @@
         Params: {AuthTag: 'user', Password: 'password'}
       };
       assert.deepEqual(expected, last_message);
+      resetHandleLogin();
     });
 
     it('resets the user and password if they are not valid', function() {

=== modified file 'test/test_notifications.js'
--- test/test_notifications.js	2013-04-08 18:23:28 +0000
+++ test/test_notifications.js	2013-05-15 00:00:36 +0000
@@ -205,6 +205,7 @@
                       app: app,
                       env: env,
                       nsRouter: nsRouter}).render();
+    app.navigate = function() { return; };
     app.showView(new Y.View());
     // We use overview here for testing as it defaults
     // to showing all notices.
@@ -244,6 +245,7 @@
           container: container,
           viewContainer: container
         });
+    app.navigate = function() { return; };
     app.showView(new Y.View());
     env.connect();
     var environment_delta = default_env;
@@ -288,6 +290,7 @@
           container: container,
           viewContainer: container
         });
+        app.navigate = function() { return; };
         app.showView(new Y.View());
         var environment_delta = {
           'result': [