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

Issue 8099047: Moved the YUI sandbox into the describe closure

Can't Edit
Can't Publish+Mail
Start Review
Created:
11 years, 1 month ago by jeff.pihach
Modified:
11 years, 1 month ago
Reviewers:
rharding, mp+156091, gary.poster
Visibility:
Public.

Description

Moved the YUI sandbox into the describe closure For whatever reason one of the describe closures in test_model.js was wrapped in a YUI sandbox. This caused whatever test suite was run after it in .only mode to fail with an extend failure. Moving the YUI sandbox inside to match our other tests has solved this. https://code.launchpad.net/~hatch/juju-gui/model-test-fix/+merge/156091 (do not edit description out of merge proposal)

Patch Set 1 #

Patch Set 2 : Moved the YUI sandbox into the describe closure #

Unified diffs Side-by-side diffs Delta from patch set Stats (+153 lines, -152 lines) Patch
A [revision details] View 1 chunk +2 lines, -0 lines 0 comments Download
M test/test_model.js View 1 chunk +151 lines, -152 lines 0 comments Download

Messages

Total messages: 4
jeff.pihach
Please take a look.
11 years, 1 month ago (2013-03-28 23:45:22 UTC) #1
gary.poster
LGTM. Thank you! Gary
11 years, 1 month ago (2013-03-28 23:49:43 UTC) #2
rharding
LGTM and I qa'd that it fixes the issue where notification.only() was set. This corrects ...
11 years, 1 month ago (2013-03-28 23:50:43 UTC) #3
jeff.pihach
11 years, 1 month ago (2013-03-28 23:55:34 UTC) #4
*** Submitted:

Moved the YUI sandbox into the describe closure

For whatever reason one of the describe closures in test_model.js
was wrapped in a YUI sandbox. This caused whatever test suite was 
run after it in .only mode to fail with an extend failure. Moving
the YUI sandbox inside to match our other tests has solved this.

R=gary.poster, rharding
CC=
https://codereview.appspot.com/8099047
Sign in to reply to this message.

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