Do not re-render MV if already exists. #352
Conversation
|
Test FAILed. |
|
Test PASSed. |
| @@ -507,8 +507,15 @@ with this program. If not, see <http://www.gnu.org/licenses/>. | |||
|
|
|||
| it('emptySectionB', function() { | |||
| stubMethods(app); | |||
| var destroyMethod = app.machineViewPanel.destroy, | |||
There was a problem hiding this comment.
Please use the stubbing methods for this.
There was a problem hiding this comment.
Not really sure how I'm to use the stubbed destroy method… Since emptySectionB now nulls out the machineViewPanel object, we lose the ability to check callCount() or anything else on the stubbed destroy. I copied the approach used in the emptySectionA test to work around this issue.
There was a problem hiding this comment.
That's a good point, can you add a comment to that effect into the code.
Just FYI: The issue with this technique is that if the destroy method never gets called then it never gets re-assigned and we have had issues in the past where this type of technique caused hard to debug cascading failures.
|
👍 After the test is converted to use the stubbing methods. |
|
Clarifying comment added. Thanks for the review and QA! |
|
Status: merge request accepted. Url: http://ci.jujugui.org:8080/job/juju-gui-merge |
|
Test PASSed. |
### QA 1. Switch to machine view. 2. Use the inspector to view bundle details for a bundle and then click on the button in the details panel to deploy it. 3. You should only see one copy of each machine show up in the machines column. One machine 0, one machine 1. Not three of each. Do some other playing around, switching between the canvas and MV (i.e., destroying MV, recreating it), etc. Make sure everything looks good.
QA
Do some other playing around, switching between the canvas and MV (i.e., destroying MV, recreating it), etc. Make sure everything looks good.