From c825e60813d850ccd453294990fe6888bc7c2e4e Mon Sep 17 00:00:00 2001 From: Rachel Fenichel Date: Wed, 29 Nov 2017 14:37:34 -0800 Subject: [PATCH] Allow variables of different types to share the same name. --- blocks/procedures.js | 18 +++++ core/block.js | 21 ++++++ core/variable_map.js | 37 +++++---- core/variables.js | 18 ++--- core/workspace.js | 55 ++++++++------ core/xml.js | 13 +++- tests/jsunit/variable_map_test.js | 36 +++++++-- tests/jsunit/workspace_test.js | 95 +++++++++++++++--------- tests/jsunit/workspace_undo_redo_test.js | 26 ++++--- tests/jsunit/xml_test.js | 6 +- 10 files changed, 214 insertions(+), 111 deletions(-) diff --git a/blocks/procedures.js b/blocks/procedures.js index 194de664c..72154eeb4 100644 --- a/blocks/procedures.js +++ b/blocks/procedures.js @@ -259,6 +259,23 @@ Blockly.Blocks['procedures_defnoreturn'] = { getVars: function() { return this.arguments_; }, + /** + * Return all variables referenced by this block. + * @return {!Array.} List of variable names. + * @this Blockly.Block + */ + getVarModels: function() { + var vars = []; + for (var i = 0, argName; argName = this.arguments_[i]; i++) { + // TODO (#1199): When we switch to tracking variables by ID, + // update this. + var model = this.workspace.getVariable(argName, ''); + if (model) { + vars.push(model); + } + } + return vars; + }, /** * Notification that a variable is renaming. * If the name matches one of this block's variables, rename it. @@ -376,6 +393,7 @@ Blockly.Blocks['procedures_defreturn'] = { return [this.getFieldValue('NAME'), this.arguments_, true]; }, getVars: Blockly.Blocks['procedures_defnoreturn'].getVars, + getVarModels: Blockly.Blocks['procedures_defnoreturn'].getVarModels, renameVar: Blockly.Blocks['procedures_defnoreturn'].renameVar, customContextMenu: Blockly.Blocks['procedures_defnoreturn'].customContextMenu, callType_: 'procedures_callreturn' diff --git a/core/block.js b/core/block.js index 11b97f1e6..570670822 100644 --- a/core/block.js +++ b/core/block.js @@ -686,6 +686,27 @@ Blockly.Block.prototype.getVars = function() { return vars; }; +/** + * Return all variables referenced by this block. + * @return {!Array.} List of variable models. + */ +Blockly.Block.prototype.getVarModels = function() { + var vars = []; + for (var i = 0, input; input = this.inputList[i]; i++) { + for (var j = 0, field; field = input.fieldRow[j]; j++) { + if (field instanceof Blockly.FieldVariable) { + // TODO (#1199): When we switch to tracking variables by ID, + // update this. + var model = this.workspace.getVariable(field.getValue(), ''); + if (model) { + vars.push(model); + } + } + } + } + return vars; +}; + /** * Notification that a variable is renaming. * If the name matches one of this block's variables, rename it. diff --git a/core/variable_map.js b/core/variable_map.js index fbf71151b..6af990a97 100644 --- a/core/variable_map.js +++ b/core/variable_map.js @@ -62,15 +62,15 @@ Blockly.VariableMap.prototype.clear = function() { * Rename the given variable by updating its name in the variable map. * @param {Blockly.VariableModel} variable Variable to rename. * @param {string} newName New variable name. + * @param {string=} opt_type The type of the variable to create if variable was + * null. */ -Blockly.VariableMap.prototype.renameVariable = function(variable, newName) { - var newVariable = this.getVariable(newName); +Blockly.VariableMap.prototype.renameVariable = function(variable, newName, + opt_type) { + var type = variable ? variable.type : (opt_type || ''); + var newVariable = this.getVariable(newName, type); var variableIndex = -1; var newVariableIndex = -1; - var type = ''; - if (variable || newVariable) { - type = (variable || newVariable).type; - } var variableList = this.getVariablesOfType(type); if (variable) { @@ -116,19 +116,14 @@ Blockly.VariableMap.prototype.renameVariable = function(variable, newName) { */ Blockly.VariableMap.prototype.createVariable = function(name, opt_type, opt_id) { - var variable = this.getVariable(name); + var variable = this.getVariable(name, opt_type); if (variable) { - if (opt_type && variable.type != opt_type) { - throw Error('Variable "' + name + '" is already in use and its type is "' - + variable.type + '" which conflicts with the passed in ' + - 'type, "' + opt_type + '".'); - } if (opt_id && variable.getId() != opt_id) { throw Error('Variable "' + name + '" is already in use and its id is "' + variable.getId() + '" which conflicts with the passed in ' + 'id, "' + opt_id + '".'); } - // The variable already exists and has the same ID and type. + // The variable already exists and has the same ID. return variable; } if (opt_id && this.getVariableById(opt_id)) { @@ -164,17 +159,19 @@ Blockly.VariableMap.prototype.deleteVariable = function(variable) { }; /** - * Find the variable by the given name and return it. Return null if it is not - * found. + * Find the variable by the given name and type and return it. Return null if + * it is not found. * @param {string} name The name to check for. + * @param {string=} opt_type The type of the variable. If not provided it + * defaults to the empty string, which is a specific type. * @return {Blockly.VariableModel} The variable with the given name, or null if * it was not found. */ -Blockly.VariableMap.prototype.getVariable = function(name) { - var keys = Object.keys(this.variableMap_); - for (var i = 0; i < keys.length; i++ ) { - var key = keys[i]; - for (var j = 0, variable; variable = this.variableMap_[key][j]; j++) { +Blockly.VariableMap.prototype.getVariable = function(name, opt_type) { + var type = opt_type || ''; + var list = this.variableMap_[type]; + if (list) { + for (var j = 0, variable; variable = list[j]; j++) { if (Blockly.Names.equals(variable.name, name)) { return variable; } diff --git a/core/variables.js b/core/variables.js index be9dcb681..f2367fd13 100644 --- a/core/variables.js +++ b/core/variables.js @@ -64,6 +64,7 @@ Blockly.Variables.allUsedVariables = function(root) { var variableHash = Object.create(null); // Iterate through every block and add each variable to the hash. for (var x = 0; x < blocks.length; x++) { + // TODO (#1199) Switch to IDs. var blockVariables = blocks[x].getVars(); if (blockVariables) { for (var y = 0; y < blockVariables.length; y++) { @@ -241,7 +242,7 @@ Blockly.Variables.createVariable = function(workspace, opt_callback, opt_type) { Blockly.Variables.promptName(Blockly.Msg.NEW_VARIABLE_TITLE, defaultName, function(text) { if (text) { - if (workspace.getVariable(text)) { + if (workspace.getVariable(text, opt_type)) { Blockly.alert(Blockly.Msg.VARIABLE_ALREADY_EXISTS.replace('%1', text.toLowerCase()), function() { @@ -281,18 +282,9 @@ Blockly.Variables.renameVariable = function(workspace, variable, Blockly.Msg.RENAME_VARIABLE_TITLE.replace('%1', variable.name), defaultName, function(newName) { if (newName) { - var newVariable = workspace.getVariable(newName); - if (newVariable && newVariable.type != variable.type) { - Blockly.alert(Blockly.Msg.VARIABLE_ALREADY_EXISTS_FOR_ANOTHER_TYPE.replace('%1', - newName.toLowerCase()).replace('%2', newVariable.type), - function() { - promptAndCheckWithAlert(newName); // Recurse - }); - } else { - workspace.renameVariable(variable.name, newName); - if (opt_callback) { - opt_callback(newName); - } + workspace.renameVariable(variable.name, newName); + if (opt_callback) { + opt_callback(newName); } } else { // User canceled prompt without a value. diff --git a/core/workspace.js b/core/workspace.js index f3c6ca4a4..c506fec02 100644 --- a/core/workspace.js +++ b/core/workspace.js @@ -239,19 +239,15 @@ Blockly.Workspace.prototype.updateVariableStore = function(clear) { * variable to rename with the given variable. * @param {?Blockly.VariableModel} variable Variable to rename. * @param {string} newName New variable name. + * @param {string=} opt_type The type of the variable to create if variable was + * null. */ Blockly.Workspace.prototype.renameVariableInternal_ = function( - variable, newName) { - var newVariable = this.getVariable(newName); + variable, newName, opt_type) { + var type = variable ? variable.type : (opt_type || ''); + var newVariable = this.getVariable(newName, type); var oldCase; - // If they are different types, throw an error. - if (variable && newVariable && variable.type != newVariable.type) { - throw Error('Variable "' + variable.name + '" is type "' + variable.type + - '" and variable "' + newName + '" is type "' + newVariable.type + - '". Both must be the same type.'); - } - // Find if newVariable case is different. if (newVariable && newVariable.name != newName) { oldCase = newVariable.name; @@ -266,7 +262,7 @@ Blockly.Workspace.prototype.renameVariableInternal_ = function( blocks[i].renameVar(oldCase, newName); } } - this.variableMap_.renameVariable(variable, newName); + this.variableMap_.renameVariable(variable, newName, type); Blockly.Events.setGroup(false); }; @@ -276,11 +272,15 @@ Blockly.Workspace.prototype.renameVariableInternal_ = function( * variable to rename with the given name. * @param {string} oldName Variable to rename. * @param {string} newName New variable name. + * @param {string=} opt_type The type of the variable. If not provided it + * defaults to the empty string, which is a specific type. */ -Blockly.Workspace.prototype.renameVariable = function(oldName, newName) { +Blockly.Workspace.prototype.renameVariable = function(oldName, newName, + opt_type) { + var type = opt_type || ''; // Warning: Prefer to use renameVariableById. - var variable = this.getVariable(oldName); - this.renameVariableInternal_(variable, newName); + var variable = this.getVariable(oldName, type); + this.renameVariableInternal_(variable, newName, opt_type); }; /** @@ -312,17 +312,25 @@ Blockly.Workspace.prototype.createVariable = function(name, opt_type, opt_id) { /** * Find all the uses of a named variable. * @param {string} name Name of variable. + * @param {string=} opt_type The type of the variable. If not provided it + * defaults to the empty string, which is a specific type. * @return {!Array.} Array of block usages. */ -Blockly.Workspace.prototype.getVariableUses = function(name) { +Blockly.Workspace.prototype.getVariableUses = function(name, opt_type) { + var type = opt_type || ''; var uses = []; var blocks = this.getAllBlocks(); // Iterate through every block and check the name. for (var i = 0; i < blocks.length; i++) { - var blockVariables = blocks[i].getVars(); + var blockVariables = blocks[i].getVarModels(); if (blockVariables) { for (var j = 0; j < blockVariables.length; j++) { - var varName = blockVariables[j]; + var varModel = blockVariables[j]; + var varName = varModel.name; + // Skip variables of the wrong type. + if (varModel.type != type) { + continue; + } // Variable name may be null if the block is only half-built. if (varName && name && Blockly.Names.equals(varName, name)) { uses.push(blocks[i]); @@ -337,10 +345,13 @@ Blockly.Workspace.prototype.getVariableUses = function(name) { * Delete a variable by the passed in name and all of its uses from this * workspace. May prompt the user for confirmation. * @param {string} name Name of variable to delete. + * @param {string=} opt_type The type of the variable. If not provided it + * defaults to the empty string, which is a specific type. */ -Blockly.Workspace.prototype.deleteVariable = function(name) { +Blockly.Workspace.prototype.deleteVariable = function(name, opt_type) { + var type = opt_type || ''; // Check whether this variable is a function parameter before deleting. - var uses = this.getVariableUses(name); + var uses = this.getVariableUses(name, type); for (var i = 0, block; block = uses[i]; i++) { if (block.type == 'procedures_defnoreturn' || block.type == 'procedures_defreturn') { @@ -354,7 +365,7 @@ Blockly.Workspace.prototype.deleteVariable = function(name) { } var workspace = this; - var variable = workspace.getVariable(name); + var variable = workspace.getVariable(name, type); if (uses.length > 1) { // Confirm before deleting multiple blocks. Blockly.confirm( @@ -423,10 +434,12 @@ Blockly.Workspace.prototype.variableIndexOf = function( * Find the variable by the given name and return it. Return null if it is not * found. * @param {!string} name The name to check for. + * @param {string=} opt_type The type of the variable. If not provided it + * defaults to the empty string, which is a specific type. * @return {?Blockly.VariableModel} the variable with the given name. */ -Blockly.Workspace.prototype.getVariable = function(name) { - return this.variableMap_.getVariable(name); +Blockly.Workspace.prototype.getVariable = function(name, opt_type) { + return this.variableMap_.getVariable(name, opt_type); }; /** diff --git a/core/xml.js b/core/xml.js index bf0d9e321..343cf8f04 100644 --- a/core/xml.js +++ b/core/xml.js @@ -713,16 +713,23 @@ Blockly.Xml.domToField_ = function(block, fieldName, xml) { return; } + var workspace = block.workspace; var text = xml.textContent; if (field instanceof Blockly.FieldVariable) { // TODO (#1199): When we change setValue and getValue to // interact with IDs instead of names, update this so that we get // the variable based on ID instead of textContent. var type = xml.getAttribute('variabletype') || ''; - var variable = block.workspace.getVariable(text); + // TODO: Consider using a different name (varID?) because this is the + // node's ID. + var id = xml.id; + if (id) { + var variable = workspace.getVariableById(id); + } else { + var variable = workspace.getVariable(text, type); + } if (!variable) { - variable = block.workspace.createVariable(text, type, - xml.getAttribute('id')); + variable = workspace.createVariable(text, type, id); } if (type != null && type !== variable.type) { throw Error('Serialized variable type with id \'' + diff --git a/tests/jsunit/variable_map_test.js b/tests/jsunit/variable_map_test.js index b2c0ccb95..568ce0b0d 100644 --- a/tests/jsunit/variable_map_test.js +++ b/tests/jsunit/variable_map_test.js @@ -43,18 +43,24 @@ function variableMapTest_tearDown() { variable_map = null; } -function test_getVariable_Trivial() { +function test_getVariable_ByNameAndType() { variableMapTest_setUp(); var var_1 = variable_map.createVariable('name1', 'type1', 'id1'); var var_2 = variable_map.createVariable('name2', 'type1', 'id2'); var var_3 = variable_map.createVariable('name3', 'type2', 'id3'); - var result_1 = variable_map.getVariable('name1'); - var result_2 = variable_map.getVariable('name2'); - var result_3 = variable_map.getVariable('name3'); + var result_1 = variable_map.getVariable('name1', 'type1'); + var result_2 = variable_map.getVariable('name2', 'type1'); + var result_3 = variable_map.getVariable('name3', 'type2'); + // Searching by name + type is correct. assertEquals(var_1, result_1); assertEquals(var_2, result_2); assertEquals(var_3, result_3); + + // Searching only by name defaults to the '' type. + assertNull(variable_map.getVariable('name1')); + assertNull(variable_map.getVariable('name2')); + assertNull(variable_map.getVariable('name3')); variableMapTest_tearDown(); } @@ -105,7 +111,7 @@ function test_createVariableAlreadyExists() { var varMapLength = variable_map.variableMap_[keys[0]].length; assertEquals(1, varMapLength); - variable_map.createVariable('name1'); + variable_map.createVariable('name1', 'type1'); checkVariableValues(variable_map, 'name1', 'type1', 'id1'); // Check that the size of the variableMap_ did not change. keys = Object.keys(variable_map.variableMap_); @@ -115,6 +121,26 @@ function test_createVariableAlreadyExists() { variableMapTest_tearDown(); } +function test_createVariableNameAlreadyExists() { + // Expect that when a variable with the same name but a different type already + // exists, the new variable is created. + variableMapTest_setUp(); + variable_map.createVariable('name1', 'type1', 'id1'); + + // Assert there is only one variable in the variable_map. + var keys = Object.keys(variable_map.variableMap_); + assertEquals(1, keys.length); + var varMapLength = variable_map.variableMap_[keys[0]].length; + assertEquals(1, varMapLength); + + variable_map.createVariable('name1', 'type2', 'id2'); + checkVariableValues(variable_map, 'name1', 'type1', 'id1'); + checkVariableValues(variable_map, 'name1', 'type2', 'id2'); + // Check that the size of the variableMap_ did change. + keys = Object.keys(variable_map.variableMap_); + assertEquals(2, keys.length); + variableMapTest_tearDown(); +} function test_createVariableNullAndUndefinedType() { variableMapTest_setUp(); variable_map.createVariable('name1', null, 'id1'); diff --git a/tests/jsunit/workspace_test.js b/tests/jsunit/workspace_test.js index a96a34ac4..59a7fa902 100644 --- a/tests/jsunit/workspace_test.js +++ b/tests/jsunit/workspace_test.js @@ -158,17 +158,19 @@ function test_getBlockById() { function test_deleteVariable_InternalTrivial() { workspaceTest_setUp(); - var var_1 = workspace.createVariable('name1', 'type1', 'id1'); - workspace.createVariable('name2', 'type2', 'id2'); + // TODO (#1199): make a similar test where the variable is given a non-empty + // type. + var var_1 = workspace.createVariable('name1', '', 'id1'); + workspace.createVariable('name2', '', 'id2'); createMockBlock('name1'); createMockBlock('name1'); createMockBlock('name2'); workspace.deleteVariableInternal_(var_1); - var variable = workspace.getVariable('name1'); + var variable = workspace.getVariable('name1', ''); var block_var_name = workspace.topBlocks_[0].getVars()[0]; assertNull(variable); - checkVariableValues(workspace, 'name2', 'type2', 'id2'); + checkVariableValues(workspace, 'name2', '', 'id2'); assertEquals('name2', block_var_name); workspaceTest_tearDown(); } @@ -207,15 +209,18 @@ function test_updateVariableStore_NameNotInvariableMap_NoClear() { function test_updateVariableStore_ClearAndAllInUse() { workspaceTest_setUp(); - workspace.createVariable('name1', 'type1', 'id1'); - workspace.createVariable('name2', 'type2', 'id2'); + workspace.createVariable('name1', '', 'id1'); + workspace.createVariable('name2', '', 'id2'); + // TODO (#1199): make a similar test where the variable is given a non-empty + // type. + // TODO (#1199): get rid of updateVariableStore if possible. setUpMockMethod(mockControl_, Blockly.Variables, 'allUsedVariables', [workspace], [['name1', 'name2']]); try { workspace.updateVariableStore(true); - checkVariableValues(workspace, 'name1', 'type1', 'id1'); - checkVariableValues(workspace, 'name2', 'type2', 'id2'); + checkVariableValues(workspace, 'name1', '', 'id1'); + checkVariableValues(workspace, 'name2', '', 'id2'); } finally { workspaceTest_tearDown(); } @@ -223,15 +228,18 @@ function test_updateVariableStore_ClearAndAllInUse() { function test_updateVariableStore_ClearAndOneInUse() { workspaceTest_setUp(); - workspace.createVariable('name1', 'type1', 'id1'); - workspace.createVariable('name2', 'type2', 'id2'); + workspace.createVariable('name1', '', 'id1'); + workspace.createVariable('name2', '', 'id2'); + // TODO (#1199): make a similar test where the variable is given a non-empty + // type. + // TODO (#1199): get rid of updateVariableStore if possible. setUpMockMethod(mockControl_, Blockly.Variables, 'allUsedVariables', [workspace], [['name1']]); try { workspace.updateVariableStore(true); - checkVariableValues(workspace, 'name1', 'type1', 'id1'); - var variabe = workspace.getVariable('name2'); + checkVariableValues(workspace, 'name1', '', 'id1'); + var variabe = workspace.getVariable('name2', ''); assertNull(variable); } finally { workspaceTest_tearDown(); @@ -302,7 +310,7 @@ function test_renameVariable_NoBlocks() { try { workspace.renameVariable(oldName, newName); checkVariableValues(workspace, 'name2', '', '1'); - var variable = workspace.getVariable(oldName); + var variable = workspace.getVariable(oldName, ''); assertNull(variable); } finally { workspaceTest_tearDown(); @@ -325,12 +333,14 @@ function test_renameVariable_OnlyOldNameBlockExists() { workspaceTest_setUp(); var oldName = 'name1'; var newName = 'name2'; - workspace.createVariable(oldName, 'type1', 'id1'); + workspace.createVariable(oldName, '', 'id1'); createMockBlock(oldName); + // TODO (#1199): make a similar test where the variable is given a non-empty + // type. workspace.renameVariable(oldName, newName); - checkVariableValues(workspace, newName, 'type1', 'id1'); - var variable = workspace.getVariable(oldName); + checkVariableValues(workspace, newName, '', 'id1'); + var variable = workspace.getVariable(oldName, ''); var block_var_name = workspace.topBlocks_[0].getVars()[0]; assertNull(variable); assertEquals(newName, block_var_name); @@ -343,13 +353,15 @@ function test_renameVariable_TwoVariablesSameType() { workspaceTest_setUp(); var oldName = 'name1'; var newName = 'name2'; - workspace.createVariable(oldName, 'type1', 'id1'); - workspace.createVariable(newName, 'type1', 'id2'); + // TODO (#1199): make a similar test where the variable is given a non-empty + // type. + workspace.createVariable(oldName, '', 'id1'); + workspace.createVariable(newName, '', 'id2'); createMockBlock(oldName); createMockBlock(newName); workspace.renameVariable(oldName, newName); - checkVariableValues(workspace, newName, 'type1', 'id2'); + checkVariableValues(workspace, newName, '', 'id2'); var variable = workspace.getVariable(oldName); var block_var_name_1 = workspace.topBlocks_[0].getVars()[0]; var block_var_name_2 = workspace.topBlocks_[1].getVars()[0]; @@ -389,12 +401,14 @@ function test_renameVariable_OldCase() { workspaceTest_setUp(); var oldCase = 'Name1'; var newName = 'name1'; - workspace.createVariable(oldCase, 'type1', 'id1'); + // TODO (#1199): make a similar test where the variable is given a non-empty + // type. + workspace.createVariable(oldCase, '', 'id1'); createMockBlock(oldCase); workspace.renameVariable(oldCase, newName); - checkVariableValues(workspace, newName, 'type1', 'id1'); - var result_oldCase = workspace.getVariable(oldCase).name; + checkVariableValues(workspace, newName, '', 'id1'); + var result_oldCase = workspace.getVariable(oldCase, '').name; assertNotEquals(oldCase, result_oldCase); workspaceTest_tearDown(); } @@ -405,14 +419,16 @@ function test_renameVariable_TwoVariablesAndOldCase() { var oldName = 'name1'; var oldCase = 'Name2'; var newName = 'name2'; - workspace.createVariable(oldName, 'type1', 'id1'); - workspace.createVariable(oldCase, 'type1', 'id2'); + // TODO (#1199): make a similar test where the variable is given a non-empty + // type. + workspace.createVariable(oldName, '', 'id1'); + workspace.createVariable(oldCase, '', 'id2'); createMockBlock(oldName); createMockBlock(oldCase); workspace.renameVariable(oldName, newName); - checkVariableValues(workspace, newName, 'type1', 'id2'); + checkVariableValues(workspace, newName, '', 'id2'); var variable = workspace.getVariable(oldName); var result_oldCase = workspace.getVariable(oldCase).name; var block_var_name_1 = workspace.topBlocks_[0].getVars()[0]; @@ -432,13 +448,15 @@ function test_renameVariableById_TwoVariablesSameType() { workspaceTest_setUp(); var oldName = 'name1'; var newName = 'name2'; - workspace.createVariable(oldName, 'type1', 'id1'); - workspace.createVariable(newName, 'type1', 'id2'); + // TODO (#1199): make a similar test where the variable is given a non-empty + // type. + workspace.createVariable(oldName, '', 'id1'); + workspace.createVariable(newName, '', 'id2'); createMockBlock(oldName); createMockBlock(newName); workspace.renameVariableById('id1', newName); - checkVariableValues(workspace, newName, 'type1', 'id2'); + checkVariableValues(workspace, newName, '', 'id2'); var variable = workspace.getVariable(oldName); var block_var_name_1 = workspace.topBlocks_[0].getVars()[0]; var block_var_name_2 = workspace.topBlocks_[1].getVars()[0]; @@ -450,14 +468,16 @@ function test_renameVariableById_TwoVariablesSameType() { function test_deleteVariable_Trivial() { workspaceTest_setUp(); - workspace.createVariable('name1', 'type1', 'id1'); - workspace.createVariable('name2', 'type1', 'id2'); + // TODO (#1199): make a similar test where the variable is given a non-empty + // type. + workspace.createVariable('name1', '', 'id1'); + workspace.createVariable('name2', '', 'id2'); createMockBlock('name1'); createMockBlock('name2'); - workspace.deleteVariable('name1'); - checkVariableValues(workspace, 'name2', 'type1', 'id2'); - var variable = workspace.getVariable('name1'); + workspace.deleteVariable('name1', ''); + checkVariableValues(workspace, 'name2', '', 'id2'); + var variable = workspace.getVariable('name1', ''); var block_var_name = workspace.topBlocks_[0].getVars()[0]; assertNull(variable); assertEquals('name2', block_var_name); @@ -466,14 +486,15 @@ function test_deleteVariable_Trivial() { function test_deleteVariableById_Trivial() { workspaceTest_setUp(); - workspace.createVariable('name1', 'type1', 'id1'); - workspace.createVariable('name2', 'type1', 'id2'); + // TODO (#1199): Make a version of this test that uses different types. + workspace.createVariable('name1', '', 'id1'); + workspace.createVariable('name2', '', 'id2'); createMockBlock('name1'); createMockBlock('name2'); workspace.deleteVariableById('id1'); - checkVariableValues(workspace, 'name2', 'type1', 'id2'); - var variable = workspace.getVariable('name1'); + checkVariableValues(workspace, 'name2', '', 'id2'); + var variable = workspace.getVariable('name1', ''); var block_var_name = workspace.topBlocks_[0].getVars()[0]; assertNull(variable); assertEquals('name2', block_var_name); diff --git a/tests/jsunit/workspace_undo_redo_test.js b/tests/jsunit/workspace_undo_redo_test.js index 33dc855d7..a8befe33c 100644 --- a/tests/jsunit/workspace_undo_redo_test.js +++ b/tests/jsunit/workspace_undo_redo_test.js @@ -146,8 +146,10 @@ function test_undoDeleteVariable_NoBlocks() { function test_undoDeleteVariable_WithBlocks() { undoRedoTest_setUp(); - workspace.createVariable('name1', 'type1', 'id1'); - workspace.createVariable('name2', 'type2', 'id2'); + // TODO (#1199): make a similar test where the variable is given a non-empty + // type. + workspace.createVariable('name1', '', 'id1'); + workspace.createVariable('name2', '', 'id2'); createMockBlock('name1'); createMockBlock('name2'); workspace.deleteVariableById('id1'); @@ -156,13 +158,13 @@ function test_undoDeleteVariable_WithBlocks() { workspace.undo(); undoRedoTest_checkBlockVariableName(0, 'name2'); assertNull(workspace.getVariableById('id1')); - checkVariableValues(workspace, 'name2', 'type2', 'id2'); + checkVariableValues(workspace, 'name2', '', 'id2'); workspace.undo(); undoRedoTest_checkBlockVariableName(0, 'name2'); undoRedoTest_checkBlockVariableName(1, 'name1'); - checkVariableValues(workspace, 'name1', 'type1', 'id1'); - checkVariableValues(workspace, 'name2', 'type2', 'id2'); + checkVariableValues(workspace, 'name1', '', 'id1'); + checkVariableValues(workspace, 'name2', '', 'id2'); undoRedoTest_tearDown(); } @@ -190,8 +192,10 @@ function test_redoAndUndoDeleteVariable_NoBlocks() { function test_redoAndUndoDeleteVariable_WithBlocks() { undoRedoTest_setUp(); - workspace.createVariable('name1', 'type1', 'id1'); - workspace.createVariable('name2', 'type2', 'id2'); + // TODO (#1199): make a similar test where the variable is given a non-empty + // type. + workspace.createVariable('name1', '', 'id1'); + workspace.createVariable('name2', '', 'id2'); createMockBlock('name1'); createMockBlock('name2'); workspace.deleteVariableById('id1'); @@ -210,7 +214,7 @@ function test_redoAndUndoDeleteVariable_WithBlocks() { // Expect that variable 'id2' is recreated undoRedoTest_checkBlockVariableName(0, 'name2'); assertNull(workspace.getVariableById('id1')); - checkVariableValues(workspace, 'name2', 'type2', 'id2'); + checkVariableValues(workspace, 'name2', '', 'id2'); undoRedoTest_tearDown(); } @@ -241,7 +245,9 @@ function test_redoAndUndoDeleteVariableTwice_NoBlocks() { function test_redoAndUndoDeleteVariableTwice_WithBlocks() { undoRedoTest_setUp(); - workspace.createVariable('name1', 'type1', 'id1'); + // TODO (#1199): make a similar test where the variable is given a non-empty + // type. + workspace.createVariable('name1', '', 'id1'); createMockBlock('name1'); workspace.deleteVariableById('id1'); workspace.deleteVariableById('id1'); @@ -255,7 +261,7 @@ function test_redoAndUndoDeleteVariableTwice_WithBlocks() { // undo delete workspace.undo(); undoRedoTest_checkBlockVariableName(0, 'name1'); - checkVariableValues(workspace, 'name1', 'type1', 'id1'); + checkVariableValues(workspace, 'name1', '', 'id1'); // redo delete workspace.undo(true); diff --git a/tests/jsunit/xml_test.js b/tests/jsunit/xml_test.js index 1a6dbce3d..0c2af7441 100644 --- a/tests/jsunit/xml_test.js +++ b/tests/jsunit/xml_test.js @@ -285,11 +285,13 @@ function test_appendDomToWorkspace() { function test_blockToDom_fieldToDom_trivial() { xmlTest_setUpWithMockBlocks(); - workspace.createVariable('name1', 'type1', 'id1'); + // TODO (#1199): make a similar test where the variable is given a non-empty + // type. + workspace.createVariable('name1', '', 'id1'); var block = new Blockly.Block(workspace, 'field_variable_test_block'); block.inputList[0].fieldRow[0].setValue('name1'); var resultFieldDom = Blockly.Xml.blockToDom(block).childNodes[0]; - xmlTest_checkVariableFieldDomValues(resultFieldDom, 'VAR', 'type1', 'id1', + xmlTest_checkVariableFieldDomValues(resultFieldDom, 'VAR', '', 'id1', 'name1'); xmlTest_tearDownWithMockBlocks(); }