Tests

blocks/procedures.js

@@ -271,7 +271,7 @@ Blockly.Blocks['procedures_defnoreturn'] = {
       return vars;
     }
     for (var i = 0, argName; argName = this.arguments_[i]; i++) {
-      // TODO (#1199): When we switch to tracking variables by ID,
+      // TODO (#1494): When we switch to tracking procedure arguments by ID,
       // update this.
       var model = this.workspace.getVariable(argName, '');
       if (model) {

core/block.js

@@ -695,12 +695,7 @@ Blockly.Block.prototype.getVarModels = function() {
   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.getVariableById(field.getValue());
-        if (model) {
-          vars.push(model);
-        }
+        vars.push(this.workspace.getVariableById(field.getValue()));
       }
     }
   }

tests/jsunit/test_utilities.js

@@ -108,3 +108,17 @@ function createMockBlock(variable_id) {
   Blockly.Events.enable();
   return block;
 }
+
+function createTwoVariablesAndBlocks(workspace) {
+  // Create two variables of different types.
+  workspace.createVariable('name1', 'type1', 'id1');
+  workspace.createVariable('name2', 'type2', 'id2');
+  // Create blocks to refer to both of them.
+  createMockBlock('id1');
+  createMockBlock('id2');
+}
+
+function createVariableAndBlock(workspace) {
+  workspace.createVariable('name1', 'type1', 'id1');
+  createMockBlock('id1');
+}

tests/jsunit/workspace_test.js

@@ -247,15 +247,12 @@ function test_renameVariable_ReferenceExists() {
   // Test renaming a variable when a reference to it exists.
   // Expect 'renameVariable' to change oldName variable name to newName.
   workspaceTest_setUp();
-  var id = 'id1';
-  var type = 'type1';
-  var oldName = 'name1';
   var newName = 'name2';
-  workspace.createVariable(oldName, type, id);
-  createMockBlock(id);
 
-  workspace.renameVariableById(id, newName);
-  checkVariableValues(workspace, newName, type, id);
+  createVariableAndBlock(workspace);
+
+  workspace.renameVariableById('id1', newName);
+  checkVariableValues(workspace, newName, 'type1', 'id1');
   // Renaming should not have created a new variable.
   assertEquals(1, workspace.getAllVariables().length);
   var block_var_name = workspace.topBlocks_[0].getVarModels()[0].name;
@@ -302,24 +299,13 @@ function test_renameVariable_TwoVariablesDifferentType() {
   // Expect the rename to succeed, because variables with different types are
   // allowed to have the same name.
   workspaceTest_setUp();
-  var id1 = 'id1';
-  var id2 = 'id2';
-  var type1 = 'type1';
-  var type2 = 'type2';
+  createTwoVariablesAndBlocks(workspace);
 
-  var oldName = 'name1';
   var newName = 'name2';
-  // Create two variables of different types.
-  workspace.createVariable(oldName, type1, id1);
-  workspace.createVariable(newName, type2, id2);
-  // Create blocks to refer to both of them.
-  createMockBlock(id1);
-  createMockBlock(id2);
+  workspace.renameVariableById('id1', newName);
 
-  workspace.renameVariableById(id1, newName);
-
-  checkVariableValues(workspace, newName, type1, id1);
-  checkVariableValues(workspace, newName, type2, id2);
+  checkVariableValues(workspace, newName, 'type1', 'id1');
+  checkVariableValues(workspace, newName, 'type2', 'id2');
 
   // References shoul have the correct names.
   var block_var_name_1 = workspace.topBlocks_[0].getVarModels()[0].name;
@@ -333,17 +319,14 @@ function test_renameVariable_TwoVariablesDifferentType() {
 function test_renameVariable_OldCase() {
   // Rename a variable with a single reference.  Update only the capitalization.
   workspaceTest_setUp();
-  var oldCase = 'Name1';
-  var newName = 'name1';
-  var type = 'type1';
-  var id = 'id1';
-  workspace.createVariable(oldCase, type, id);
-  createMockBlock(id);
+  var newName = 'Name1';
 
-  workspace.renameVariableById(id, newName);
-  checkVariableValues(workspace, newName, type, id);
-  var variable = workspace.getVariableById(id);
-  assertNotEquals(oldCase, id.name);
+  createVariableAndBlock(workspace);
+
+  workspace.renameVariableById('id1', newName);
+  checkVariableValues(workspace, newName, 'type1', 'id1');
+  var variable = workspace.getVariableById('id1');
+  assertNotEquals('name1', variable.name);
   workspaceTest_tearDown();
 }
 
@@ -389,10 +372,7 @@ function test_renameVariable_TwoVariablesAndOldCase() {
 
 function test_deleteVariableById_Trivial() {
   workspaceTest_setUp();
-  workspace.createVariable('name1', 'type1', 'id1');
-  workspace.createVariable('name2', 'type2', 'id2');
-  createMockBlock('id1');
-  createMockBlock('id2');
+  createTwoVariablesAndBlocks(workspace);
 
   workspace.deleteVariableById('id1');
   checkVariableValues(workspace, 'name2', 'type2', 'id2');

tests/jsunit/workspace_undo_redo_test.js

@@ -82,10 +82,14 @@ function createTwoVarsEmptyType() {
   workspace.createVariable('name2', '', 'id2');
 }
 
-function test_undoCreateVariable_Trivial() {
-  undoRedoTest_setUp();
+function createTwoVarsDifferentTypes() {
   workspace.createVariable('name1', 'type1', 'id1');
   workspace.createVariable('name2', 'type2', 'id2');
+}
+
+function test_undoCreateVariable_Trivial() {
+  undoRedoTest_setUp();
+  createTwoVarsDifferentTypes();
 
   workspace.undo();
   checkVariableValues(workspace, 'name1', 'type1', 'id1');
@@ -98,8 +102,7 @@ function test_undoCreateVariable_Trivial() {
 
 function test_redoAndUndoCreateVariable_Trivial() {
   undoRedoTest_setUp();
-  workspace.createVariable('name1', 'type1', 'id1');
-  workspace.createVariable('name2', 'type2', 'id2');
+  createTwoVarsDifferentTypes();
 
   workspace.undo();
   workspace.undo(true);
@@ -120,8 +123,7 @@ function test_redoAndUndoCreateVariable_Trivial() {
 
 function test_undoDeleteVariable_NoBlocks() {
   undoRedoTest_setUp();
-  workspace.createVariable('name1', 'type1', 'id1');
-  workspace.createVariable('name2', 'type2', 'id2');
+  createTwoVarsDifferentTypes();
   workspace.deleteVariableById('id1');
   workspace.deleteVariableById('id2');
 
@@ -137,10 +139,9 @@ function test_undoDeleteVariable_NoBlocks() {
 
 function test_undoDeleteVariable_WithBlocks() {
   undoRedoTest_setUp();
-  workspace.createVariable('name1', 'type1', 'id1');
-  workspace.createVariable('name2', 'type2', 'id2');
-  createMockBlock('id1');
-  createMockBlock('id2');
+
+  createTwoVariablesAndBlocks(workspace);
+
   workspace.deleteVariableById('id1');
   workspace.deleteVariableById('id2');
 
@@ -159,8 +160,9 @@ function test_undoDeleteVariable_WithBlocks() {
 
 function test_redoAndUndoDeleteVariable_NoBlocks() {
   undoRedoTest_setUp();
-  workspace.createVariable('name1', 'type1', 'id1');
-  workspace.createVariable('name2', 'type2', 'id2');
+
+  createTwoVarsDifferentTypes();
+
   workspace.deleteVariableById('id1');
   workspace.deleteVariableById('id2');
 
@@ -181,10 +183,9 @@ function test_redoAndUndoDeleteVariable_NoBlocks() {
 
 function test_redoAndUndoDeleteVariable_WithBlocks() {
   undoRedoTest_setUp();
-  workspace.createVariable('name1', 'type1', 'id1');
-  workspace.createVariable('name2', 'type2', 'id2');
-  createMockBlock('id1');
-  createMockBlock('id2');
+
+  createTwoVariablesAndBlocks(workspace);
+
   workspace.deleteVariableById('id1');
   workspace.deleteVariableById('id2');
 
@@ -261,24 +262,6 @@ function test_redoAndUndoDeleteVariableTwice_WithBlocks() {
   undoRedoTest_tearDown();
 }
 
-// TODO: Decide if this needs to be possible.
-// function test_undoRedoRenameVariable_NeitherVariableExists() {
-//   // Expect that a variable with the name, 'name2', and the generated UUID,
-//   // 'id2', to be created when rename is called. Undo removes this variable
-//   // and redo recreates it.
-//   undoRedoTest_setUp();
-//   setUpMockMethod(mockControl_, Blockly.utils, 'genUid', null,
-//     ['rename_group', 'id2', 'delete_group']);
-//   workspace.renameVariable('name1', 'name2');
-
-//   workspace.undo();
-//   assertNull(workspace.getVariableById('id2'));
-
-//   workspace.undo(true);
-//   checkVariableValues(workspace, 'name2', '', 'id2');
-//   undoRedoTest_tearDown();
-// }
-
 function test_undoRedoRenameVariable_OneExists_NoBlocks() {
   undoRedoTest_setUp();
   workspace.createVariable('name1', '', 'id1');