Remove "requires" from modules-debug.js

test/test_app.js

 'use strict';
 
 function injectData(app, data) {
   var d = data || {
     'result': [
       ['service', 'add',
        {'charm': 'cs:precise/wordpress-6',
          'id': 'wordpress', 'exposed': false}],
       ['service', 'add', {'charm': 'cs:precise/mysql-6', 'id': 'mysql'}],
       ['relation', 'add',
@@ skipping 13 matching lines @@
                 {'machine': 0, 'agent-state': 'started',
           'public-address': '192.168.122.113', 'id': 'wordpress/0'}],
       ['unit', 'add',
        {'machine': 0, 'agent-state': 'started',
                   'public-address': '192.168.122.222', 'id': 'mysql/0'}]],
     'op': 'delta'};
   app.env.dispatch_result(d);
   return app;
 }
 
-describe('Application basics', function() {
-  var Y, app, container;
+YUI(GlobalConfig).use(['juju-gui', 'juju-tests-utils'], function(Y) {

[gary.poster, 2012/11/21 15:41:51]

This looks good to me.  However, it is a big change to how we write our tests,
and not consistently applied to our tests.  As I said on IRC, I think there is a
good argument to change our tests to be written this way, but I don't understand
why we need to change them here, in this branch.

In the interest of progress, I'm ok with landing this if you then make a
retrospective discussion card for us to collectively determine the way we want
to move forward.

+  describe('Application basics', function() {
+    var app, container;
 
-  before(function(done) {
-    Y = YUI(GlobalConfig).use(['juju-gui', 'juju-tests-utils'], function(Y) {
-      done();
+    beforeEach(function() {
+      if (container) {
+        container.remove(true);
+      }
+      container = Y.one('#main')
+        .appendChild(Y.Node.create('<div/>'))
+          .set('id', 'test-container')
+          .addClass('container')
+          .append(Y.Node.create('<span/>')
+            .set('id', 'environment-name'))
+          .append(Y.Node.create('<span/>')
+            .set('id', 'provider-type'));
+      app = new Y.juju.App(
+          { container: container,
+            viewContainer: container});
+      injectData(app);
+    });
+
+    it('should produce a valid index', function() {
+      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() {
+      // take handles to database objects and ensure we can route to the view
+      // needed to show them
+      var wordpress = app.db.services.getById('wordpress'),
+          wp0 = app.db.units.get_units_for_service(wordpress)[0],
+          wp_charm = app.db.charms.add({id: wordpress.get('charm')});
+
+      // 'service/wordpress/' is the primary and so other URL are not returned
+      app.getModelURL(wordpress).should.equal('/service/wordpress/');
+      // however passing 'intent' can force selection of another
+      app.getModelURL(wordpress, 'config').should.equal(
+          '/service/wordpress/config');
+
+      // service units use argument rewriting (thus not /u/wp/0)
+      app.getModelURL(wp0).should.equal('/unit/wordpress-0/');
+
+      // charms also require a mapping but only a name, not a function
+      app.getModelURL(wp_charm).should.equal(
+          '/charms/charms/precise/wordpress-6/json');
+    });
+
+    it('should display the configured environment name', function() {
+      var environment_name = 'This is the environment name.  Deal with it.';
+      app = new Y.juju.App(
+          { container: container,
+            viewContainer: container,
+            environment_name: environment_name});
+      assert.equal(
+          container.one('#environment-name').get('text'),
+          environment_name);
+    });
+
+    it('should show a generic environment name if none configured', function() {
+      app = new Y.juju.App(
+          { container: container,
+            viewContainer: container});
+      assert.equal(
+          container.one('#environment-name').get('text'),
+          'Environment');
+    });
+
+    it('should show the provider type, when available', function() {
+      var providerType = 'excellent provider';
+      // Since no provider type has been set yet, none is displayed.
+      assert.equal(
+          container.one('#provider-type').get('text'),
+          '');
+      app.env.set('providerType', providerType);
+      // The provider type has been displayed.
+      assert.equal(
+          container.one('#provider-type').get('text'),
+          'on ' + providerType);
+    });
+
+  });
+});
+
+YUI(GlobalConfig).use(['juju-gui', 'juju-tests-utils'], function(Y) {
+  describe('Application Connection State', function() {
+    var container;
+
+    before(function() {
+      container = Y.Node.create('<div id="test" class="container"></div>');
+    });
+
+    it('should be able to handle env connection status changes', function() {
+      var juju = Y.namespace('juju'),
+          conn = new(Y.namespace('juju-tests.utils')).SocketStub(),
+          env = new juju.Environment({conn: conn}),
+          app = new Y.juju.App({env: env, container: container}),
+          reset_called = false,
+          noop = function() {return this;};
+
+
+      // mock the db
+      app.db = {
+        // mock out notifications
+        // so app can start normally
+        notifications: {
+          addTarget: noop,
+          after: noop,
+          filter: noop,
+          map: noop,
+          on: noop,
+          size: function() {return 0;}
+        },
+        reset: function() {
+          reset_called = true;
+                 }
+      };
+      env.connect();
+      conn.open();
+      reset_called.should.equal(true);
+
+      // trigger a second time and verify
+      reset_called = false;
+      conn.open();
+      reset_called.should.equal(true);
+    });
+
+  });
+});
+
+YUI(GlobalConfig).use(['juju-models', 'juju-gui', 'datasource-local',
+  'juju-tests-utils', 'json-stringify'], function(Y) {
+  describe('Application prefetching', function() {
+    var models, conn, env, app, container, charm_store, data, juju;
+
+    before(function() {
+      models = Y.namespace('juju.models');
+    });
+
+    beforeEach(function() {
+      conn = new (Y.namespace('juju-tests.utils')).SocketStub(),
+      env = new Y.juju.Environment({conn: conn});
+      env.connect();
+      conn.open();
+      container = Y.Node.create('<div id="test" class="container"></div>');
+      data = [];
+      app = new Y.juju.App(
+          { container: container,
+            viewContainer: container,
+            env: env,
+            charm_store: {} });
+
+      app.updateEndpoints = function() {};
+      env.get_endpoints = function() {};
+    });
+
+    afterEach(function() {
+      container.destroy();
+      app.destroy();
+    });
+
+    it('must prefetch charm and service for service pages', function() {
+      injectData(app);
+      var _ = expect(
+          app.db.charms.getById('cs:precise/wordpress-6')).to.not.exist;
+      app.show_service({params: {id: 'wordpress'}, query: {}});
+      // The app made a request of juju for the service info.
+      conn.messages[conn.messages.length - 2].op.should.equal('get_service');
+      // The app also requested juju (not the charm store--see discussion in
+      // app/models/charm.js) for the charm info.
+      conn.last_message().op.should.equal('get_charm');
+      // Tests of the actual load machinery are in the model and env tests, and
+      // so are not repeated here.
     });
   });
-
-  beforeEach(function(done) {
-    //  XXX Apparently removing a DOM node is asynchronous (on Chrome at least)
-    //  and we occasionally lose the race if this code is in the afterEach
-    //  function, so instead we do it here, but only if one has previously been
-    //  created.
-    if (container) {
-      container.remove(true);
-    }
-    container = Y.one('#main')
-      .appendChild(Y.Node.create('<div/>'))
-        .set('id', 'test-container')
-        .addClass('container')
-        .append(Y.Node.create('<span/>')
-          .set('id', 'environment-name'))
-        .append(Y.Node.create('<span/>')
-          .set('id', 'provider-type'));
-    app = new Y.juju.App(
-        { container: container,
-          viewContainer: container});
-    injectData(app);
-    done();
-  });
-
-  it('should produce a valid index', function() {
-    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() {
-    // take handles to database objects and ensure we can route to the view
-    // needed to show them
-    var wordpress = app.db.services.getById('wordpress'),
-        wp0 = app.db.units.get_units_for_service(wordpress)[0],
-        wp_charm = app.db.charms.add({id: wordpress.get('charm')});
-
-    // 'service/wordpress/' is the primary and so other URL are not returned
-    app.getModelURL(wordpress).should.equal('/service/wordpress/');
-    // however passing 'intent' can force selection of another
-    app.getModelURL(wordpress, 'config').should.equal(
-        '/service/wordpress/config');
-
-    // service units use argument rewriting (thus not /u/wp/0)
-    app.getModelURL(wp0).should.equal('/unit/wordpress-0/');
-
-    // charms also require a mapping but only a name, not a function
-    app.getModelURL(wp_charm).should.equal(
-        '/charms/charms/precise/wordpress-6/json');
-  });
-
-  it('should display the configured environment name', function() {
-    var environment_name = 'This is the environment name.  Deal with it.';
-    app = new Y.juju.App(
-        { container: container,
-          viewContainer: container,
-          environment_name: environment_name});
-    assert.equal(
-        container.one('#environment-name').get('text'),
-        environment_name);
-  });
-
-  it('should show a generic environment name if none configured', function() {
-    app = new Y.juju.App(
-        { container: container,
-          viewContainer: container});
-    assert.equal(
-        container.one('#environment-name').get('text'),
-        'Environment');
-  });
-
-  it('should show the provider type, when available', function() {
-    var providerType = 'excellent provider';
-    // Since no provider type has been set yet, none is displayed.
-    assert.equal(
-        container.one('#provider-type').get('text'),
-        '');
-    app.env.set('providerType', providerType);
-    // The provider type has been displayed.
-    assert.equal(
-        container.one('#provider-type').get('text'),
-        'on ' + providerType);
-  });
-
 });
-
-
-describe('Application Connection State', function() {
-  var Y, container;
-
-  before(function(done) {
-    Y = YUI(GlobalConfig).use(['juju-gui', 'juju-tests-utils'], function(Y) {
-      container = Y.Node.create('<div id="test" class="container"></div>');
-      done();
-        });
-
-  });
-
-  it('should be able to handle env connection status changes', function() {
-    var juju = Y.namespace('juju'),
-        conn = new(Y.namespace('juju-tests.utils')).SocketStub(),
-        env = new juju.Environment({conn: conn}),
-        app = new Y.juju.App({env: env, container: container}),
-        reset_called = false,
-        noop = function() {return this;};
-
-
-    // mock the db
-    app.db = {
-      // mock out notifications
-      // so app can start normally
-      notifications: {
-        addTarget: noop,
-        after: noop,
-        filter: noop,
-        map: noop,
-        on: noop,
-        size: function() {return 0;}
-      },
-      reset: function() {
-        reset_called = true;
-               }
-    };
-    env.connect();
-    conn.open();
-    reset_called.should.equal(true);
-
-    // trigger a second time and verify
-    reset_called = false;
-    conn.open();
-    reset_called.should.equal(true);
-  });
-
-});
-
-describe('Application prefetching', function() {
-  var Y, models, conn, env, app, container, charm_store, data, juju;
-
-  before(function(done) {
-    Y = YUI(GlobalConfig).use(
-        'juju-models', 'juju-gui', 'datasource-local', 'juju-tests-utils',
-        'json-stringify',
-        function(Y) {
-          models = Y.namespace('juju.models');
-          done();
-        });
-  });
-
-  beforeEach(function() {
-    conn = new (Y.namespace('juju-tests.utils')).SocketStub(),
-    env = new Y.juju.Environment({conn: conn});
-    env.connect();
-    conn.open();
-    container = Y.Node.create('<div id="test" class="container"></div>');
-    data = [];
-    app = new Y.juju.App(
-        { container: container,
-          viewContainer: container,
-          env: env,
-          charm_store: {} });
-
-    app.updateEndpoints = function() {};
-    env.get_endpoints = function() {};
-  });
-
-  afterEach(function() {
-    container.destroy();
-    app.destroy();
-  });
-
-  it('must prefetch charm and service for service pages', function() {
-    injectData(app);
-    var _ = expect(
-        app.db.charms.getById('cs:precise/wordpress-6')).to.not.exist;
-    app.show_service({params: {id: 'wordpress'}, query: {}});
-    // The app made a request of juju for the service info.
-    conn.messages[conn.messages.length - 2].op.should.equal('get_service');
-    // The app also requested juju (not the charm store--see discussion in
-    // app/models/charm.js) for the charm info.
-    conn.last_message().op.should.equal('get_charm');
-    // Tests of the actual load machinery are in the model and env tests, and
-    // so are not repeated here.
-  });
-});