Merge lp:~benji/juju-gui/bug-1088507 into lp:juju-gui/experimental

[Benji York]

Make the prod tests pass.

Changes a test that was failing because of some slight style difference to be
more direct. Some slight style differences still exist between prod and dev.

The charm model module (app/models/charm.js) is now included in the combined
and minified JS file.

The event-simulate and node-event-simulate functions are needed for the tests,
so they are now directly loaded by test/index.html.

The test_charm_configuration.js file needs juju-charm-models but that module
was not specified.

test/test_model.js had to be rearranged so the YUI().use method was called
inside the "before" method instead of around the entire test module. This
resulted in a mass reindenting.

Incidentally: Fixes the Makefile so YUI module assets are in the right place
so requests for them do not 404.

https://codereview.appspot.com/6947057/

[Gary Poster (gary)]

Land with changes.

Thank you for accomplishing this difficult task.

In addition to the comments below, please add a request in process.rst
that reviewers run both prod tests and debug tests. Then, in the
Makefile, either make the default test the prod test, or make it thumb
its nose at us and tell us to run one of the other targets.

Gary

https://codereview.appspot.com/6947057/diff/1/Makefile
File Makefile (right):

https://codereview.appspot.com/6947057/diff/1/Makefile#newcode293
Makefile:293: cp -r --parents */assets
"$(PWD)/build-$(1)/juju-ui/assets/")
Looks nicer to me, thanks.

https://codereview.appspot.com/6947057/diff/1/bin/merge-files
File bin/merge-files (right):

https://codereview.appspot.com/6947057/diff/1/bin/merge-files#newcode50
bin/merge-files:50: paths.push(syspath.join(process.cwd(),
'app/models/charm.js'));
As we discussed, please either figure out why readdir is not finding
charm.js, or put an XXX and a bug, and we will come back to this.

https://codereview.appspot.com/6947057/diff/1/test/index.html
File test/index.html (right):

https://codereview.appspot.com/6947057/diff/1/test/index.html#newcode9
test/index.html:9: <script
src="/juju-ui/assets/node-event-simulate.js"></script>
You explained the horror behind these two inclusions. Please add a
comment so others can enjoy.

https://codereview.appspot.com/6947057/

[Nicola Larosa (teknico)]

Needs fixing.

Running "make test-prod" produces 60 failures here, which is admittedly
a nice improvement down from 79, but still not enough. :-)

You most likely don't see these failures: I'd like to understand why I
do.

There's one more thing I'd like to clarify, see comment inline.

https://codereview.appspot.com/6947057/diff/1/Makefile
File Makefile (right):

https://codereview.appspot.com/6947057/diff/1/Makefile#newcode293
Makefile:293: cp -r --parents */assets
"$(PWD)/build-$(1)/juju-ui/assets/")
There's been some back and forth on this. You originally put in the "cp"
instruction; then I wanted to have symlinks in the build directories
rather than copies, so I spent some effort to come up with the "find"
command that created the right links in the right places, and did not
see requests for them return 404. Your experience has apparently been
different. I'd like to understand: 1) why; 2) if there's value enough,
in having symlinks instead of copies, to worry about this.

https://codereview.appspot.com/6947057/

[lp:~benji/juju-gui/bug-1088507]

278. By Benji York on 2012-12-14

Review change: remove the "test" target and have it tell the user to be explicit about what they want

279. By Benji York on 2012-12-14

add more commentary from reveis

280. By Benji York on 2012-12-17

add comment suggested in review

281. By Benji York on 2012-12-17

add some test tiles to the build directories

[Nicola Larosa (teknico)]

It works now, thanks. Do not worry about the other issue, we'll worry
about it later.

Land this.

https://codereview.appspot.com/6947057/

=== modified file 'Makefile'
--- Makefile	2012-12-14 18:04:28 +0000
+++ Makefile	2012-12-17 14:57:47 +0000
@@ -284,15 +284,20 @@
 	ln -sf "$(PWD)/build/juju-ui/assets/sprite.png" build-$(1)/juju-ui/assets/
 	ln -sf "$(PWD)/node_modules/yui/assets/skins/sam/rail-x.png" \
 		build-$(1)/juju-ui/assets/combined-css/rail-x.png
-	# Link each YUI module's assets.
-	mkdir -p build-$(1)/juju-ui/assets/skins/night/ \
-		build-$(1)/juju-ui/assets/skins/sam/
-	find node_modules/yui/ -path "*/skins/night/*" -type f \
-		-exec ln -sf "$(PWD)/{}" build-$(1)/juju-ui/assets/skins/night/ \;
-	find node_modules/yui/ -path "*/skins/sam/*" -type f \
-		-exec ln -sf "$(PWD)/{}" build-$(1)/juju-ui/assets/skins/sam/ \;
-	find node_modules/yui/ -path "*/assets/*" \! -path "*/skins/*" -type f \
-		-exec ln -sf "$(PWD)/{}" build-$(1)/juju-ui/assets/ \;
+	ln -sf "$(PWD)/node_modules/yui/event-simulate/event-simulate.js" \
+		build-$(1)/juju-ui/assets/
+	ln -sf "$(PWD)/node_modules/yui/node-event-simulate/node-event-simulate.js" \
+		build-$(1)/juju-ui/assets
+	# Copy each YUI module's assets to a parallel directory in the build
+	# location.  This is run in a subshell (indicated by the parenthesis)
+	# so we can change directory and have it not effect this process.  To
+	# understand how it does what it does look at the man page for cp,
+	# particularly "--parents".  Notice that this makes copies instead of
+	# links.  This goes against the way the dependencies are structured and
+	# so may be a problem in the future.  If so, a way to do this as links
+	# would be called for.
+	(cd node_modules/yui/ && \
+	 cp -r --parents */assets "$(PWD)/build-$(1)/juju-ui/assets/")
 endef
 
 $(LINK_DEBUG_FILES):
@@ -319,7 +324,11 @@
 test-prod: build-prod
 	./test-server.sh prod
 
-test: test-debug
+test:
+	@echo "Deprecated. Please run either 'make test-prod' or 'make"
+	@echo "test-debug', to test the production or debug environments"
+	@echo "respectively.  Run 'make help' to list the main available "
+	@echo "targets."
 
 server:
 	@echo "Deprecated. Please run either 'make prod' or 'make debug',"
 
=== modified file 'app/app.js'
--- app/app.js	2012-12-04 22:08:40 +0000
+++ app/app.js	2012-12-17 14:57:47 +0000
@@ -761,6 +761,7 @@
 }, '0.5.2', {
   requires: [
     'juju-models',
+    'juju-charm-models',
     'juju-views',
     'juju-controllers',
     'juju-view-charm-search',
 
=== modified file 'app/views/charm-panel.js'
--- app/views/charm-panel.js	2012-12-03 22:24:16 +0000
+++ app/views/charm-panel.js	2012-12-17 14:57:47 +0000
@@ -697,6 +697,31 @@
           'input.config-field[type=checkbox]':
               {click: function(evt) {evt.target.focus();}}
         },
+        /**
+         * Determines the Y coordinate that would center a tooltip on a field.
+         *
+         * @static
+         * @param {Number} fieldY The current Y position of the tooltip.
+         * @param {Number} fieldHeight The hight of the field.
+         * @param {Number} tooltipHeight The height of the tooltip.
+         * @return {Number} New Y coordinate for the tooltip.
+         */
+        _calculateTooltipY: function(fieldY, fieldHeight, tooltipHeight) {
+          var y_offset = (tooltipHeight - fieldHeight) / 2;
+          return fieldY - y_offset;
+        },
+        /**
+         * Determines the X coordinate that would place a tooltip next to a
+         * field.
+         *
+         * @static
+         * @param {Number} fieldX The current X position of the tooltip.
+         * @param {Number} tooltipWidth The width of the tooltip.
+         * @return {Number} New X coordinate for the tooltip.
+         */
+        _calculateTooltipX: function(fieldX, tooltipWidth) {
+          return fieldX - tooltipWidth - 15;
+        },
         _moveTooltip: function() {
           if (this.tooltip.field &&
               Y.DOM.inRegion(
@@ -708,10 +733,13 @@
               var widget = this.tooltip.get('boundingBox'),
                   tooltipWidth = widget.get('clientWidth'),
                   tooltipHeight = widget.get('clientHeight'),
-                  y_offset = (tooltipHeight - fieldHeight) / 2;
-              this.tooltip.move(  // These are the x, y coordinates.
-                  [this.tooltip.panel.getX() - tooltipWidth - 15,
-                   this.tooltip.field.getY() - y_offset]);
+                  fieldX = this.tooltip.panel.getX(),
+                  fieldY = this.tooltip.field.getY(),
+                  tooltipX = this._calculateTooltipX(
+                      fieldX, tooltipWidth),
+                  tooltipY = this._calculateTooltipY(
+                      fieldY, fieldHeight, tooltipHeight);
+              this.tooltip.move([tooltipX, tooltipY]);
               if (!this.tooltip.get('visible')) {
                 this.tooltip.show();
               }
 
=== modified file 'bin/merge-files'
--- bin/merge-files	2012-12-14 20:39:08 +0000
+++ bin/merge-files	2012-12-17 14:57:47 +0000
@@ -48,6 +48,8 @@
 
   // templates.js is a generated file. It is not part of the app directory.
   paths.push(syspath.join(process.cwd(), 'build/juju-ui/templates.js'));
+  // XXX Why do we have to do this?  (bug 1090563).
+  paths.push(syspath.join(process.cwd(), 'app/models/charm.js'));
 
   merge.combineJs(paths, 'build/juju-ui/assets/app.js');
 
 
=== modified file 'test/index.html'
--- test/index.html	2012-12-11 04:11:39 +0000
+++ test/index.html	2012-12-17 14:57:47 +0000
@@ -3,8 +3,12 @@
 <head>
   <meta charset="utf-8">
   <link rel="stylesheet" href="assets/mocha.css">
+  <script src="/juju-ui/assets/modules.js"></script>
   <script src="/juju-ui/assets/all-yui.js"></script>
-  <script src="/juju-ui/assets/modules.js"></script>
+  <!-- Since only the tests depend on these files and the prod tests disable
+    the YUI loader, we have to include them manually here. -->
+  <script src="/juju-ui/assets/event-simulate.js"></script>
+  <script src="/juju-ui/assets/node-event-simulate.js"></script>
   <script src="assets/chai.js"></script>
   <script src="assets/mocha.js"></script>
   <script src="utils.js"></script>
 
=== modified file 'test/test_charm_configuration.js'
--- test/test_charm_configuration.js	2012-10-29 10:01:29 +0000
+++ test/test_charm_configuration.js	2012-12-17 14:57:47 +0000
@@ -24,6 +24,7 @@
   before(function(done) {
     Y = YUI(GlobalConfig).use(
         'juju-models',
+        'juju-charm-models',
         'juju-views',
         'juju-gui',
         'juju-env',
@@ -199,35 +200,25 @@
     tooltip.get('visible').should.equal(true);
   });
 
-  it('must keep the tooltip aligned with its field', function() {
-    var charm = new models.Charm({id: 'precise/mysql-7'}),
-        view = new views.CharmConfigurationView(
-        { container: container,
-          model: charm,
-          tooltipDelay: 0 });
-    charm.setAttrs(charmConfig);
-    charm.loaded = true;
-    view.render();
-    var tooltip = view.tooltip,
-        controls = container.all('.control-group input'),
-        panel = container.one('.charm-panel');
-    // The panel needs to be scrollable and smaller than what it contains.  We
-    // do this by setting a height to the panel and then setting the height to
-    // one of the controls to something much bigger.
-    panel.setStyles({height: '400px', overflowY: 'auto'});
-    controls.item(2).set('height', '4000px');
-    // We need to have the field visible or else the call to "focus" will
-    // change the positioning after our calculation has occurred, thus
-    // changing our Y field.
-    controls.item(1).scrollIntoView();
-    controls.item(1).focus();
-    var originalY = tooltip.get('boundingBox').getY();
-    panel.set('scrollTop', panel.get('scrollTop') + 10);
-    // The simulate module does not support firing scroll events so we call
-    // the associated method directly.
-    view._moveTooltip();
-    Math.floor(tooltip.get('boundingBox').getY())
-      .should.equal(Math.floor(originalY - 10));
+  it('must keep the tooltip aligned with its field vertically', function() {
+    // The tooltip's Y coordinate should be such that it is centered vertically
+    // on its associated field.
+    var fieldHeight = 7;
+    var tooltipHeight = 17;
+    var fieldY = 1000;
+    var view = new views.CharmConfigurationView();
+    var y = view._calculateTooltipY(fieldY, fieldHeight, tooltipHeight);
+    assert.equal(y, 995);
+  });
+
+  it('must keep the tooltip to the left of its field', function() {
+    // The tooltip's X coordinate should be such that it is to the left of its
+    // associated field.
+    var tooltipWidth = 100;
+    var fieldX = 1000;
+    var view = new views.CharmConfigurationView();
+    var x = view._calculateTooltipX(fieldX, tooltipWidth);
+    assert.equal(x, 885);
   });
 
   it('must hide the tooltip when its field scrolls away', function() {
 
=== modified file 'test/test_model.js'
--- test/test_model.js	2012-11-23 16:21:32 +0000
+++ test/test_model.js	2012-12-17 14:57:47 +0000
@@ -1,356 +1,359 @@
 'use strict';
 
-YUI(GlobalConfig).use('juju-models', function(Y) {
-  describe('charm normalization', function() {
-    var models;
-
-    before(function() {
-      models = Y.namespace('juju.models');
-    });
-
-    it('must create derived attributes from official charm id', function() {
-      var charm = new models.Charm(
-          {id: 'cs:precise/openstack-dashboard-0'});
-      charm.get('scheme').should.equal('cs');
-      var _ = expect(charm.get('owner')).to.not.exist;
-      charm.get('full_name').should.equal('precise/openstack-dashboard');
-      charm.get('charm_store_path').should.equal(
-          'charms/precise/openstack-dashboard-0/json');
-    });
-
-    it('must convert timestamps into time objects', function() {
-      var time = 1349797266.032,
-          date = new Date(time),
-          charm = new models.Charm(
-          { id: 'cs:precise/foo-9', last_change: {created: time / 1000} });
-      charm.get('last_change').created.should.eql(date);
-    });
-
-  });
-});
-
-YUI(GlobalConfig).use('juju-models', function(Y) {
-  describe('juju models', function() {
-    var models;
-
-    before(function() {
-      models = Y.namespace('juju.models');
-    });
-
-    it('must be able to create charm', function() {
-      var charm = new models.Charm(
-          {id: 'cs:~alt-bac/precise/openstack-dashboard-0'});
-      charm.get('scheme').should.equal('cs');
-      charm.get('owner').should.equal('alt-bac');
-      charm.get('series').should.equal('precise');
-      charm.get('package_name').should.equal('openstack-dashboard');
-      charm.get('revision').should.equal(0);
-      charm.get('full_name').should.equal(
-          '~alt-bac/precise/openstack-dashboard');
-      charm.get('charm_store_path').should.equal(
-          '~alt-bac/precise/openstack-dashboard-0/json');
-    });
-
-    it('must be able to parse real-world charm names', function() {
-      var charm = new models.Charm({id: 'cs:precise/openstack-dashboard-0'});
-      charm.get('full_name').should.equal('precise/openstack-dashboard');
-      charm.get('package_name').should.equal('openstack-dashboard');
-      charm.get('charm_store_path').should.equal(
-          'charms/precise/openstack-dashboard-0/json');
-      charm.get('scheme').should.equal('cs');
-      var _ = expect(charm.get('owner')).to.not.exist;
-      charm.get('series').should.equal('precise');
-      charm.get('package_name').should.equal('openstack-dashboard');
-      charm.get('revision').should.equal(0);
-    });
-
-    it('must be able to parse individually owned charms', function() {
-      // Note that an earlier version of the parsing code did not handle
-      // hyphens in user names, so this test intentionally includes one.
-      var charm = new models.Charm(
-          {id: 'cs:~marco-ceppi/precise/wordpress-17'});
-      charm.get('full_name').should.equal('~marco-ceppi/precise/wordpress');
-      charm.get('package_name').should.equal('wordpress');
-      charm.get('charm_store_path').should.equal(
-          '~marco-ceppi/precise/wordpress-17/json');
-      charm.get('revision').should.equal(17);
-    });
-
-    it('must reject bad charm ids.', function() {
-      try {
-        var charm = new models.Charm({id: 'foobar'});
-        assert.fail('Should have thrown an error');
-      } catch (e) {
-        e.should.equal(
-            'Developers must initialize charms with a well-formed id.');
-      }
-    });
-
-    it('must reject missing charm ids at initialization.', function() {
-      try {
-        var charm = new models.Charm();
-        assert.fail('Should have thrown an error');
-      } catch (e) {
-        e.should.equal(
-            'Developers must initialize charms with a well-formed id.');
-      }
-    });
-
-    it('must be able to create charm list', function() {
-      var c1 = new models.Charm(
-          { id: 'cs:precise/mysql-2',
-            description: 'A DB'}),
-          c2 = new models.Charm(
-          { id: 'cs:precise/logger-3',
-            description: 'Log sub'}),
-          clist = new models.CharmList();
-      clist.add([c1, c2]);
-      var names = clist.map(function(c) {return c.get('package_name');});
-      names[0].should.equal('mysql');
-      names[1].should.equal('logger');
-    });
-
-
-    it('service unit list should be able to get units of a given service',
-       function() {
-         var sl = new models.ServiceList();
-         var sul = new models.ServiceUnitList();
-         var mysql = new models.Service({id: 'mysql'});
-         var wordpress = new models.Service({id: 'wordpress'});
-         sl.add([mysql, wordpress]);
-         sl.getById('mysql').should.equal(mysql);
-         sl.getById('wordpress').should.equal(wordpress);
-
-         sul.add([{id: 'mysql/0'}, {id: 'mysql/1'}]);
-
-         var wp0 = {id: 'wordpress/0'},
-         wp1 = {id: 'wordpress/1'};
-         sul.add([wp0, wp1]);
-         wp0.service.should.equal('wordpress');
-
-         sul.get_units_for_service(mysql, true).getAttrs(['id']).id.should.eql(
-         ['mysql/0', 'mysql/1']);
-         sul.get_units_for_service(wordpress, true).getAttrs(
-         ['id']).id.should.eql(['wordpress/0', 'wordpress/1']);
-       });
-
-    it('service unit list should be able to aggregate unit statuses',
-       function() {
-         var sl = new models.ServiceList();
-         var sul = new models.ServiceUnitList();
-         var mysql = new models.Service({id: 'mysql'});
-         var wordpress = new models.Service({id: 'wordpress'});
-         sl.add([mysql, wordpress]);
-
-         var my0 = new models.ServiceUnit(
-         {id: 'mysql/0', agent_state: 'pending'}),
-         my1 = new models.ServiceUnit(
-         {id: 'mysql/1', agent_state: 'pending'});
-
-         sul.add([my0, my1]);
-
-         var wp0 = new models.ServiceUnit(
-         { id: 'wordpress/0',
-           agent_state: 'pending'}),
-         wp1 = new models.ServiceUnit(
-         { id: 'wordpress/1',
-           agent_state: 'error'});
-         sul.add([wp0, wp1]);
-
-         sul.get_informative_states_for_service(mysql).should.eql(
-         {'pending': 2});
-         sul.get_informative_states_for_service(wordpress).should.eql(
-         {'pending': 1, 'error': 1});
-       });
-
-    it('service unit objects should parse the service name from unit id',
-       function() {
-         var service_unit = {id: 'mysql/0'},
-         db = new models.Database();
-         db.units.add(service_unit);
-         service_unit.service.should.equal('mysql');
-       });
-
-    it('service unit objects should report their number correctly',
-       function() {
-         var service_unit = {id: 'mysql/5'},
-         db = new models.Database();
-         db.units.add(service_unit);
-         service_unit.number.should.equal(5);
-       });
-
-    it('must be able to resolve models by modelId', function() {
-      var db = new models.Database();
-
-      db.services.add([{id: 'wordpress'}, {id: 'mediawiki'}]);
-      db.units.add([{id: 'wordpress/0'}, {id: 'wordpress/1'}]);
-
-      var model = db.services.item(0);
-      // Single parameter calling
-      db.getModelById([model.name, model.get('id')])
-               .get('id').should.equal('wordpress');
-      // Two parameter interface
-      db.getModelById(model.name, model.get('id'))
-               .get('id').should.equal('wordpress');
-
-      var unit = db.units.item(0);
-      db.getModelById([unit.name, unit.id]).id.should.equal('wordpress/0');
-      db.getModelById(unit.name, unit.id).id.should.equal('wordpress/0');
-    });
-
-    it('on_delta should handle remove changes correctly',
-       function() {
-         var db = new models.Database();
-         var my0 = new models.ServiceUnit({id: 'mysql/0',
-           agent_state: 'pending'}),
-         my1 = new models.ServiceUnit({id: 'mysql/1',
-           agent_state: 'pending'});
-         db.units.add([my0, my1]);
-         db.on_delta({data: {result: [
-           ['unit', 'remove', 'mysql/1']
-          ]}});
-         var names = db.units.get('id');
-         names.length.should.equal(1);
-         names[0].should.equal('mysql/0');
-       });
-
-    it('on_delta should be able to reuse existing services with add',
-       function() {
-         var db = new models.Database();
-         var my0 = new models.Service({id: 'mysql', exposed: true});
-         db.services.add([my0]);
-         // Note that exposed is not set explicitly to false.
-         db.on_delta({data: {result: [
-           ['service', 'add', {id: 'mysql'}]
-          ]}});
-         my0.get('exposed').should.equal(false);
-       });
-
-    it('on_delta should be able to reuse existing units with add',
-       // Units are special because they use the LazyModelList.
-       function() {
-         var db = new models.Database();
-         var my0 = {id: 'mysql/0', agent_state: 'pending'};
-         db.units.add([my0]);
-         db.on_delta({data: {result: [
-           ['unit', 'add', {id: 'mysql/0', agent_state: 'another'}]
-          ]}});
-         my0.agent_state.should.equal('another');
-       });
-
-    it('on_delta should reset relation_errors',
-       function() {
-         var db = new models.Database();
-         var my0 = {id: 'mysql/0', relation_errors: {'cache': ['memcached']}};
-         db.units.add([my0]);
-         // Note that relation_errors is not set.
-         db.on_delta({data: {result: [
-           ['unit', 'change', {id: 'mysql/0'}]
-          ]}});
-         my0.relation_errors.should.eql({});
-       });
-
-    it('ServiceUnitList should accept a list of units at instantiation and ' +
-       'decorate them', function() {
-         var mysql = new models.Service({id: 'mysql'});
-         var objs = [{id: 'mysql/0'},
-                     {id: 'mysql/1'}];
-         var sul = new models.ServiceUnitList({items: objs});
-         var unit_data = sul.get_units_for_service(
-                 mysql, true).getAttrs(['service', 'number']);
-         unit_data.service.should.eql(['mysql', 'mysql']);
-         unit_data.number.should.eql([0, 1]);
-       });
-
-    it('RelationList.has_relations.. should return true if rel found.',
-        function() {
-          var db = new models.Database(),
-              service = new models.Service({id: 'mysql', exposed: false}),
-              rel0 = new models.Relation({
-                id: 'relation-0',
-                endpoints: [
-                  ['mediawiki', {name: 'cache', role: 'source'}],
-                  ['squid', {name: 'cache', role: 'front'}]],
-                'interface': 'cache'
-              }),
-              rel1 = new models.Relation({
-                id: 'relation-4',
-                endpoints: [
-                  ['something', {name: 'foo', role: 'bar'}],
-                  ['mysql', {name: 'la', role: 'lee'}]],
-                'interface': 'thing'
-              });
-          db.relations.add([rel0, rel1]);
-          db.relations.has_relation_for_endpoint(
-              {service: 'squid', name: 'cache', type: 'cache'}
-          ).should.equal(true);
-          db.relations.has_relation_for_endpoint(
-              {service: 'mysql', name: 'la', type: 'thing'}
-          ).should.equal(true);
-          db.relations.has_relation_for_endpoint(
-              {service: 'squid', name: 'cache', type: 'http'}
-          ).should.equal(false);
-
-          // We can also pass a service name which must match for the
-          // same relation.
-
-          db.relations.has_relation_for_endpoint(
-              {service: 'squid', name: 'cache', type: 'cache'},
-              'kafka'
-          ).should.equal(false);
-
-          db.relations.has_relation_for_endpoint(
-              {service: 'squid', name: 'cache', type: 'cache'},
-              'mediawiki'
-          ).should.equal(true);
-
-        });
-
-    it('RelationList.get_relations_for_service should do what it says',
-        function() {
-          var db = new models.Database(),
-             service = new models.Service({id: 'mysql', exposed: false}),
-             rel0 = new models.Relation(
-             { id: 'relation-0',
-               endpoints:
-               [['mediawiki', {name: 'cache', role: 'source'}],
-                 ['squid', {name: 'cache', role: 'front'}]],
-               'interface': 'cache'
-             }),
-             rel1 = new models.Relation(
-             { id: 'relation-1',
-               endpoints:
-               [['wordpress', {role: 'peer', name: 'loadbalancer'}]],
-               'interface': 'reversenginx'
-             }),
-             rel2 = new models.Relation(
-             { id: 'relation-2',
-               endpoints:
-               [['mysql', {name: 'db', role: 'db'}],
-                 ['mediawiki', {name: 'storage', role: 'app'}]],
-               'interface': 'db'
-             }),
-             rel3 = new models.Relation(
-             { id: 'relation-3',
-               endpoints:
-               [['mysql', {role: 'peer', name: 'loadbalancer'}]],
-               'interface': 'mysql-loadbalancer'
-             }),
-             rel4 = new models.Relation(
-             { id: 'relation-4',
-               endpoints:
-               [['something', {name: 'foo', role: 'bar'}],
-                 ['mysql', {name: 'la', role: 'lee'}]],
-               'interface': 'thing'
-             });
-          db.relations.add([rel0, rel1, rel2, rel3, rel4]);
-          Y.Array.map(
-              db.relations.get_relations_for_service(service),
-              function(r) { return r.get('id'); })
-                .should.eql(['relation-2', 'relation-3', 'relation-4']);
-        });
-  });
-});
+describe('charm normalization', function() {
+  var models;
+
+  before(function() {
+    YUI(GlobalConfig).use('juju-models', 'juju-charm-models', function(Y) {
+      models = Y.namespace('juju.models');
+    });
+  });
+
+  it('must create derived attributes from official charm id', function() {
+    var charm = new models.Charm(
+        {id: 'cs:precise/openstack-dashboard-0'});
+    charm.get('scheme').should.equal('cs');
+    var _ = expect(charm.get('owner')).to.not.exist;
+    charm.get('full_name').should.equal('precise/openstack-dashboard');
+    charm.get('charm_store_path').should.equal(
+        'charms/precise/openstack-dashboard-0/json');
+  });
+
+  it('must convert timestamps into time objects', function() {
+    var time = 1349797266.032,
+        date = new Date(time),
+        charm = new models.Charm(
+        { id: 'cs:precise/foo-9', last_change: {created: time / 1000} });
+    charm.get('last_change').created.should.eql(date);
+  });
+
+});
+
+describe('juju models', function() {
+  var models;
+
+  before(function(done) {
+    YUI(GlobalConfig).use('juju-models', 'juju-charm-models', function(Y) {
+      models = Y.namespace('juju.models');
+      done();
+    });
+  });
+
+  it('must be able to create charm', function() {
+    var charm = new models.Charm(
+        {id: 'cs:~alt-bac/precise/openstack-dashboard-0'});
+    charm.get('scheme').should.equal('cs');
+    charm.get('owner').should.equal('alt-bac');
+    charm.get('series').should.equal('precise');
+    charm.get('package_name').should.equal('openstack-dashboard');
+    charm.get('revision').should.equal(0);
+    charm.get('full_name').should.equal(
+        '~alt-bac/precise/openstack-dashboard');
+    charm.get('charm_store_path').should.equal(
+        '~alt-bac/precise/openstack-dashboard-0/json');
+  });
+
+  it('must be able to parse real-world charm names', function() {
+    var charm = new models.Charm({id: 'cs:precise/openstack-dashboard-0'});
+    charm.get('full_name').should.equal('precise/openstack-dashboard');
+    charm.get('package_name').should.equal('openstack-dashboard');
+    charm.get('charm_store_path').should.equal(
+        'charms/precise/openstack-dashboard-0/json');
+    charm.get('scheme').should.equal('cs');
+    var _ = expect(charm.get('owner')).to.not.exist;
+    charm.get('series').should.equal('precise');
+    charm.get('package_name').should.equal('openstack-dashboard');
+    charm.get('revision').should.equal(0);
+  });
+
+  it('must be able to parse individually owned charms', function() {
+    // Note that an earlier version of the parsing code did not handle
+    // hyphens in user names, so this test intentionally includes one.
+    var charm = new models.Charm(
+        {id: 'cs:~marco-ceppi/precise/wordpress-17'});
+    charm.get('full_name').should.equal('~marco-ceppi/precise/wordpress');
+    charm.get('package_name').should.equal('wordpress');
+    charm.get('charm_store_path').should.equal(
+        '~marco-ceppi/precise/wordpress-17/json');
+    charm.get('revision').should.equal(17);
+  });
+
+  it('must reject bad charm ids.', function() {
+    try {
+      var charm = new models.Charm({id: 'foobar'});
+      assert.fail('Should have thrown an error');
+    } catch (e) {
+      e.should.equal(
+          'Developers must initialize charms with a well-formed id.');
+    }
+  });
+
+  it('must reject missing charm ids at initialization.', function() {
+    try {
+      var charm = new models.Charm();
+      assert.fail('Should have thrown an error');
+    } catch (e) {
+      e.should.equal(
+          'Developers must initialize charms with a well-formed id.');
+    }
+  });
+
+  it('must be able to create charm list', function() {
+    var c1 = new models.Charm(
+        { id: 'cs:precise/mysql-2',
+          description: 'A DB'}),
+        c2 = new models.Charm(
+        { id: 'cs:precise/logger-3',
+          description: 'Log sub'}),
+        clist = new models.CharmList();
+    clist.add([c1, c2]);
+    var names = clist.map(function(c) {return c.get('package_name');});
+    names[0].should.equal('mysql');
+    names[1].should.equal('logger');
+  });
+
+
+  it('service unit list should be able to get units of a given service',
+      function() {
+        var sl = new models.ServiceList();
+        var sul = new models.ServiceUnitList();
+        var mysql = new models.Service({id: 'mysql'});
+        var wordpress = new models.Service({id: 'wordpress'});
+        sl.add([mysql, wordpress]);
+        sl.getById('mysql').should.equal(mysql);
+        sl.getById('wordpress').should.equal(wordpress);
+
+        sul.add([{id: 'mysql/0'}, {id: 'mysql/1'}]);
+
+        var wp0 = {id: 'wordpress/0'};
+        var wp1 = {id: 'wordpress/1'};
+        sul.add([wp0, wp1]);
+        wp0.service.should.equal('wordpress');
+
+        sul.get_units_for_service(mysql, true).getAttrs(['id']).id.should.eql(
+           ['mysql/0', 'mysql/1']);
+        sul.get_units_for_service(wordpress, true).getAttrs(
+           ['id']).id.should.eql(['wordpress/0', 'wordpress/1']);
+      });
+
+  it('service unit list should be able to aggregate unit statuses',
+      function() {
+        var sl = new models.ServiceList();
+        var sul = new models.ServiceUnitList();
+        var mysql = new models.Service({id: 'mysql'});
+        var wordpress = new models.Service({id: 'wordpress'});
+        sl.add([mysql, wordpress]);
+
+        var my0 = new models.ServiceUnit({
+          id: 'mysql/0',
+          agent_state: 'pending'});
+        var my1 = new models.ServiceUnit({
+          id: 'mysql/1',
+          agent_state: 'pending'});
+
+        sul.add([my0, my1]);
+
+        var wp0 = new models.ServiceUnit({
+          id: 'wordpress/0',
+          agent_state: 'pending'});
+        var wp1 = new models.ServiceUnit({
+          id: 'wordpress/1',
+          agent_state: 'error'});
+        sul.add([wp0, wp1]);
+
+        sul.get_informative_states_for_service(mysql).should.eql(
+            {'pending': 2});
+        sul.get_informative_states_for_service(wordpress).should.eql(
+            {'pending': 1, 'error': 1});
+      });
+
+  it('service unit objects should parse the service name from unit id',
+      function() {
+        var service_unit = {id: 'mysql/0'};
+        var db = new models.Database();
+        db.units.add(service_unit);
+        service_unit.service.should.equal('mysql');
+      });
+
+  it('service unit objects should report their number correctly',
+      function() {
+        var service_unit = {id: 'mysql/5'};
+        var db = new models.Database();
+        db.units.add(service_unit);
+        service_unit.number.should.equal(5);
+      });
+
+  it('must be able to resolve models by modelId', function() {
+    var db = new models.Database();
+
+    db.services.add([{id: 'wordpress'}, {id: 'mediawiki'}]);
+    db.units.add([{id: 'wordpress/0'}, {id: 'wordpress/1'}]);
+
+    var model = db.services.item(0);
+    // Single parameter calling
+    db.getModelById([model.name, model.get('id')])
+              .get('id').should.equal('wordpress');
+    // Two parameter interface
+    db.getModelById(model.name, model.get('id'))
+              .get('id').should.equal('wordpress');
+
+    var unit = db.units.item(0);
+    db.getModelById([unit.name, unit.id]).id.should.equal('wordpress/0');
+    db.getModelById(unit.name, unit.id).id.should.equal('wordpress/0');
+  });
+
+  it('on_delta should handle remove changes correctly',
+      function() {
+        var db = new models.Database();
+        var my0 = new models.ServiceUnit({id: 'mysql/0',
+          agent_state: 'pending'});
+        var my1 = new models.ServiceUnit({id: 'mysql/1',
+          agent_state: 'pending'});
+        db.units.add([my0, my1]);
+        db.on_delta({data: {result: [
+          ['unit', 'remove', 'mysql/1']
+        ]}});
+        var names = db.units.get('id');
+        names.length.should.equal(1);
+        names[0].should.equal('mysql/0');
+      });
+
+  it('on_delta should be able to reuse existing services with add',
+      function() {
+        var db = new models.Database();
+        var my0 = new models.Service({id: 'mysql', exposed: true});
+        db.services.add([my0]);
+        // Note that exposed is not set explicitly to false.
+        db.on_delta({data: {result: [
+          ['service', 'add', {id: 'mysql'}]
+        ]}});
+        my0.get('exposed').should.equal(false);
+      });
+
+  it('on_delta should be able to reuse existing units with add',
+      // Units are special because they use the LazyModelList.
+      function() {
+        var db = new models.Database();
+        var my0 = {id: 'mysql/0', agent_state: 'pending'};
+        db.units.add([my0]);
+        db.on_delta({data: {result: [
+          ['unit', 'add', {id: 'mysql/0', agent_state: 'another'}]
+        ]}});
+        my0.agent_state.should.equal('another');
+      });
+
+  it('on_delta should reset relation_errors',
+      function() {
+        var db = new models.Database();
+        var my0 = {id: 'mysql/0', relation_errors: {'cache': ['memcached']}};
+        db.units.add([my0]);
+        // Note that relation_errors is not set.
+        db.on_delta({data: {result: [
+          ['unit', 'change', {id: 'mysql/0'}]
+        ]}});
+        my0.relation_errors.should.eql({});
+      });
+
+  it('ServiceUnitList should accept a list of units at instantiation and ' +
+      'decorate them', function() {
+        var mysql = new models.Service({id: 'mysql'});
+        var objs = [{id: 'mysql/0'},
+                    {id: 'mysql/1'}];
+        var sul = new models.ServiceUnitList({items: objs});
+        var unit_data = sul.get_units_for_service(
+                mysql, true).getAttrs(['service', 'number']);
+        unit_data.service.should.eql(['mysql', 'mysql']);
+        unit_data.number.should.eql([0, 1]);
+      });
+
+  it('RelationList.has_relations.. should return true if rel found.',
+      function() {
+        var db = new models.Database(),
+            service = new models.Service({id: 'mysql', exposed: false}),
+            rel0 = new models.Relation({
+              id: 'relation-0',
+              endpoints: [
+                ['mediawiki', {name: 'cache', role: 'source'}],
+                ['squid', {name: 'cache', role: 'front'}]],
+              'interface': 'cache'
+            }),
+            rel1 = new models.Relation({
+              id: 'relation-4',
+              endpoints: [
+                ['something', {name: 'foo', role: 'bar'}],
+                ['mysql', {name: 'la', role: 'lee'}]],
+              'interface': 'thing'
+            });
+        db.relations.add([rel0, rel1]);
+        db.relations.has_relation_for_endpoint(
+            {service: 'squid', name: 'cache', type: 'cache'}
+        ).should.equal(true);
+        db.relations.has_relation_for_endpoint(
+            {service: 'mysql', name: 'la', type: 'thing'}
+        ).should.equal(true);
+        db.relations.has_relation_for_endpoint(
+            {service: 'squid', name: 'cache', type: 'http'}
+        ).should.equal(false);
+
+        // We can also pass a service name which must match for the
+        // same relation.
+
+        db.relations.has_relation_for_endpoint(
+            {service: 'squid', name: 'cache', type: 'cache'},
+            'kafka'
+        ).should.equal(false);
+
+        db.relations.has_relation_for_endpoint(
+            {service: 'squid', name: 'cache', type: 'cache'},
+            'mediawiki'
+        ).should.equal(true);
+
+      });
+
+  it('RelationList.get_relations_for_service should do what it says',
+      function() {
+        var db = new models.Database(),
+            service = new models.Service({id: 'mysql', exposed: false}),
+            rel0 = new models.Relation(
+            { id: 'relation-0',
+              endpoints:
+         [['mediawiki', {name: 'cache', role: 'source'}],
+          ['squid', {name: 'cache', role: 'front'}]],
+              'interface': 'cache'
+            }),
+            rel1 = new models.Relation(
+            { id: 'relation-1',
+              endpoints:
+         [['wordpress', {role: 'peer', name: 'loadbalancer'}]],
+              'interface': 'reversenginx'
+            }),
+            rel2 = new models.Relation(
+            { id: 'relation-2',
+              endpoints:
+         [['mysql', {name: 'db', role: 'db'}],
+          ['mediawiki', {name: 'storage', role: 'app'}]],
+              'interface': 'db'
+            }),
+            rel3 = new models.Relation(
+            { id: 'relation-3',
+              endpoints:
+         [['mysql', {role: 'peer', name: 'loadbalancer'}]],
+              'interface': 'mysql-loadbalancer'
+            }),
+            rel4 = new models.Relation(
+            { id: 'relation-4',
+              endpoints:
+         [['something', {name: 'foo', role: 'bar'}],
+          ['mysql', {name: 'la', role: 'lee'}]],
+              'interface': 'thing'
+            });
+        db.relations.add([rel0, rel1, rel2, rel3, rel4]);
+        db.relations.get_relations_for_service(service).map(
+       function(r) { return r.get('id'); })
+            .should.eql(['relation-2', 'relation-3', 'relation-4']);
+      });
+});
+
 
 YUI(GlobalConfig).use(['juju-models', 'juju-gui', 'datasource-local',
   'juju-tests-utils', 'json-stringify',