From 399f1a5c11db3f56652545ae32b96777c50fd1db Mon Sep 17 00:00:00 2001 From: Rachel Fenichel Date: Wed, 6 Dec 2017 15:49:20 -0800 Subject: [PATCH] Tests --- blocks/procedures.js | 2 +- core/block.js | 7 +--- tests/jsunit/test_utilities.js | 14 +++++++ tests/jsunit/workspace_test.js | 52 ++++++++---------------- tests/jsunit/workspace_undo_redo_test.js | 51 ++++++++--------------- 5 files changed, 49 insertions(+), 77 deletions(-) diff --git a/blocks/procedures.js b/blocks/procedures.js index d8fc6f30a..a6800f934 100644 --- a/blocks/procedures.js +++ b/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) { diff --git a/core/block.js b/core/block.js index d55b508c6..5080c3b37 100644 --- a/core/block.js +++ b/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())); } } } diff --git a/tests/jsunit/test_utilities.js b/tests/jsunit/test_utilities.js index d7f6085bf..aa5840d9e 100644 --- a/tests/jsunit/test_utilities.js +++ b/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'); +} diff --git a/tests/jsunit/workspace_test.js b/tests/jsunit/workspace_test.js index 999aa81f8..e991106a3 100644 --- a/tests/jsunit/workspace_test.js +++ b/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'); diff --git a/tests/jsunit/workspace_undo_redo_test.js b/tests/jsunit/workspace_undo_redo_test.js index 092dbb4a8..f014c8ba2 100644 --- a/tests/jsunit/workspace_undo_redo_test.js +++ b/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');