Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(977)

Issue 6947057: Make the prod tests pass.

Can't Edit
Can't Publish+Mail
Start Review
Created:
7 years, 9 months ago by benji
Modified:
7 years, 9 months ago
Reviewers:
mp+140020
Visibility:
Public.

Description

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)

Patch Set 1 #

Total comments: 6

Patch Set 2 : Make the prod tests pass. #

Patch Set 3 : Make the prod tests pass. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+435 lines, -395 lines) Patch
M Makefile View 1 2 chunks +19 lines, -10 lines 0 comments Download
A [revision details] View 1 1 chunk +2 lines, -0 lines 0 comments Download
M app/app.js View 1 chunk +1 line, -0 lines 0 comments Download
M app/views/charm-panel.js View 2 chunks +32 lines, -4 lines 0 comments Download
M bin/merge-files View 1 1 chunk +2 lines, -0 lines 0 comments Download
M test/index.html View 1 1 chunk +5 lines, -1 line 0 comments Download
M test/test_charm_configuration.js View 2 chunks +20 lines, -29 lines 0 comments Download
M test/test_model.js View 1 chunk +354 lines, -351 lines 0 comments Download

Messages

Total messages: 7
benji
Please take a look.
7 years, 9 months ago (2012-12-14 20:45:46 UTC) #1
gary.poster
Land with changes. Thank you for accomplishing this difficult task. In addition to the comments ...
7 years, 9 months ago (2012-12-14 21:18:37 UTC) #2
benji
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): ...
7 years, 9 months ago (2012-12-14 21:43:32 UTC) #3
teknico
Needs fixing. Running "make test-prod" produces 60 failures here, which is admittedly a nice improvement ...
7 years, 9 months ago (2012-12-17 12:09:16 UTC) #4
benji
Please take a look.
7 years, 9 months ago (2012-12-17 14:55:55 UTC) #5
teknico
It works now, thanks. Do not worry about the other issue, we'll worry about it ...
7 years, 9 months ago (2012-12-17 15:09:31 UTC) #6
benji
7 years, 9 months ago (2012-12-17 15:15:38 UTC) #7
*** 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
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b