Make the prod tests pass.

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://code.launchpad.net/~benji/juju-gui/bug-1088507/+merge/140020

(do not edit description out of merge proposal)

[benji, 2012-12-14 20:45:46]

Please take a look.

[gary.poster, 2012-12-14 21:18:37]

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.

[benji, 2012-12-14 21:43:32]

Thanks for the review.  I have done all requested of me.

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'));
On 2012/12/14 21:18:37, gary.poster wrote:
> 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.

Done.

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>
On 2012/12/14 21:18:37, gary.poster wrote:
> You explained the horror behind these two inclusions.  Please add a comment so
> others can enjoy.

Done.

[teknico, 2012-12-17 12:09:16]

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.

[benji, 2012-12-17 14:55:55]

Please take a look.

[teknico, 2012-12-17 15:09:31]

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

Land this.

[benji, 2012-12-17 15:15:38]

*** Submitted:

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.

R=gary.poster, teknico
CC=
https://codereview.appspot.com/6947057