From 13df3e7bd627dbb31738f0548d2a0952cdc3f8c4 Mon Sep 17 00:00:00 2001 From: Karishma Chadha Date: Wed, 11 Jul 2018 12:27:20 -0400 Subject: [PATCH] Add unit tests and fix object shallow vs. deep copying bug. --- src/virtual-machine.js | 12 ++- test/unit/virtual-machine.js | 203 ++++++++++++++++++++++++++++++++++- 2 files changed, 209 insertions(+), 6 deletions(-) diff --git a/src/virtual-machine.js b/src/virtual-machine.js index 87a81e8eb..95f497072 100644 --- a/src/virtual-machine.js +++ b/src/virtual-machine.js @@ -1000,12 +1000,14 @@ class VirtualMachine extends EventEmitter { * @param {!string} targetId Id of target to add blocks to. */ shareBlocksToTarget (blocks, targetId) { + const copiedBlocks = JSON.parse(JSON.stringify(blocks)); const currTarget = this.runtime.getEditingTarget(); const target = this.runtime.getTargetById(targetId); // Resolve any potential local variable conflicts if the current sprite (sharing the love) // is a non-stage target. if (!currTarget.isStage) { - const allVarListRefs = currTarget.blocks.getAllVariableAndListReferences(blocks); + const allVarListRefs = currTarget.blocks.getAllVariableAndListReferences( + copiedBlocks); for (const varId in allVarListRefs) { const currVarListRefs = allVarListRefs[varId]; const currVar = currTarget.variables[varId]; @@ -1018,8 +1020,8 @@ class VirtualMachine extends EventEmitter { if (target.isStage) { // If a local var is being shared with the stage, // we'll prefix the ID of the current variable, - // and use a fresh, prefixed name for the variable. - // This way, multiple share the loves from the same target, referring + // and use a fresh, prefixed name for the new global variable. + // This way, multiple share the loves from the same target, referring to // the same variable will map to the same new variable on the stage. const varIdForStage = `StageVarFromLocal_${varId}`; existingVar = target.lookupVariableById(varIdForStage); @@ -1060,8 +1062,8 @@ class VirtualMachine extends EventEmitter { } } - for (let i = 0; i < blocks.length; i++) { - target.blocks.createBlock(blocks[i]); + for (let i = 0; i < copiedBlocks.length; i++) { + target.blocks.createBlock(copiedBlocks[i]); } target.blocks.updateTargetSpecificBlocks(target.isStage); } diff --git a/test/unit/virtual-machine.js b/test/unit/virtual-machine.js index 61534e03d..e228363c5 100644 --- a/test/unit/virtual-machine.js +++ b/test/unit/virtual-machine.js @@ -2,6 +2,7 @@ const test = require('tap').test; const VirtualMachine = require('../../src/virtual-machine.js'); const Sprite = require('../../src/sprites/sprite.js'); const Variable = require('../../src/engine/variable.js'); +const adapter = require('../../src/engine/adapter'); const events = require('../fixtures/events.json'); test('addSprite throws on invalid string', t => { @@ -622,7 +623,7 @@ test('getVariableValue', t => { // Returns null if there is no variable with that id t.equal(vm.getVariableValue(target.id, 'not-a-variable'), null); - // Returns null if there is no variable with that id + // Returns null if there is no target with that id t.equal(vm.getVariableValue('not-a-target', 'a-variable'), null); // Returns true and updates the value if variable is present @@ -656,3 +657,203 @@ test('comment_create event updates comment with null position', t => { t.end(); }); + +test('shareBlocksToTarget shares global variables without any name changes', t => { + const vm = new VirtualMachine(); + const runtime = vm.runtime; + const spr1 = new Sprite(null, runtime); + const stage = spr1.createClone(); + stage.isStage = true; + + const spr2 = new Sprite(null, runtime); + const target = spr2.createClone(); + + runtime.targets = [stage, target]; + vm.editingTarget = target; + vm.runtime.setEditingTarget(target); + + stage.createVariable('mock var id', 'a mock variable', Variable.SCALAR_TYPE); + t.equal(Object.keys(target.variables).length, 0); + t.equal(Object.keys(stage.variables).length, 1); + t.equal(stage.variables['mock var id'].name, 'a mock variable'); + + + vm.setVariableValue(stage.id, 'mock var id', 10); + t.equal(vm.getVariableValue(stage.id, 'mock var id'), 10); + + target.blocks.createBlock(adapter(events.mockVariableBlock)[0]); + + // Verify the block exists on the target, and that it references the global variable + t.type(target.blocks.getBlock('a block'), 'object'); + t.type(target.blocks.getBlock('a block').fields, 'object'); + t.type(target.blocks.getBlock('a block').fields.VARIABLE, 'object'); + t.equal(target.blocks.getBlock('a block').fields.VARIABLE.id, 'mock var id'); + + // Verify that the block does not exist on the stage + t.type(stage.blocks.getBlock('a block'), 'undefined'); + + // Share the block to the stage + vm.shareBlocksToTarget([target.blocks.getBlock('a block')], stage.id); + + // Verify that the block now exists on the target as well as the stage + t.type(target.blocks.getBlock('a block'), 'object'); + t.type(target.blocks.getBlock('a block').fields, 'object'); + t.type(target.blocks.getBlock('a block').fields.VARIABLE, 'object'); + t.equal(target.blocks.getBlock('a block').fields.VARIABLE.id, 'mock var id'); + + t.type(stage.blocks.getBlock('a block'), 'object'); + t.type(stage.blocks.getBlock('a block').fields, 'object'); + t.type(stage.blocks.getBlock('a block').fields.VARIABLE, 'object'); + t.equal(stage.blocks.getBlock('a block').fields.VARIABLE.id, 'mock var id'); + + // Verify that the variables haven't changed, the variable still exists on the + // stage, it should still have the same name and value, and there should be + // no variables on the target. + t.equal(Object.keys(target.variables).length, 0); + t.equal(Object.keys(stage.variables).length, 1); + t.equal(stage.variables['mock var id'].name, 'a mock variable'); + t.equal(vm.getVariableValue(stage.id, 'mock var id'), 10); + + t.end(); +}); + +test('shareBlocksToTarget shares a local variable to the stage, creating a global variable with a new name', t => { + const vm = new VirtualMachine(); + const runtime = vm.runtime; + const spr1 = new Sprite(null, runtime); + const stage = spr1.createClone(); + stage.isStage = true; + + const spr2 = new Sprite(null, runtime); + const target = spr2.createClone(); + + runtime.targets = [stage, target]; + vm.editingTarget = target; + vm.runtime.setEditingTarget(target); + + target.createVariable('mock var id', 'a mock variable', Variable.SCALAR_TYPE); + t.equal(Object.keys(stage.variables).length, 0); + t.equal(Object.keys(target.variables).length, 1); + t.equal(target.variables['mock var id'].name, 'a mock variable'); + + + vm.setVariableValue(target.id, 'mock var id', 10); + t.equal(vm.getVariableValue(target.id, 'mock var id'), 10); + + target.blocks.createBlock(adapter(events.mockVariableBlock)[0]); + + // Verify the block exists on the target, and that it references the global variable + t.type(target.blocks.getBlock('a block'), 'object'); + t.type(target.blocks.getBlock('a block').fields, 'object'); + t.type(target.blocks.getBlock('a block').fields.VARIABLE, 'object'); + t.equal(target.blocks.getBlock('a block').fields.VARIABLE.id, 'mock var id'); + + // Verify that the block does not exist on the stage + t.type(stage.blocks.getBlock('a block'), 'undefined'); + + // Share the block to the stage + vm.shareBlocksToTarget([target.blocks.getBlock('a block')], stage.id); + + // Verify that the block still exists on the target and remains unchanged + t.type(target.blocks.getBlock('a block'), 'object'); + t.type(target.blocks.getBlock('a block').fields, 'object'); + t.type(target.blocks.getBlock('a block').fields.VARIABLE, 'object'); + t.equal(target.blocks.getBlock('a block').fields.VARIABLE.id, 'mock var id'); + + t.type(stage.blocks.getBlock('a block'), 'object'); + t.type(stage.blocks.getBlock('a block').fields, 'object'); + t.type(stage.blocks.getBlock('a block').fields.VARIABLE, 'object'); + t.equal(stage.blocks.getBlock('a block').fields.VARIABLE.id, 'StageVarFromLocal_mock var id'); + + // Verify that a new global variable was created, the old one still exists on + // the target and still has the same name and value, and the new one has + // a new name and value 0. + t.equal(Object.keys(target.variables).length, 1); + t.equal(target.variables['mock var id'].name, 'a mock variable'); + t.equal(vm.getVariableValue(target.id, 'mock var id'), 10); + + // Verify that a new variable was created on the stage, with a new name and new id + t.equal(Object.keys(stage.variables).length, 1); + t.type(stage.variables['mock var id'], 'undefined'); + const newGlobalVar = Object.values(stage.variables)[0]; + t.equal(newGlobalVar.name, 'Stage: a mock variable'); + const newId = newGlobalVar.id; + t.notEqual(newId, 'mock var id'); + t.equals(vm.getVariableValue(stage.id, newId), 0); + + t.end(); +}); + +test('shareBlocksToTarget chooses a fresh name for a new global variable checking for conflicts on all sprites', t => { + const vm = new VirtualMachine(); + const runtime = vm.runtime; + const spr1 = new Sprite(null, runtime); + const stage = spr1.createClone(); + stage.isStage = true; + + const spr2 = new Sprite(null, runtime); + const target = spr2.createClone(); + + const spr3 = new Sprite(null, runtime); + const otherTarget = spr3.createClone(); + + runtime.targets = [stage, target, otherTarget]; + vm.editingTarget = target; + vm.runtime.setEditingTarget(target); + + target.createVariable('mock var id', 'a mock variable', Variable.SCALAR_TYPE); + t.equal(Object.keys(stage.variables).length, 0); + t.equal(Object.keys(target.variables).length, 1); + t.equal(target.variables['mock var id'].name, 'a mock variable'); + + + vm.setVariableValue(target.id, 'mock var id', 10); + t.equal(vm.getVariableValue(target.id, 'mock var id'), 10); + + target.blocks.createBlock(adapter(events.mockVariableBlock)[0]); + + // Verify the block exists on the target, and that it references the global variable + t.type(target.blocks.getBlock('a block'), 'object'); + t.type(target.blocks.getBlock('a block').fields, 'object'); + t.type(target.blocks.getBlock('a block').fields.VARIABLE, 'object'); + t.equal(target.blocks.getBlock('a block').fields.VARIABLE.id, 'mock var id'); + + // Verify that the block does not exist on the stage + t.type(stage.blocks.getBlock('a block'), 'undefined'); + + // Create a variable that conflicts with what will be the new name for the + // new global variable to ensure a fresh name is chosen + otherTarget.createVariable('a different var', 'Stage: a mock variable', Variable.SCALAR_TYPE); + + // Share the block to the stage + vm.shareBlocksToTarget([target.blocks.getBlock('a block')], stage.id); + + // Verify that the block still exists on the target and remains unchanged + t.type(target.blocks.getBlock('a block'), 'object'); + t.type(target.blocks.getBlock('a block').fields, 'object'); + t.type(target.blocks.getBlock('a block').fields.VARIABLE, 'object'); + t.equal(target.blocks.getBlock('a block').fields.VARIABLE.id, 'mock var id'); + + t.type(stage.blocks.getBlock('a block'), 'object'); + t.type(stage.blocks.getBlock('a block').fields, 'object'); + t.type(stage.blocks.getBlock('a block').fields.VARIABLE, 'object'); + t.equal(stage.blocks.getBlock('a block').fields.VARIABLE.id, 'StageVarFromLocal_mock var id'); + + // Verify that a new global variable was created, the old one still exists on + // the target and still has the same name and value, and the new one has + // a new name and value 0. + t.equal(Object.keys(target.variables).length, 1); + t.equal(target.variables['mock var id'].name, 'a mock variable'); + t.equal(vm.getVariableValue(target.id, 'mock var id'), 10); + + // Verify that a new variable was created on the stage, with a new name and new id + t.equal(Object.keys(stage.variables).length, 1); + t.type(stage.variables['mock var id'], 'undefined'); + const newGlobalVar = Object.values(stage.variables)[0]; + t.equal(newGlobalVar.name, 'Stage: a mock variable2'); + const newId = newGlobalVar.id; + t.notEqual(newId, 'mock var id'); + t.equals(vm.getVariableValue(stage.id, newId), 0); + + t.end(); +});