Merge lp:~tveronezi/juju-gui/change-requires-param into lp:juju-gui/experimental

Proposed by Thiago Veronezi
Status: Merged
Merged at revision: 256
Proposed branch: lp:~tveronezi/juju-gui/change-requires-param
Merge into: lp:juju-gui/experimental
Diff against target: 1823 lines (+808/-847)
10 files modified
app/app.js (+1/-0)
app/models/charm.js (+2/-1)
app/models/models.js (+1/-0)
app/modules-debug.js (+8/-12)
app/store/charm.js (+1/-0)
test/test_app.js (+176/-191)
test/test_app_hotkeys.js (+52/-52)
test/test_model.js (+18/-25)
test/test_notifications.js (+454/-469)
test/test_notifier_widget.js (+95/-97)
To merge this branch: bzr merge lp:~tveronezi/juju-gui/change-requires-param
Reviewer Review Type Date Requested Status
Juju GUI Hackers Pending
Review via email: mp+135198@code.launchpad.net

Description of the change

Remove "requires" from modules-debug.js

The minification process does not read the "requires" parameters defined in "modules-debug.js". This parameter should be defined in each custom yui object that uses some kind of internal or external requirement. The sole use of "all-app-debug.js" is to define the fullpath of the file that defines a given module.

This patch removes all the existing "requires" properties from "modules-debug.js" and add then where they are needed.

https://codereview.appspot.com/6856070/

To post a comment you must log in.
Revision history for this message
Thiago Veronezi (tveronezi) wrote :
Download full text (4.1 KiB)

Reviewers: mp+135198_code.launchpad.net,

Message:
Please take a look.

Description:
Remove "requires" from modules-debug.js

The minification process does not read the "requires" parameters defined
in "modules-debug.js". This parameter should be defined in each custom
yui object that uses some kind of internal or external requirement. The
sole use of "all-app-debug.js" is to define the fullpath of the file
that defines a given module.

This patch removes all the existing "requires" properties from
"modules-debug.js" and add then where they are needed.

https://code.launchpad.net/~tveronezi/juju-gui/change-requires-param/+merge/135198

(do not edit description out of merge proposal)

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

Affected files:
   A [revision details]
   M app/app.js
   M app/models/charm.js
   M app/models/models.js
   M app/modules-debug.js
   M app/store/charm.js

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision: <email address hidden>
+New revision: <email address hidden>

Index: app/app.js
=== modified file 'app/app.js'
--- app/app.js 2012-11-20 14:51:46 +0000
+++ app/app.js 2012-11-20 16:55:43 +0000
@@ -746,6 +746,7 @@
      'juju-models',
      'juju-views',
      'juju-controllers',
+ 'juju-view-charm-search',
      'io',
      'json-parse',
      'app-base',

Index: app/modules-debug.js
=== modified file 'app/modules-debug.js'
--- app/modules-debug.js 2012-11-15 15:44:00 +0000
+++ app/modules-debug.js 2012-11-20 16:55:43 +0000
@@ -1,6 +1,8 @@
  // This file is used for development only. In order to use it you should
call
  // the "make debug" command. This command passes the "debug" argument to
the
-// "lib/server.js".
+// "lib/server.js". The sole use of this file is to define the "aliases"
("use"
+// property) and the "fullpath" of the file that implement a given module.
The
+// "requires" property should not be used here.
  var GlobalConfig = {
    filter: 'debug',
    // Set "true" for verbose logging of YUI
@@ -93,19 +95,15 @@
          },

          'juju-charm-models': {
- requires: ['juju-charm-id'],
            fullpath: '/juju-ui/models/charm.js'
          },

          'juju-models': {
- requires: [
- 'model', 'model-list', 'juju-endpoints', 'juju-charm-models'],
            fullpath: '/juju-ui/models/models.js'
          },

          // Connectivity
          'juju-env': {
- requires: ['reconnecting-websocket'],
            fullpath: '/juju-ui/store/env.js'
          },

@@ -114,7 +112,6 @@
          },

          'juju-charm-store': {
- requires: ['juju-charm-id'],
            fullpath: '/juju-ui/store/charm.js'
          },

@@ -125,13 +122,7 @@

          // App
          'juju-gui': {
- fullpath: '/juju-ui/app.js',
- requires: [
- 'juju-controllers',
- 'juju-views',
- 'juju-models',
- 'juju-view-charm-search'
- ]
+ fullpath: '/juj...

Read more...

256. By Thiago Veronezi

fix tests

257. By Thiago Veronezi

make lint happy

Revision history for this message
Thiago Veronezi (tveronezi) wrote :
Revision history for this message
Gary Poster (gary) wrote :
Download full text (3.1 KiB)

Hi Thiago. Thank you for the quick turnaround on this fix. I have only
done a code review of this so far, and not a functional test/review.
However, I have some concerns with the way you changed the tests, and
I'd like to see those resolved before I approve.

Gary

https://codereview.appspot.com/6856070/diff/3001/app/modules-debug.js
File app/modules-debug.js (left):

https://codereview.appspot.com/6856070/diff/3001/app/modules-debug.js#oldcode96
app/modules-debug.js:96: requires: ['juju-charm-id'],
I don't really understand why these were here instead of in the modules
to begin with. That makes me nervous, because often people have a good
reason for this sort of thing. Do you know the answer?

https://codereview.appspot.com/6856070/diff/3001/app/modules-debug.js
File app/modules-debug.js (right):

https://codereview.appspot.com/6856070/diff/3001/app/modules-debug.js#newcode4
app/modules-debug.js:4: // property) and the "fullpath" of the file that
implement a given module. The
Might as well take the opportunity to be clearer about what the "use"
property does. ("""This file declares which files implement modules,
using the "fullpath" property; and declares the membership of rollup
modules, using the "use" property to specify what the module name
aliases.""" or something like that.)

https://codereview.appspot.com/6856070/diff/3001/app/modules-debug.js#newcode5
app/modules-debug.js:5: // "requires" property should not be used here.
Please explain why ("...should not be used here because the javascript
minimizer will not parse it" or something like that).

https://codereview.appspot.com/6856070/diff/3001/app/modules-debug.js#newcode16
app/modules-debug.js:16: modules: {
I really am tempted to suggest that the production files should parse
this and rewrite it after removing the "fullpath" modules, instead of
making developers maintain two versions of this "modules" file. Even if
that's a good idea, that's a separate bug/branch. I'm just thinking.

https://codereview.appspot.com/6856070/diff/3001/test/index.html
File test/index.html (right):

https://codereview.appspot.com/6856070/diff/3001/test/index.html#newcode40
test/index.html:40: YUI().use('node', 'event', function(runnerY) {
In terms of scoping rules, I'm pretty sure that this could be "Y" and
the one used by tests, below, could be "Y" also. Did you not do it that
way because you felt it was more readable this way?

https://codereview.appspot.com/6856070/diff/3001/test/index.html#newcode61
test/index.html:61: window.Y = testsYUI;
We usually create our own Y in the tests. Why do we have to do this?
Wouldn't it be better to make our own Y in each test?

https://codereview.appspot.com/6856070/diff/3001/test/test_app.js
File test/test_app.js (left):

https://codereview.appspot.com/6856070/diff/3001/test/test_app.js#oldcode184
test/test_app.js:184: 'json-stringify',
This was specifying different requirements than you have put in the
global Y. We really should not share the Y object. We should keep our
dependencies tight to our tests in order to increase the maintainability
and understandability of our code. This comment addresses all changes
like this one.

https://codereview.apps...

Read more...

Revision history for this message
Madison Scott-Clary (makyo) wrote :

 From IRC, this branch looks good functionally. +1 from me, so long as
Gary's comments are addressed to both of your satisfaction.

https://codereview.appspot.com/6856070/

258. By Thiago Veronezi

code review

Revision history for this message
Thiago Veronezi (tveronezi) wrote :
Download full text (3.6 KiB)

Thanks for your review, guys!

https://codereview.appspot.com/6856070/diff/3001/app/modules-debug.js
File app/modules-debug.js (left):

https://codereview.appspot.com/6856070/diff/3001/app/modules-debug.js#oldcode96
app/modules-debug.js:96: requires: ['juju-charm-id'],
On 2012/11/20 21:01:19, gary.poster wrote:
> I don't really understand why these were here instead of in the
modules to begin
> with. That makes me nervous, because often people have a good reason
for this
> sort of thing. Do you know the answer?

It is not their fault. The problem is that YUI has two places to define
the very same thing... which is not bad when we are loading non yui js
files that depend on other js files.

https://codereview.appspot.com/6856070/diff/3001/app/modules-debug.js
File app/modules-debug.js (right):

https://codereview.appspot.com/6856070/diff/3001/app/modules-debug.js#newcode4
app/modules-debug.js:4: // property) and the "fullpath" of the file that
implement a given module. The
On 2012/11/20 21:01:19, gary.poster wrote:
> Might as well take the opportunity to be clearer about what the "use"
property
> does. ("""This file declares which files implement modules, using the
> "fullpath" property; and declares the membership of rollup modules,
using the
> "use" property to specify what the module name aliases.""" or
something like
> that.)

Done.

https://codereview.appspot.com/6856070/diff/3001/app/modules-debug.js#newcode5
app/modules-debug.js:5: // "requires" property should not be used here.
On 2012/11/20 21:01:19, gary.poster wrote:
> Please explain why ("...should not be used here because the
javascript
> minimizer will not parse it" or something like that).

Done.

https://codereview.appspot.com/6856070/diff/3001/app/modules-debug.js#newcode16
app/modules-debug.js:16: modules: {
On 2012/11/20 21:01:19, gary.poster wrote:
> I really am tempted to suggest that the production files should parse
this and
> rewrite it after removing the "fullpath" modules, instead of making
developers
> maintain two versions of this "modules" file. Even if that's a good
idea,
> that's a separate bug/branch. I'm just thinking.

I am tempted to simply remove the aliases. They are one of the reasons
we cannot completely turn the loader off, remember?

https://codereview.appspot.com/6856070/diff/3001/test/index.html
File test/index.html (right):

https://codereview.appspot.com/6856070/diff/3001/test/index.html#newcode40
test/index.html:40: YUI().use('node', 'event', function(runnerY) {
On 2012/11/20 21:01:19, gary.poster wrote:
> In terms of scoping rules, I'm pretty sure that this could be "Y" and
the one
> used by tests, below, could be "Y" also. Did you not do it that way
because you
> felt it was more readable this way?

Reverting...

https://codereview.appspot.com/6856070/diff/3001/test/index.html#newcode61
test/index.html:61: window.Y = testsYUI;
On 2012/11/20 21:01:19, gary.poster wrote:
> We usually create our own Y in the tests. Why do we have to do this?
Wouldn't
> it be better to make our own Y in each test?

Reverting...

https://codereview.appspot.com/6856070/diff/3001/test/test_app.js
File test/test_app.js (left):

https://codereview.appspot.com/68...

Read more...

259. By Thiago Veronezi

code review

260. By Thiago Veronezi

code review

Revision history for this message
Thiago Veronezi (tveronezi) wrote :
Revision history for this message
Gary Poster (gary) wrote :

Hi Thiago. I like what you did with the tests, even though I wish it
didn't have to be part of this branch. I'm OK with it landing though.

As I mentioned on IRC, I do see a test failure, but it seems shallow.
Please fix that, and maybe the small comment change I suggest.

Thank you,

Gary

https://codereview.appspot.com/6856070/diff/9001/app/modules-debug.js
File app/modules-debug.js (right):

https://codereview.appspot.com/6856070/diff/9001/app/modules-debug.js#newcode10
app/modules-debug.js:10: // minimizer will not parse it.
Maybe add "Please put in in the module itself, instead."

https://codereview.appspot.com/6856070/diff/9001/test/test_app.js
File test/test_app.js (right):

https://codereview.appspot.com/6856070/diff/9001/test/test_app.js#newcode34
test/test_app.js:34: YUI(GlobalConfig).use(['juju-gui',
'juju-tests-utils'], function(Y) {
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.

https://codereview.appspot.com/6856070/

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'app/app.js'
2--- app/app.js 2012-11-20 14:51:46 +0000
3+++ app/app.js 2012-11-20 23:07:20 +0000
4@@ -746,6 +746,7 @@
5 'juju-models',
6 'juju-views',
7 'juju-controllers',
8+ 'juju-view-charm-search',
9 'io',
10 'json-parse',
11 'app-base',
12
13=== modified file 'app/models/charm.js'
14--- app/models/charm.js 2012-10-29 11:20:31 +0000
15+++ app/models/charm.js 2012-11-20 23:07:20 +0000
16@@ -226,6 +226,7 @@
17 }, '0.1.0', {
18 requires: [
19 'model',
20- 'model-list'
21+ 'model-list',
22+ 'juju-charm-id'
23 ]
24 });
25
26=== modified file 'app/models/models.js'
27--- app/models/models.js 2012-11-16 08:25:02 +0000
28+++ app/models/models.js 2012-11-20 23:07:20 +0000
29@@ -487,6 +487,7 @@
30 'datasource-jsonschema',
31 'io-base',
32 'json-parse',
33+ 'juju-endpoints',
34 'juju-view-utils',
35 'juju-charm-models'
36 ]
37
38=== modified file 'app/modules-debug.js'
39--- app/modules-debug.js 2012-11-15 15:44:00 +0000
40+++ app/modules-debug.js 2012-11-20 23:07:20 +0000
41@@ -1,6 +1,13 @@
42 // This file is used for development only. In order to use it you should call
43 // the "make debug" command. This command passes the "debug" argument to the
44 // "lib/server.js".
45+//
46+// This file declares which files implement modules, using the
47+// "fullpath" property; and declares the membership of rollup modules, using
48+// the "use" property to specify what the module name aliases.
49+//
50+// The "requires" property should not be used here because the javascript
51+// minimizer will not parse it.
52 var GlobalConfig = {
53 filter: 'debug',
54 // Set "true" for verbose logging of YUI
55@@ -93,19 +100,15 @@
56 },
57
58 'juju-charm-models': {
59- requires: ['juju-charm-id'],
60 fullpath: '/juju-ui/models/charm.js'
61 },
62
63 'juju-models': {
64- requires: [
65- 'model', 'model-list', 'juju-endpoints', 'juju-charm-models'],
66 fullpath: '/juju-ui/models/models.js'
67 },
68
69 // Connectivity
70 'juju-env': {
71- requires: ['reconnecting-websocket'],
72 fullpath: '/juju-ui/store/env.js'
73 },
74
75@@ -114,7 +117,6 @@
76 },
77
78 'juju-charm-store': {
79- requires: ['juju-charm-id'],
80 fullpath: '/juju-ui/store/charm.js'
81 },
82
83@@ -125,13 +127,7 @@
84
85 // App
86 'juju-gui': {
87- fullpath: '/juju-ui/app.js',
88- requires: [
89- 'juju-controllers',
90- 'juju-views',
91- 'juju-models',
92- 'juju-view-charm-search'
93- ]
94+ fullpath: '/juju-ui/app.js'
95 }
96 }
97 }
98
99=== modified file 'app/store/charm.js'
100--- app/store/charm.js 2012-11-05 21:24:42 +0000
101+++ app/store/charm.js 2012-11-20 23:07:20 +0000
102@@ -114,6 +114,7 @@
103
104 }, '0.1.0', {
105 requires: [
106+ 'juju-charm-id',
107 'datasource-io',
108 'json-parse'
109 ]
110
111=== modified file 'test/test_app.js'
112--- test/test_app.js 2012-11-14 14:34:31 +0000
113+++ test/test_app.js 2012-11-20 23:07:20 +0000
114@@ -31,196 +31,181 @@
115 return app;
116 }
117
118-describe('Application basics', function() {
119- var Y, app, container;
120-
121- before(function(done) {
122- Y = YUI(GlobalConfig).use(['juju-gui', 'juju-tests-utils'], function(Y) {
123- done();
124- });
125- });
126-
127- beforeEach(function(done) {
128- // XXX Apparently removing a DOM node is asynchronous (on Chrome at least)
129- // and we occasionally lose the race if this code is in the afterEach
130- // function, so instead we do it here, but only if one has previously been
131- // created.
132- if (container) {
133- container.remove(true);
134- }
135- container = Y.one('#main')
136- .appendChild(Y.Node.create('<div/>'))
137- .set('id', 'test-container')
138- .addClass('container')
139- .append(Y.Node.create('<span/>')
140- .set('id', 'environment-name'))
141- .append(Y.Node.create('<span/>')
142- .set('id', 'provider-type'));
143- app = new Y.juju.App(
144- { container: container,
145- viewContainer: container});
146- injectData(app);
147- done();
148- });
149-
150- it('should produce a valid index', function() {
151- var container = app.get('container');
152- app.render();
153- container.getAttribute('id').should.equal('test-container');
154- container.getAttribute('class').should.include('container');
155- });
156-
157- it('should be able to route objects to internal URLs', function() {
158- // take handles to database objects and ensure we can route to the view
159- // needed to show them
160- var wordpress = app.db.services.getById('wordpress'),
161- wp0 = app.db.units.get_units_for_service(wordpress)[0],
162- wp_charm = app.db.charms.add({id: wordpress.get('charm')});
163-
164- // 'service/wordpress/' is the primary and so other URL are not returned
165- app.getModelURL(wordpress).should.equal('/service/wordpress/');
166- // however passing 'intent' can force selection of another
167- app.getModelURL(wordpress, 'config').should.equal(
168- '/service/wordpress/config');
169-
170- // service units use argument rewriting (thus not /u/wp/0)
171- app.getModelURL(wp0).should.equal('/unit/wordpress-0/');
172-
173- // charms also require a mapping but only a name, not a function
174- app.getModelURL(wp_charm).should.equal(
175- '/charms/charms/precise/wordpress-6/json');
176- });
177-
178- it('should display the configured environment name', function() {
179- var environment_name = 'This is the environment name. Deal with it.';
180- app = new Y.juju.App(
181- { container: container,
182- viewContainer: container,
183- environment_name: environment_name});
184- assert.equal(
185- container.one('#environment-name').get('text'),
186- environment_name);
187- });
188-
189- it('should show a generic environment name if none configured', function() {
190- app = new Y.juju.App(
191- { container: container,
192- viewContainer: container});
193- assert.equal(
194- container.one('#environment-name').get('text'),
195- 'Environment');
196- });
197-
198- it('should show the provider type, when available', function() {
199- var providerType = 'excellent provider';
200- // Since no provider type has been set yet, none is displayed.
201- assert.equal(
202- container.one('#provider-type').get('text'),
203- '');
204- app.env.set('providerType', providerType);
205- // The provider type has been displayed.
206- assert.equal(
207- container.one('#provider-type').get('text'),
208- 'on ' + providerType);
209- });
210-
211-});
212-
213-
214-describe('Application Connection State', function() {
215- var Y, container;
216-
217- before(function(done) {
218- Y = YUI(GlobalConfig).use(['juju-gui', 'juju-tests-utils'], function(Y) {
219- container = Y.Node.create('<div id="test" class="container"></div>');
220- done();
221- });
222-
223- });
224-
225- it('should be able to handle env connection status changes', function() {
226- var juju = Y.namespace('juju'),
227- conn = new(Y.namespace('juju-tests.utils')).SocketStub(),
228- env = new juju.Environment({conn: conn}),
229- app = new Y.juju.App({env: env, container: container}),
230- reset_called = false,
231- noop = function() {return this;};
232-
233-
234- // mock the db
235- app.db = {
236- // mock out notifications
237- // so app can start normally
238- notifications: {
239- addTarget: noop,
240- after: noop,
241- filter: noop,
242- map: noop,
243- on: noop,
244- size: function() {return 0;}
245- },
246- reset: function() {
247- reset_called = true;
248- }
249- };
250- env.connect();
251- conn.open();
252- reset_called.should.equal(true);
253-
254- // trigger a second time and verify
255- reset_called = false;
256- conn.open();
257- reset_called.should.equal(true);
258- });
259-
260-});
261-
262-describe('Application prefetching', function() {
263- var Y, models, conn, env, app, container, charm_store, data, juju;
264-
265- before(function(done) {
266- Y = YUI(GlobalConfig).use(
267- 'juju-models', 'juju-gui', 'datasource-local', 'juju-tests-utils',
268- 'json-stringify',
269- function(Y) {
270- models = Y.namespace('juju.models');
271- done();
272- });
273- });
274-
275- beforeEach(function() {
276- conn = new (Y.namespace('juju-tests.utils')).SocketStub(),
277- env = new Y.juju.Environment({conn: conn});
278- env.connect();
279- conn.open();
280- container = Y.Node.create('<div id="test" class="container"></div>');
281- data = [];
282- app = new Y.juju.App(
283- { container: container,
284- viewContainer: container,
285- env: env,
286- charm_store: {} });
287-
288- app.updateEndpoints = function() {};
289- env.get_endpoints = function() {};
290- });
291-
292- afterEach(function() {
293- container.destroy();
294- app.destroy();
295- });
296-
297- it('must prefetch charm and service for service pages', function() {
298- injectData(app);
299- var _ = expect(
300- app.db.charms.getById('cs:precise/wordpress-6')).to.not.exist;
301- app.show_service({params: {id: 'wordpress'}, query: {}});
302- // The app made a request of juju for the service info.
303- conn.messages[conn.messages.length - 2].op.should.equal('get_service');
304- // The app also requested juju (not the charm store--see discussion in
305- // app/models/charm.js) for the charm info.
306- conn.last_message().op.should.equal('get_charm');
307- // Tests of the actual load machinery are in the model and env tests, and
308- // so are not repeated here.
309+YUI(GlobalConfig).use(['juju-gui', 'juju-tests-utils'], function(Y) {
310+ describe('Application basics', function() {
311+ var app, container;
312+
313+ beforeEach(function() {
314+ if (container) {
315+ container.remove(true);
316+ }
317+ container = Y.one('#main')
318+ .appendChild(Y.Node.create('<div/>'))
319+ .set('id', 'test-container')
320+ .addClass('container')
321+ .append(Y.Node.create('<span/>')
322+ .set('id', 'environment-name'))
323+ .append(Y.Node.create('<span/>')
324+ .set('id', 'provider-type'));
325+ app = new Y.juju.App(
326+ { container: container,
327+ viewContainer: container});
328+ injectData(app);
329+ });
330+
331+ it('should produce a valid index', function() {
332+ var container = app.get('container');
333+ app.render();
334+ container.getAttribute('id').should.equal('test-container');
335+ container.getAttribute('class').should.include('container');
336+ });
337+
338+ it('should be able to route objects to internal URLs', function() {
339+ // take handles to database objects and ensure we can route to the view
340+ // needed to show them
341+ var wordpress = app.db.services.getById('wordpress'),
342+ wp0 = app.db.units.get_units_for_service(wordpress)[0],
343+ wp_charm = app.db.charms.add({id: wordpress.get('charm')});
344+
345+ // 'service/wordpress/' is the primary and so other URL are not returned
346+ app.getModelURL(wordpress).should.equal('/service/wordpress/');
347+ // however passing 'intent' can force selection of another
348+ app.getModelURL(wordpress, 'config').should.equal(
349+ '/service/wordpress/config');
350+
351+ // service units use argument rewriting (thus not /u/wp/0)
352+ app.getModelURL(wp0).should.equal('/unit/wordpress-0/');
353+
354+ // charms also require a mapping but only a name, not a function
355+ app.getModelURL(wp_charm).should.equal(
356+ '/charms/charms/precise/wordpress-6/json');
357+ });
358+
359+ it('should display the configured environment name', function() {
360+ var environment_name = 'This is the environment name. Deal with it.';
361+ app = new Y.juju.App(
362+ { container: container,
363+ viewContainer: container,
364+ environment_name: environment_name});
365+ assert.equal(
366+ container.one('#environment-name').get('text'),
367+ environment_name);
368+ });
369+
370+ it('should show a generic environment name if none configured', function() {
371+ app = new Y.juju.App(
372+ { container: container,
373+ viewContainer: container});
374+ assert.equal(
375+ container.one('#environment-name').get('text'),
376+ 'Environment');
377+ });
378+
379+ it('should show the provider type, when available', function() {
380+ var providerType = 'excellent provider';
381+ // Since no provider type has been set yet, none is displayed.
382+ assert.equal(
383+ container.one('#provider-type').get('text'),
384+ '');
385+ app.env.set('providerType', providerType);
386+ // The provider type has been displayed.
387+ assert.equal(
388+ container.one('#provider-type').get('text'),
389+ 'on ' + providerType);
390+ });
391+
392+ });
393+});
394+
395+YUI(GlobalConfig).use(['juju-gui', 'juju-tests-utils'], function(Y) {
396+ describe('Application Connection State', function() {
397+ var container;
398+
399+ before(function() {
400+ container = Y.Node.create('<div id="test" class="container"></div>');
401+ });
402+
403+ it('should be able to handle env connection status changes', function() {
404+ var juju = Y.namespace('juju'),
405+ conn = new(Y.namespace('juju-tests.utils')).SocketStub(),
406+ env = new juju.Environment({conn: conn}),
407+ app = new Y.juju.App({env: env, container: container}),
408+ reset_called = false,
409+ noop = function() {return this;};
410+
411+
412+ // mock the db
413+ app.db = {
414+ // mock out notifications
415+ // so app can start normally
416+ notifications: {
417+ addTarget: noop,
418+ after: noop,
419+ filter: noop,
420+ map: noop,
421+ on: noop,
422+ size: function() {return 0;}
423+ },
424+ reset: function() {
425+ reset_called = true;
426+ }
427+ };
428+ env.connect();
429+ conn.open();
430+ reset_called.should.equal(true);
431+
432+ // trigger a second time and verify
433+ reset_called = false;
434+ conn.open();
435+ reset_called.should.equal(true);
436+ });
437+
438+ });
439+});
440+
441+YUI(GlobalConfig).use(['juju-models', 'juju-gui', 'datasource-local',
442+ 'juju-tests-utils', 'json-stringify'], function(Y) {
443+ describe('Application prefetching', function() {
444+ var models, conn, env, app, container, charm_store, data, juju;
445+
446+ before(function() {
447+ models = Y.namespace('juju.models');
448+ });
449+
450+ beforeEach(function() {
451+ conn = new (Y.namespace('juju-tests.utils')).SocketStub(),
452+ env = new Y.juju.Environment({conn: conn});
453+ env.connect();
454+ conn.open();
455+ container = Y.Node.create('<div id="test" class="container"></div>');
456+ data = [];
457+ app = new Y.juju.App(
458+ { container: container,
459+ viewContainer: container,
460+ env: env,
461+ charm_store: {} });
462+
463+ app.updateEndpoints = function() {};
464+ env.get_endpoints = function() {};
465+ });
466+
467+ afterEach(function() {
468+ container.destroy();
469+ app.destroy();
470+ });
471+
472+ it('must prefetch charm and service for service pages', function() {
473+ injectData(app);
474+ var _ = expect(
475+ app.db.charms.getById('cs:precise/wordpress-6')).to.not.exist;
476+ app.show_service({params: {id: 'wordpress'}, query: {}});
477+ // The app made a request of juju for the service info.
478+ conn.messages[conn.messages.length - 2].op.should.equal('get_service');
479+ // The app also requested juju (not the charm store--see discussion in
480+ // app/models/charm.js) for the charm info.
481+ conn.last_message().op.should.equal('get_charm');
482+ // Tests of the actual load machinery are in the model and env tests, and
483+ // so are not repeated here.
484+ });
485 });
486 });
487
488=== modified file 'test/test_app_hotkeys.js'
489--- test/test_app_hotkeys.js 2012-11-20 15:39:46 +0000
490+++ test/test_app_hotkeys.js 2012-11-20 23:07:20 +0000
491@@ -1,12 +1,11 @@
492 'use strict';
493
494-describe('application hotkeys', function() {
495- var Y, app, container, env, conn, testUtils, windowNode;
496+YUI(GlobalConfig).use(['juju-gui', 'juju-tests-utils', 'node-event-simulate'],
497+ function(Y) {
498+ describe('application hotkeys', function() {
499+ var app, container, env, conn, testUtils, windowNode;
500
501- before(function() {
502- Y = YUI(GlobalConfig).use(
503- ['juju-gui', 'juju-tests-utils',
504- 'node-event-simulate'], function(Y) {
505+ before(function() {
506 windowNode = Y.one(window);
507 app = new Y.juju.App({
508 env: env,
509@@ -15,49 +14,50 @@
510 });
511 app.activateHotkeys();
512 });
513- });
514-
515- afterEach(function() {
516- container.remove(true);
517- });
518-
519- beforeEach(function() {
520- container = Y.one('#main').appendChild(Y.Node.create('<div/>')).set('id',
521- 'test-container').append(
522- Y.Node.create('<input />').set('id', 'charm-search-field'));
523- testUtils = Y.namespace('juju-tests.utils');
524- env = {
525- get: function() {
526- },
527- on: function() {
528- },
529- after: function() {
530- }
531- };
532- });
533-
534- it('should listen for alt-S events', function() {
535- app.render();
536- windowNode.simulate('keydown', {
537- keyCode: 83, // "S" key
538- altKey: true
539- });
540- // Did charm-search-field get the focus?
541- assert.equal(Y.one('#charm-search-field'), Y.one(document.activeElement));
542- });
543-
544- it('should listen for alt-E events', function() {
545- var altEtriggered = false;
546- app.on('navigateTo', function(param) {
547- if (param && param.url === '/') {
548- altEtriggered = true;
549- }
550- });
551- app.render();
552- windowNode.simulate('keydown', {
553- keyCode: 69, // "E" key
554- altKey: true
555- });
556- assert.isTrue(altEtriggered);
557- });
558-});
559+
560+ afterEach(function() {
561+ container.remove(true);
562+ });
563+
564+ beforeEach(function() {
565+ container = Y.one('#main').appendChild(Y.Node.create(
566+ '<div/>')).set('id', 'test-container').append(
567+ Y.Node.create('<input />').set('id', 'charm-search-field'));
568+ testUtils = Y.namespace('juju-tests.utils');
569+ env = {
570+ get: function() {
571+ },
572+ on: function() {
573+ },
574+ after: function() {
575+ }
576+ };
577+ });
578+
579+ it('should listen for alt-S events', function() {
580+ app.render();
581+ windowNode.simulate('keydown', {
582+ keyCode: 83, // "S" key
583+ altKey: true
584+ });
585+ // Did charm-search-field get the focus?
586+ assert.equal(Y.one('#charm-search-field'),
587+ Y.one(document.activeElement));
588+ });
589+
590+ it('should listen for alt-E events', function() {
591+ var altEtriggered = false;
592+ app.on('navigateTo', function(param) {
593+ if (param && param.url === '/') {
594+ altEtriggered = true;
595+ }
596+ });
597+ app.render();
598+ windowNode.simulate('keydown', {
599+ keyCode: 69, // "E" key
600+ altKey: true
601+ });
602+ assert.isTrue(altEtriggered);
603+ });
604+ });
605+ });
606
607=== modified file 'test/test_model.js'
608--- test/test_model.js 2012-10-23 20:02:17 +0000
609+++ test/test_model.js 2012-11-20 23:07:20 +0000
610@@ -1,15 +1,11 @@
611 'use strict';
612
613-(function() {
614-
615+YUI(GlobalConfig).use('juju-models', function(Y) {
616 describe('charm normalization', function() {
617- var Y, models;
618+ var models;
619
620- before(function(done) {
621- Y = YUI(GlobalConfig).use('juju-models', function(Y) {
622- models = Y.namespace('juju.models');
623- done();
624- });
625+ before(function() {
626+ models = Y.namespace('juju.models');
627 });
628
629 it('must create derived attributes from official charm id', function() {
630@@ -31,15 +27,14 @@
631 });
632
633 });
634+});
635
636+YUI(GlobalConfig).use('juju-models', function(Y) {
637 describe('juju models', function() {
638- var Y, models;
639+ var models;
640
641- before(function(done) {
642- Y = YUI(GlobalConfig).use('juju-models', function(Y) {
643- models = Y.namespace('juju.models');
644- done();
645- });
646+ before(function() {
647+ models = Y.namespace('juju.models');
648 });
649
650 it('must be able to create charm', function() {
651@@ -355,19 +350,17 @@
652 .should.eql(['relation-2', 'relation-3', 'relation-4']);
653 });
654 });
655+});
656
657+YUI(GlobalConfig).use(['juju-models', 'juju-gui', 'datasource-local',
658+ 'juju-tests-utils', 'json-stringify',
659+ 'juju-charm-store'], function(Y) {
660 describe('juju charm load', function() {
661- var Y, models, conn, env, app, container, charm_store, data, juju;
662+ var models, conn, env, app, container, charm_store, data, juju;
663
664- before(function(done) {
665- Y = YUI(GlobalConfig).use(
666- 'juju-models', 'juju-gui', 'datasource-local', 'juju-tests-utils',
667- 'json-stringify', 'juju-charm-store',
668- function(Y) {
669- models = Y.namespace('juju.models');
670- juju = Y.namespace('juju');
671- done();
672- });
673+ before(function() {
674+ models = Y.namespace('juju.models');
675+ juju = Y.namespace('juju');
676 });
677
678 beforeEach(function() {
679@@ -513,4 +506,4 @@
680 });
681
682 });
683-})();
684+});
685
686=== modified file 'test/test_notifications.js'
687--- test/test_notifications.js 2012-11-16 13:38:21 +0000
688+++ test/test_notifications.js 2012-11-20 23:07:20 +0000
689@@ -1,477 +1,462 @@
690 'use strict';
691
692-describe('notifications', function() {
693- var Y, juju, models, views;
694-
695- var default_env = {
696- 'result': [
697- ['service', 'add', {
698- 'charm': 'cs:precise/wordpress-6',
699- 'id': 'wordpress',
700- 'exposed': false
701- }],
702- ['service', 'add', {
703- 'charm': 'cs:precise/mediawiki-3',
704- 'id': 'mediawiki',
705- 'exposed': false
706- }],
707- ['service', 'add', {
708- 'charm': 'cs:precise/mysql-6',
709- 'id': 'mysql'
710- }],
711- ['relation', 'add', {
712- 'interface': 'reversenginx',
713- 'scope': 'global',
714- 'endpoints':
715- [['wordpress', {'role': 'peer', 'name': 'loadbalancer'}]],
716- 'id': 'relation-0000000000'
717- }],
718- ['relation', 'add', {
719- 'interface': 'mysql',
720- 'scope': 'global',
721- 'endpoints':
722- [['mysql', {'role': 'server', 'name': 'db'}],
723- ['wordpress', {'role': 'client', 'name': 'db'}]],
724- 'id': 'relation-0000000001'
725- }],
726- ['machine', 'add', {
727- 'agent-state': 'running',
728- 'instance-state': 'running',
729- 'id': 0,
730- 'instance-id': 'local',
731- 'dns-name': 'localhost'
732- }],
733- ['unit', 'add', {
734- 'machine': 0,
735- 'agent-state': 'started',
736- 'public-address': '192.168.122.113',
737- 'id': 'wordpress/0'
738- }],
739- ['unit', 'add', {
740- 'machine': 0,
741- 'agent-state': 'error',
742- 'public-address': '192.168.122.222',
743- 'id': 'mysql/0'
744- }]
745- ],
746- 'op': 'delta'
747- };
748-
749-
750- before(function(done) {
751- Y = YUI(GlobalConfig).use([
752- 'juju-models',
753- 'juju-views',
754- 'juju-gui',
755- 'juju-env',
756- 'node-event-simulate',
757- 'juju-tests-utils'],
758-
759+YUI(GlobalConfig).use(['juju-gui', 'node-event-simulate', 'juju-tests-utils'],
760 function(Y) {
761- juju = Y.namespace('juju');
762- models = Y.namespace('juju.models');
763- views = Y.namespace('juju.views');
764- done();
765- });
766- });
767-
768- it('must be able to make notification and lists of notifications',
769- function() {
770- var note1 = new models.Notification({
771- title: 'test1',
772- message: 'Hello'
773- }),
774- note2 = new models.Notification({
775- title: 'test2',
776- message: 'I said goodnight!'
777- }),
778- notifications = new models.NotificationList();
779-
780- notifications.add([note1, note2]);
781- notifications.size().should.equal(2);
782-
783- // timestamp should be generated once
784- var ts = note1.get('timestamp');
785- note1.get('timestamp').should.equal(ts);
786- // force an update so we can test ordering
787- // fast execution can result in same timestamp
788- note2.set('timestamp', ts + 1);
789- note2.get('timestamp').should.be.above(ts);
790-
791- // defaults as expected
792- note1.get('level').should.equal('info');
793- note2.get('level').should.equal('info');
794- // the sort order on the list should be by
795- // timestamp
796- notifications.get('title').should.eql(['test2', 'test1']);
797- });
798-
799- it('must be able to render its view with sample data',
800- function() {
801- var note1 = new models.Notification({
802- title: 'test1', message: 'Hello'}),
803- note2 = new models.Notification({
804- title: 'test2', message: 'I said goodnight!'}),
805- notifications = new models.NotificationList(),
806- container = Y.Node.create('<div id="test">'),
807- env = new juju.Environment(),
808- view = new views.NotificationsView({
809- container: container,
810- notifications: notifications,
811- env: env});
812- view.render();
813- // Verify the expected elements appear in the view
814- container.one('#notify-list').should.not.equal(undefined);
815- container.destroy();
816- });
817-
818- it('must be able to limit the size of notification events',
819- function() {
820- var note1 = new models.Notification({
821- title: 'test1',
822- message: 'Hello'
823- }),
824- note2 = new models.Notification({
825- title: 'test2',
826- message: 'I said goodnight!'
827- }),
828- note3 = new models.Notification({
829- title: 'test3',
830- message: 'Never remember'
831- }),
832- notifications = new models.NotificationList({
833- max_size: 2
834- });
835-
836- notifications.add([note1, note2]);
837- notifications.size().should.equal(2);
838-
839- // Adding a new notification should pop the oldest from the list (we
840- // exceed max_size)
841- notifications.add(note3);
842- notifications.size().should.equal(2);
843- notifications.get('title').should.eql(['test3', 'test2']);
844- });
845-
846- it('must be able to get notifications for a given model',
847- function() {
848- var m = new models.Service({id: 'mediawiki'}),
849- note1 = new models.Notification({
850- title: 'test1',
851- message: 'Hello',
852- modelId: m
853- }),
854- note2 = new models.Notification({
855- title: 'test2',
856- message: 'I said goodnight!'
857- }),
858- notifications = new models.NotificationList();
859-
860- notifications.add([note1, note2]);
861- notifications.size().should.equal(2);
862- notifications.getNotificationsForModel(m).should.eql(
863- [note1]);
864-
865- });
866-
867- it('must be able to include and show object links', function() {
868- var container = Y.Node.create('<div id="test">'),
869- conn = new(Y.namespace('juju-tests.utils')).SocketStub(),
870- env = new juju.Environment({conn: conn}),
871- app = new Y.juju.App({env: env, container: container}),
872- db = app.db,
873- mw = db.services.create({id: 'mediawiki',
874- name: 'mediawiki'}),
875- notifications = db.notifications,
876- view = new views.NotificationsOverview({
877- container: container,
878- notifications: notifications,
879- app: app,
880- env: env}).render();
881- // we use overview here for testing as it defaults
882- // to showing all notices
883-
884- // we can use app's routing table to derive a link
885- notifications.create({title: 'Service Down',
886- message: 'Your service has an error',
887- link: app.getModelURL(mw)
888- });
889- view.render();
890- var link = container.one('.notice').one('a');
891- link.getAttribute('href').should.equal(
892- '/service/mediawiki/');
893- link.getHTML().should.contain('View Details');
894-
895-
896- // create a new notice passing the link_title
897- notifications.create({title: 'Service Down',
898- message: 'Your service has an error',
899- link: app.getModelURL(mw),
900- link_title: 'Resolve this'
901- });
902- view.render();
903- link = container.one('.notice').one('a');
904- link.getAttribute('href').should.equal(
905- '/service/mediawiki/');
906- link.getHTML().should.contain('Resolve this');
907- });
908-
909- it('must be able to evict irrelevant notices', function() {
910- var container = Y.Node.create(
911- '<div id="test" class="container"></div>'),
912- conn = new(Y.namespace('juju-tests.utils')).SocketStub(),
913- env = new juju.Environment({conn: conn}),
914- app = new Y.juju.App({
915- env: env,
916- container: container,
917- viewContainer: container
918- });
919- var environment_delta = default_env;
920-
921- var notifications = app.db.notifications,
922- view = new views.NotificationsView({
923- container: container,
924- notifications: notifications,
925- env: app.env}).render();
926-
927-
928- app.env.dispatch_result(environment_delta);
929-
930-
931- notifications.size().should.equal(7);
932- // we have one unit in error
933- view.getShowable().length.should.equal(1);
934-
935- // now fire another delta event marking that node as
936- // started
937- app.env.dispatch_result({result: [['unit', 'change', {
938- 'machine': 0,
939- 'agent-state': 'started',
940- 'public-address': '192.168.122.222',
941- 'id': 'mysql/0'
942- }]], op: 'delta'});
943- notifications.size().should.equal(8);
944- // This should have evicted the prior notice from seen
945- view.getShowable().length.should.equal(0);
946- });
947-
948- it('must properly construct title and message based on level from ' +
949- 'event data',
950- function() {
951- var container = Y.Node.create(
952- '<div id="test" class="container"></div>'),
953- app = new Y.juju.App({
954- container: container,
955- viewContainer: container
956- });
957- var environment_delta = {
958- 'result': [
959- ['service', 'add', {
960- 'charm': 'cs:precise/wordpress-6',
961- 'id': 'wordpress'
962- }],
963- ['service', 'add', {
964- 'charm': 'cs:precise/mediawiki-3',
965- 'id': 'mediawiki'
966- }],
967- ['service', 'add', {
968- 'charm': 'cs:precise/mysql-6',
969- 'id': 'mysql'
970- }],
971- ['unit', 'add', {
972- 'agent-state': 'install-error',
973- 'id': 'wordpress/0'
974- }],
975- ['unit', 'add', {
976- 'agent-state': 'error',
977- 'public-address': '192.168.122.222',
978- 'id': 'mysql/0'
979- }],
980- ['unit', 'add', {
981- 'public-address': '192.168.122.222',
982- 'id': 'mysql/2'
983- }]
984- ],
985- 'op': 'delta'
986- };
987-
988- var notifications = app.db.notifications,
989- view = new views.NotificationsView({
990- container: container,
991- notifications: notifications,
992- app: app,
993- env: app.env}).render();
994-
995- app.env.dispatch_result(environment_delta);
996-
997- notifications.size().should.equal(6);
998- // we have one unit in error
999- var showable = view.getShowable();
1000- showable.length.should.equal(2);
1001- // The first showable notification should indicate an error.
1002- showable[0].level.should.equal('error');
1003- showable[0].title.should.equal('Error with mysql/0');
1004- showable[0].message.should.equal('Agent-state = error.');
1005- // The second showable notification should also indicate an error.
1006- showable[1].level.should.equal('error');
1007- showable[1].title.should.equal('Error with wordpress/0');
1008- showable[1].message.should.equal('Agent-state = install-error.');
1009- // The first non-error notice should have an 'info' level and less
1010- // severe messaging.
1011- var notice = notifications.item(0);
1012- notice.get('level').should.equal('info');
1013- notice.get('title').should.equal('Problem with mysql/2');
1014- notice.get('message').should.equal('');
1015- });
1016-
1017-
1018- it('should open on click and close on clickoutside', function(done) {
1019- var container = Y.Node.create(
1020- '<div id="test-container" style="display: none" class="container"/>'),
1021- notifications = new models.NotificationList(),
1022- env = new juju.Environment(),
1023- view = new views.NotificationsView({
1024- container: container,
1025- notifications: notifications,
1026- env: env}).render(),
1027- indicator;
1028-
1029- Y.one('body').append(container);
1030- notifications.add({title: 'testing', 'level': 'error'});
1031- indicator = container.one('#notify-indicator');
1032-
1033- indicator.simulate('click');
1034- indicator.ancestor().hasClass('open').should.equal(true);
1035-
1036- Y.one('body').simulate('click');
1037- indicator.ancestor().hasClass('open').should.equal(false);
1038-
1039- container.remove();
1040- done();
1041- });
1042-
1043-});
1044-
1045-
1046-describe('changing notifications to words', function() {
1047- var Y, juju;
1048-
1049- before(function(done) {
1050- Y = YUI(GlobalConfig).use(
1051- ['juju-notification-controller'],
1052- function(Y) {
1053- juju = Y.namespace('juju');
1054- done();
1055- });
1056- });
1057-
1058- it('should correctly translate notification operations into English',
1059- function() {
1060- assert.equal(juju._changeNotificationOpToWords('add'), 'created');
1061- assert.equal(juju._changeNotificationOpToWords('remove'), 'removed');
1062- assert.equal(juju._changeNotificationOpToWords('not-an-op'), 'changed');
1063- });
1064-});
1065-
1066-describe('relation notifications', function() {
1067- var Y, juju;
1068-
1069- before(function(done) {
1070- Y = YUI(GlobalConfig).use(
1071- ['juju-notification-controller'],
1072- function(Y) {
1073- juju = Y.namespace('juju');
1074- done();
1075- });
1076- });
1077-
1078- it('should produce reasonable titles', function() {
1079- assert.equal(
1080- juju._relationNotifications.title(undefined, 'add'),
1081- 'Relation created');
1082- assert.equal(
1083- juju._relationNotifications.title(undefined, 'remove'),
1084- 'Relation removed');
1085- });
1086-
1087- it('should generate messages about two-party relations', function() {
1088- var changeData =
1089- { endpoints:
1090- [['endpoint0', {name: 'relation0'}],
1091- ['endpoint1', {name: 'relation1'}]]};
1092- assert.equal(
1093- juju._relationNotifications.message(undefined, 'add', changeData),
1094- 'Relation between endpoint0 (relation type "relation0") and ' +
1095- 'endpoint1 (relation type "relation1") was created');
1096- });
1097-
1098- it('should generate messages about one-party relations', function() {
1099- var changeData =
1100- { endpoints:
1101- [['endpoint1', {name: 'relation1'}]]};
1102- assert.equal(
1103- juju._relationNotifications.message(undefined, 'add', changeData),
1104- 'Relation with endpoint1 (relation type "relation1") was created');
1105- });
1106-});
1107-
1108-describe('notification visual feedback', function() {
1109- var env, models, notifications, notificationsView, notifierBox, views, Y;
1110-
1111- before(function(done) {
1112- Y = YUI(GlobalConfig).use('juju-env', 'juju-models', 'juju-views',
1113- function(Y) {
1114+ describe('notifications', function() {
1115+ var juju, models, views;
1116+
1117+ var default_env = {
1118+ 'result': [
1119+ ['service', 'add', {
1120+ 'charm': 'cs:precise/wordpress-6',
1121+ 'id': 'wordpress',
1122+ 'exposed': false
1123+ }],
1124+ ['service', 'add', {
1125+ 'charm': 'cs:precise/mediawiki-3',
1126+ 'id': 'mediawiki',
1127+ 'exposed': false
1128+ }],
1129+ ['service', 'add', {
1130+ 'charm': 'cs:precise/mysql-6',
1131+ 'id': 'mysql'
1132+ }],
1133+ ['relation', 'add', {
1134+ 'interface': 'reversenginx',
1135+ 'scope': 'global',
1136+ 'endpoints':
1137+ [['wordpress', {'role': 'peer', 'name': 'loadbalancer'}]],
1138+ 'id': 'relation-0000000000'
1139+ }],
1140+ ['relation', 'add', {
1141+ 'interface': 'mysql',
1142+ 'scope': 'global',
1143+ 'endpoints':
1144+ [['mysql', {'role': 'server', 'name': 'db'}],
1145+ ['wordpress', {'role': 'client', 'name': 'db'}]],
1146+ 'id': 'relation-0000000001'
1147+ }],
1148+ ['machine', 'add', {
1149+ 'agent-state': 'running',
1150+ 'instance-state': 'running',
1151+ 'id': 0,
1152+ 'instance-id': 'local',
1153+ 'dns-name': 'localhost'
1154+ }],
1155+ ['unit', 'add', {
1156+ 'machine': 0,
1157+ 'agent-state': 'started',
1158+ 'public-address': '192.168.122.113',
1159+ 'id': 'wordpress/0'
1160+ }],
1161+ ['unit', 'add', {
1162+ 'machine': 0,
1163+ 'agent-state': 'error',
1164+ 'public-address': '192.168.122.222',
1165+ 'id': 'mysql/0'
1166+ }]
1167+ ],
1168+ 'op': 'delta'
1169+ };
1170+
1171+
1172+ before(function() {
1173+ juju = Y.namespace('juju');
1174+ models = Y.namespace('juju.models');
1175+ views = Y.namespace('juju.views');
1176+ });
1177+
1178+ it('must be able to make notification and lists of notifications',
1179+ function() {
1180+ var note1 = new models.Notification({
1181+ title: 'test1',
1182+ message: 'Hello'
1183+ }),
1184+ note2 = new models.Notification({
1185+ title: 'test2',
1186+ message: 'I said goodnight!'
1187+ }),
1188+ notifications = new models.NotificationList();
1189+
1190+ notifications.add([note1, note2]);
1191+ notifications.size().should.equal(2);
1192+
1193+ // timestamp should be generated once
1194+ var ts = note1.get('timestamp');
1195+ note1.get('timestamp').should.equal(ts);
1196+ // force an update so we can test ordering
1197+ // fast execution can result in same timestamp
1198+ note2.set('timestamp', ts + 1);
1199+ note2.get('timestamp').should.be.above(ts);
1200+
1201+ // defaults as expected
1202+ note1.get('level').should.equal('info');
1203+ note2.get('level').should.equal('info');
1204+ // the sort order on the list should be by
1205+ // timestamp
1206+ notifications.get('title').should.eql(['test2', 'test1']);
1207+ });
1208+
1209+ it('must be able to render its view with sample data',
1210+ function() {
1211+ var note1 = new models.Notification({
1212+ title: 'test1', message: 'Hello'}),
1213+ note2 = new models.Notification({
1214+ title: 'test2', message: 'I said goodnight!'}),
1215+ notifications = new models.NotificationList(),
1216+ container = Y.Node.create('<div id="test">'),
1217+ env = new juju.Environment(),
1218+ view = new views.NotificationsView({
1219+ container: container,
1220+ notifications: notifications,
1221+ env: env});
1222+ view.render();
1223+ // Verify the expected elements appear in the view
1224+ container.one('#notify-list').should.not.equal(undefined);
1225+ container.destroy();
1226+ });
1227+
1228+ it('must be able to limit the size of notification events',
1229+ function() {
1230+ var note1 = new models.Notification({
1231+ title: 'test1',
1232+ message: 'Hello'
1233+ }),
1234+ note2 = new models.Notification({
1235+ title: 'test2',
1236+ message: 'I said goodnight!'
1237+ }),
1238+ note3 = new models.Notification({
1239+ title: 'test3',
1240+ message: 'Never remember'
1241+ }),
1242+ notifications = new models.NotificationList({
1243+ max_size: 2
1244+ });
1245+
1246+ notifications.add([note1, note2]);
1247+ notifications.size().should.equal(2);
1248+
1249+ // Adding a new notification should pop the oldest from the list
1250+ // (we exceed max_size)
1251+ notifications.add(note3);
1252+ notifications.size().should.equal(2);
1253+ notifications.get('title').should.eql(['test3', 'test2']);
1254+ });
1255+
1256+ it('must be able to get notifications for a given model',
1257+ function() {
1258+ var m = new models.Service({id: 'mediawiki'}),
1259+ note1 = new models.Notification({
1260+ title: 'test1',
1261+ message: 'Hello',
1262+ modelId: m
1263+ }),
1264+ note2 = new models.Notification({
1265+ title: 'test2',
1266+ message: 'I said goodnight!'
1267+ }),
1268+ notifications = new models.NotificationList();
1269+
1270+ notifications.add([note1, note2]);
1271+ notifications.size().should.equal(2);
1272+ notifications.getNotificationsForModel(m).should.eql(
1273+ [note1]);
1274+
1275+ });
1276+
1277+ it('must be able to include and show object links', function() {
1278+ var container = Y.Node.create('<div id="test">'),
1279+ conn = new(Y.namespace('juju-tests.utils')).SocketStub(),
1280+ env = new juju.Environment({conn: conn}),
1281+ app = new Y.juju.App({env: env, container: container}),
1282+ db = app.db,
1283+ mw = db.services.create({id: 'mediawiki',
1284+ name: 'mediawiki'}),
1285+ notifications = db.notifications,
1286+ view = new views.NotificationsOverview({
1287+ container: container,
1288+ notifications: notifications,
1289+ app: app,
1290+ env: env}).render();
1291+ // we use overview here for testing as it defaults
1292+ // to showing all notices
1293+
1294+ // we can use app's routing table to derive a link
1295+ notifications.create({title: 'Service Down',
1296+ message: 'Your service has an error',
1297+ link: app.getModelURL(mw)
1298+ });
1299+ view.render();
1300+ var link = container.one('.notice').one('a');
1301+ link.getAttribute('href').should.equal(
1302+ '/service/mediawiki/');
1303+ link.getHTML().should.contain('View Details');
1304+
1305+
1306+ // create a new notice passing the link_title
1307+ notifications.create({title: 'Service Down',
1308+ message: 'Your service has an error',
1309+ link: app.getModelURL(mw),
1310+ link_title: 'Resolve this'
1311+ });
1312+ view.render();
1313+ link = container.one('.notice').one('a');
1314+ link.getAttribute('href').should.equal(
1315+ '/service/mediawiki/');
1316+ link.getHTML().should.contain('Resolve this');
1317+ });
1318+
1319+ it('must be able to evict irrelevant notices', function() {
1320+ var container = Y.Node.create(
1321+ '<div id="test" class="container"></div>'),
1322+ conn = new(Y.namespace('juju-tests.utils')).SocketStub(),
1323+ env = new juju.Environment({conn: conn}),
1324+ app = new Y.juju.App({
1325+ env: env,
1326+ container: container,
1327+ viewContainer: container
1328+ });
1329+ var environment_delta = default_env;
1330+
1331+ var notifications = app.db.notifications,
1332+ view = new views.NotificationsView({
1333+ container: container,
1334+ notifications: notifications,
1335+ env: app.env}).render();
1336+
1337+
1338+ app.env.dispatch_result(environment_delta);
1339+
1340+
1341+ notifications.size().should.equal(7);
1342+ // we have one unit in error
1343+ view.getShowable().length.should.equal(1);
1344+
1345+ // now fire another delta event marking that node as
1346+ // started
1347+ app.env.dispatch_result({result: [['unit', 'change', {
1348+ 'machine': 0,
1349+ 'agent-state': 'started',
1350+ 'public-address': '192.168.122.222',
1351+ 'id': 'mysql/0'
1352+ }]], op: 'delta'});
1353+ notifications.size().should.equal(8);
1354+ // This should have evicted the prior notice from seen
1355+ view.getShowable().length.should.equal(0);
1356+ });
1357+
1358+ it('must properly construct title and message based on level from ' +
1359+ 'event data',
1360+ function() {
1361+ var container = Y.Node.create(
1362+ '<div id="test" class="container"></div>'),
1363+ app = new Y.juju.App({
1364+ container: container,
1365+ viewContainer: container
1366+ });
1367+ var environment_delta = {
1368+ 'result': [
1369+ ['service', 'add', {
1370+ 'charm': 'cs:precise/wordpress-6',
1371+ 'id': 'wordpress'
1372+ }],
1373+ ['service', 'add', {
1374+ 'charm': 'cs:precise/mediawiki-3',
1375+ 'id': 'mediawiki'
1376+ }],
1377+ ['service', 'add', {
1378+ 'charm': 'cs:precise/mysql-6',
1379+ 'id': 'mysql'
1380+ }],
1381+ ['unit', 'add', {
1382+ 'agent-state': 'install-error',
1383+ 'id': 'wordpress/0'
1384+ }],
1385+ ['unit', 'add', {
1386+ 'agent-state': 'error',
1387+ 'public-address': '192.168.122.222',
1388+ 'id': 'mysql/0'
1389+ }],
1390+ ['unit', 'add', {
1391+ 'public-address': '192.168.122.222',
1392+ 'id': 'mysql/2'
1393+ }]
1394+ ],
1395+ 'op': 'delta'
1396+ };
1397+
1398+ var notifications = app.db.notifications,
1399+ view = new views.NotificationsView({
1400+ container: container,
1401+ notifications: notifications,
1402+ app: app,
1403+ env: app.env}).render();
1404+
1405+ app.env.dispatch_result(environment_delta);
1406+
1407+ notifications.size().should.equal(6);
1408+ // we have one unit in error
1409+ var showable = view.getShowable();
1410+ showable.length.should.equal(2);
1411+ // The first showable notification should indicate an error.
1412+ showable[0].level.should.equal('error');
1413+ showable[0].title.should.equal('Error with mysql/0');
1414+ showable[0].message.should.equal('Agent-state = error.');
1415+ // The second showable notification should also indicate an error.
1416+ showable[1].level.should.equal('error');
1417+ showable[1].title.should.equal('Error with wordpress/0');
1418+ showable[1].message.should.equal('Agent-state = install-error.');
1419+ // The first non-error notice should have an 'info' level and less
1420+ // severe messaging.
1421+ var notice = notifications.item(0);
1422+ notice.get('level').should.equal('info');
1423+ notice.get('title').should.equal('Problem with mysql/2');
1424+ notice.get('message').should.equal('');
1425+ });
1426+
1427+
1428+ it('should open on click and close on clickoutside', function(done) {
1429+ var container = Y.Node.create('<div id="test-container" ' +
1430+ 'style="display: none" class="container"/>'),
1431+ notifications = new models.NotificationList(),
1432+ env = new juju.Environment(),
1433+ view = new views.NotificationsView({
1434+ container: container,
1435+ notifications: notifications,
1436+ env: env}).render(),
1437+ indicator;
1438+
1439+ Y.one('body').append(container);
1440+ notifications.add({title: 'testing', 'level': 'error'});
1441+ indicator = container.one('#notify-indicator');
1442+
1443+ indicator.simulate('click');
1444+ indicator.ancestor().hasClass('open').should.equal(true);
1445+
1446+ Y.one('body').simulate('click');
1447+ indicator.ancestor().hasClass('open').should.equal(false);
1448+
1449+ container.remove();
1450+ done();
1451+ });
1452+ });
1453+
1454+ describe('changing notifications to words', function() {
1455+ var juju;
1456+
1457+ before(function() {
1458+ juju = Y.namespace('juju');
1459+ });
1460+
1461+ it('should correctly translate notification operations into English',
1462+ function() {
1463+ assert.equal(juju._changeNotificationOpToWords('add'), 'created');
1464+ assert.equal(juju._changeNotificationOpToWords('remove'),
1465+ 'removed');
1466+ assert.equal(juju._changeNotificationOpToWords('not-an-op'),
1467+ 'changed');
1468+ });
1469+ });
1470+
1471+ describe('relation notifications', function() {
1472+ var juju;
1473+
1474+ before(function() {
1475+ juju = Y.namespace('juju');
1476+ });
1477+
1478+ it('should produce reasonable titles', function() {
1479+ assert.equal(
1480+ juju._relationNotifications.title(undefined, 'add'),
1481+ 'Relation created');
1482+ assert.equal(
1483+ juju._relationNotifications.title(undefined, 'remove'),
1484+ 'Relation removed');
1485+ });
1486+
1487+ it('should generate messages about two-party relations', function() {
1488+ var changeData =
1489+ { endpoints:
1490+ [['endpoint0', {name: 'relation0'}],
1491+ ['endpoint1', {name: 'relation1'}]]};
1492+ assert.equal(
1493+ juju._relationNotifications.message(undefined, 'add',
1494+ changeData), 'Relation between endpoint0 (relation type ' +
1495+ '"relation0") and endpoint1 (relation type "relation1") ' +
1496+ 'was created');
1497+ });
1498+
1499+ it('should generate messages about one-party relations', function() {
1500+ var changeData =
1501+ { endpoints:
1502+ [['endpoint1', {name: 'relation1'}]]};
1503+ assert.equal(
1504+ juju._relationNotifications.message(undefined, 'add',
1505+ changeData), 'Relation with endpoint1 (relation type' +
1506+ '"relation1") was created');
1507+ });
1508+ });
1509+
1510+ describe('notification visual feedback', function() {
1511+ var env, models, notifications, notificationsView, notifierBox, views;
1512+
1513+ before(function() {
1514 var juju = Y.namespace('juju');
1515 env = new juju.Environment();
1516 models = Y.namespace('juju.models');
1517 views = Y.namespace('juju.views');
1518- done();
1519- });
1520- });
1521-
1522- // Instantiate the notifications model list and view.
1523- // Also create the notifier box and attach it as first element of the body.
1524- beforeEach(function() {
1525- notifications = new models.NotificationList();
1526- notificationsView = new views.NotificationsView({
1527- env: env,
1528- notifications: notifications
1529+ });
1530+
1531+ // Instantiate the notifications model list and view.
1532+ // Also create the notifier box and attach it as first element of the
1533+ // body.
1534+ beforeEach(function() {
1535+ notifications = new models.NotificationList();
1536+ notificationsView = new views.NotificationsView({
1537+ env: env,
1538+ notifications: notifications
1539+ });
1540+ notifierBox = Y.Node.create('<div id="notifier-box"></div>');
1541+ notifierBox.setStyle('display', 'none');
1542+ Y.one('body').prepend(notifierBox);
1543+ });
1544+
1545+ // Destroy the notifier box created in beforeEach.
1546+ afterEach(function() {
1547+ notifierBox.remove();
1548+ notifierBox.destroy(true);
1549+ });
1550+
1551+ // Assert the notifier box contains the expectedNumber of notifiers.
1552+ var assertNumNotifiers = function(expectedNumber) {
1553+ assert.equal(expectedNumber, notifierBox.get('children').size());
1554+ };
1555+
1556+ it('should appear when a new error is notified', function() {
1557+ notifications.add({title: 'mytitle', level: 'error'});
1558+ assertNumNotifiers(1);
1559+ });
1560+
1561+ it('should only appear when the DOM contains the notifier box',
1562+ function() {
1563+ notifierBox.remove();
1564+ notifications.add({title: 'mytitle', level: 'error'});
1565+ assertNumNotifiers(0);
1566+ });
1567+
1568+ it('should not appear when the notification is not an error',
1569+ function() {
1570+ notifications.add({title: 'mytitle', level: 'info'});
1571+ assertNumNotifiers(0);
1572+ });
1573+
1574+ it('should not appear when the notification comes form delta',
1575+ function() {
1576+ notifications.add({title: 'mytitle', level: 'error', isDelta:
1577+ true});
1578+ assertNumNotifiers(0);
1579+ });
1580+
1581+ });
1582 });
1583- notifierBox = Y.Node.create('<div id="notifier-box"></div>');
1584- notifierBox.setStyle('display', 'none');
1585- Y.one('body').prepend(notifierBox);
1586- });
1587-
1588- // Destroy the notifier box created in beforeEach.
1589- afterEach(function() {
1590- notifierBox.remove();
1591- notifierBox.destroy(true);
1592- });
1593-
1594- // Assert the notifier box contains the expectedNumber of notifiers.
1595- var assertNumNotifiers = function(expectedNumber) {
1596- assert.equal(expectedNumber, notifierBox.get('children').size());
1597- };
1598-
1599- it('should appear when a new error is notified', function() {
1600- notifications.add({title: 'mytitle', level: 'error'});
1601- assertNumNotifiers(1);
1602- });
1603-
1604- it('should only appear when the DOM contains the notifier box', function() {
1605- notifierBox.remove();
1606- notifications.add({title: 'mytitle', level: 'error'});
1607- assertNumNotifiers(0);
1608- });
1609-
1610- it('should not appear when the notification is not an error', function() {
1611- notifications.add({title: 'mytitle', level: 'info'});
1612- assertNumNotifiers(0);
1613- });
1614-
1615- it('should not appear when the notification comes form delta', function() {
1616- notifications.add({title: 'mytitle', level: 'error', isDelta: true});
1617- assertNumNotifiers(0);
1618- });
1619-
1620-});
1621
1622=== modified file 'test/test_notifier_widget.js'
1623--- test/test_notifier_widget.js 2012-11-16 13:37:20 +0000
1624+++ test/test_notifier_widget.js 2012-11-20 23:07:20 +0000
1625@@ -1,103 +1,101 @@
1626 'use strict';
1627
1628-describe('notifier widget', function() {
1629- var Notifier, notifierBox, Y;
1630-
1631- before(function(done) {
1632- Y = YUI(GlobalConfig).use('notifier', 'node-event-simulate',
1633- function(Y) {
1634- Notifier = Y.namespace('juju.widgets').Notifier;
1635- done();
1636- });
1637- });
1638-
1639- // Create the notifier box and attach it as first element of the body.
1640- beforeEach(function() {
1641- notifierBox = Y.Node.create('<div id="notifier-box"></div>');
1642- notifierBox.setStyle('display', 'none');
1643- Y.one('body').prepend(notifierBox);
1644- });
1645-
1646- // Destroy the notifier box created in beforeEach.
1647- afterEach(function() {
1648- notifierBox.remove();
1649- notifierBox.destroy(true);
1650- });
1651-
1652- // Factory rendering and returning a notifier instance.
1653- var makeNotifier = function(title, message, timeout) {
1654- var notifier = new Notifier({
1655- title: title || 'mytitle',
1656- message: message || 'mymessage',
1657- timeout: timeout || 10000
1658- });
1659- notifier.render(notifierBox);
1660- return notifier;
1661- };
1662-
1663- // Assert the notifier box contains the expectedNumber of notifiers.
1664- var assertNumNotifiers = function(expectedNumber) {
1665- assert.equal(expectedNumber, notifierBox.get('children').size());
1666- };
1667-
1668- it('should be able to display a notification', function() {
1669- makeNotifier();
1670- assertNumNotifiers(1);
1671- });
1672-
1673- it('should display the given title and message', function() {
1674- makeNotifier('mytitle', 'mymessage');
1675- var notifierNode = notifierBox.one('*');
1676- assert.equal('mytitle', notifierNode.one('h3').getContent());
1677- assert.equal('mymessage', notifierNode.one('div').getContent());
1678- });
1679-
1680- it('should be able to display multiple notifications', function() {
1681- var number = 10;
1682- for (var i = 0; i < number; i += 1) {
1683+YUI(GlobalConfig).use(['notifier', 'node-event-simulate'], function(Y) {
1684+ describe('notifier widget', function() {
1685+ var Notifier, notifierBox;
1686+
1687+ before(function() {
1688+ Notifier = Y.namespace('juju.widgets').Notifier;
1689+ });
1690+
1691+ // Create the notifier box and attach it as first element of the body.
1692+ beforeEach(function() {
1693+ notifierBox = Y.Node.create('<div id="notifier-box"></div>');
1694+ notifierBox.setStyle('display', 'none');
1695+ Y.one('body').prepend(notifierBox);
1696+ });
1697+
1698+ // Destroy the notifier box created in beforeEach.
1699+ afterEach(function() {
1700+ notifierBox.remove();
1701+ notifierBox.destroy(true);
1702+ });
1703+
1704+ // Factory rendering and returning a notifier instance.
1705+ var makeNotifier = function(title, message, timeout) {
1706+ var notifier = new Notifier({
1707+ title: title || 'mytitle',
1708+ message: message || 'mymessage',
1709+ timeout: timeout || 10000
1710+ });
1711+ notifier.render(notifierBox);
1712+ return notifier;
1713+ };
1714+
1715+ // Assert the notifier box contains the expectedNumber of notifiers.
1716+ var assertNumNotifiers = function(expectedNumber) {
1717+ assert.equal(expectedNumber, notifierBox.get('children').size());
1718+ };
1719+
1720+ it('should be able to display a notification', function() {
1721 makeNotifier();
1722- }
1723- assertNumNotifiers(number);
1724- });
1725-
1726- it('should display new notifications on top', function() {
1727- makeNotifier('mytitle1', 'mymessage1');
1728- makeNotifier('mytitle2', 'mymessage2');
1729- var notifierNode = notifierBox.one('*');
1730- assert.equal('mytitle2', notifierNode.one('h3').getContent());
1731- assert.equal('mymessage2', notifierNode.one('div').getContent());
1732- });
1733-
1734- it('should destroy notifications after N milliseconds', function(done) {
1735- makeNotifier('mytitle', 'mymessage', 1);
1736- // A timeout of 250 milliseconds is used so that we ensure the destroying
1737- // animation can be completed.
1738- setTimeout(function() {
1739- assertNumNotifiers(0);
1740- done();
1741- }, 250);
1742- });
1743-
1744- it('should destroy notifications on click', function(done) {
1745- makeNotifier();
1746- notifierBox.one('*').simulate('click');
1747- // A timeout of 250 milliseconds is used so that we ensure the destroying
1748- // animation can be completed.
1749- setTimeout(function() {
1750- assertNumNotifiers(0);
1751- done();
1752- }, 250);
1753- });
1754-
1755- it('should prevent notification removal on mouse enter', function(done) {
1756- makeNotifier('mytitle', 'mymessage', 1);
1757- notifierBox.one('*').simulate('mouseover');
1758- // A timeout of 250 milliseconds is used so that we ensure the node is not
1759- // preserved by the destroying animation.
1760- setTimeout(function() {
1761 assertNumNotifiers(1);
1762- done();
1763- }, 250);
1764+ });
1765+
1766+ it('should display the given title and message', function() {
1767+ makeNotifier('mytitle', 'mymessage');
1768+ var notifierNode = notifierBox.one('*');
1769+ assert.equal('mytitle', notifierNode.one('h3').getContent());
1770+ assert.equal('mymessage', notifierNode.one('div').getContent());
1771+ });
1772+
1773+ it('should be able to display multiple notifications', function() {
1774+ var number = 10;
1775+ for (var i = 0; i < number; i += 1) {
1776+ makeNotifier();
1777+ }
1778+ assertNumNotifiers(number);
1779+ });
1780+
1781+ it('should display new notifications on top', function() {
1782+ makeNotifier('mytitle1', 'mymessage1');
1783+ makeNotifier('mytitle2', 'mymessage2');
1784+ var notifierNode = notifierBox.one('*');
1785+ assert.equal('mytitle2', notifierNode.one('h3').getContent());
1786+ assert.equal('mymessage2', notifierNode.one('div').getContent());
1787+ });
1788+
1789+ it('should destroy notifications after N milliseconds', function(done) {
1790+ makeNotifier('mytitle', 'mymessage', 1);
1791+ // A timeout of 250 milliseconds is used so that we ensure the destroying
1792+ // animation can be completed.
1793+ setTimeout(function() {
1794+ assertNumNotifiers(0);
1795+ done();
1796+ }, 250);
1797+ });
1798+
1799+ it('should destroy notifications on click', function(done) {
1800+ makeNotifier();
1801+ notifierBox.one('*').simulate('click');
1802+ // A timeout of 250 milliseconds is used so that we ensure the destroying
1803+ // animation can be completed.
1804+ setTimeout(function() {
1805+ assertNumNotifiers(0);
1806+ done();
1807+ }, 250);
1808+ });
1809+
1810+ it('should prevent notification removal on mouse enter', function(done) {
1811+ makeNotifier('mytitle', 'mymessage', 1);
1812+ notifierBox.one('*').simulate('mouseover');
1813+ // A timeout of 250 milliseconds is used so that we ensure the node is not
1814+ // preserved by the destroying animation.
1815+ setTimeout(function() {
1816+ assertNumNotifiers(1);
1817+ done();
1818+ }, 250);
1819+ });
1820+
1821 });
1822-
1823 });

Subscribers

People subscribed via source and target branches