From 234c53157f676978ab9de232ed28305720443003 Mon Sep 17 00:00:00 2001 From: marisaleung Date: Fri, 28 Jul 2017 10:26:02 -0700 Subject: [PATCH] Fix Blockly.Procedures.isNameUsed return values. Add tests for Procedures.isNameUsed() so this bug never happens again. --- core/procedures.js | 4 +- core/variables.js | 4 +- tests/jsunit/index.html | 1 + tests/jsunit/procedures_test.js | 83 +++++++++++++++++++++++++++++++++ 4 files changed, 88 insertions(+), 4 deletions(-) create mode 100644 tests/jsunit/procedures_test.js diff --git a/core/procedures.js b/core/procedures.js index eef9c9d2a..98ebf41b6 100644 --- a/core/procedures.js +++ b/core/procedures.js @@ -140,11 +140,11 @@ Blockly.Procedures.isNameUsed = function(name, workspace, opt_exclude) { if (blocks[i].getProcedureDef) { var procName = blocks[i].getProcedureDef(); if (Blockly.Names.equals(procName[0], name)) { - return false; + return true; } } } - return true; + return false; }; /** diff --git a/core/variables.js b/core/variables.js index 2cf00d5f4..ac952844e 100644 --- a/core/variables.js +++ b/core/variables.js @@ -248,7 +248,7 @@ Blockly.Variables.createVariable = function(workspace, opt_callback, opt_type) { promptAndCheckWithAlert(text); // Recurse }); } - else if (!Blockly.Procedures.isNameUsed(text, workspace)) { + else if (Blockly.Procedures.isNameUsed(text, workspace)) { Blockly.alert(Blockly.Msg.PROCEDURE_ALREADY_EXISTS.replace('%1', text.toLowerCase()), function() { @@ -297,7 +297,7 @@ Blockly.Variables.renameVariable = function(workspace, variable, promptAndCheckWithAlert(newName); // Recurse }); } - else if (!Blockly.Procedures.isNameUsed(newName, workspace)) { + else if (Blockly.Procedures.isNameUsed(newName, workspace)) { Blockly.alert(Blockly.Msg.PROCEDURE_ALREADY_EXISTS.replace('%1', newName.toLowerCase()), function() { diff --git a/tests/jsunit/index.html b/tests/jsunit/index.html index dd62a536a..940111ecb 100644 --- a/tests/jsunit/index.html +++ b/tests/jsunit/index.html @@ -25,6 +25,7 @@ + diff --git a/tests/jsunit/procedures_test.js b/tests/jsunit/procedures_test.js new file mode 100644 index 000000000..10003ec74 --- /dev/null +++ b/tests/jsunit/procedures_test.js @@ -0,0 +1,83 @@ +/** + * @license + * Blockly Tests + * + * Copyright 2017 Google Inc. + * https://developers.google.com/blockly/ + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + + /** + * @fileoverview Tests for procedures. + * @author marisaleung@google.com (Marisa Leung) + */ +'use strict'; + +goog.require('goog.testing'); + +var workspace; + +function proceduresTest_setUpWithMockBlocks() { + workspace = new Blockly.Workspace(); + Blockly.defineBlocksWithJsonArray([{ + getProcedureDef: function() { + }, + 'type': 'procedure_mock_block', + 'message0': '%1', + 'args0': [ + { + 'type': 'field_variable', + 'name': 'NAME', + 'variable': 'item' + } + ], + }]); + Blockly.Blocks['procedure_mock_block'].getProcedureDef = function() { + return [this.getFieldValue('NAME'), [], false]; + }; +} + +function proceduresTest_tearDownWithMockBlocks() { + workspace.dispose(); + delete Blockly.Blocks.procedures_mock_block; +} + + +function test_isNameUsed_NoBlocks() { + workspace = new Blockly.Workspace(); + var result = Blockly.Procedures.isNameUsed('name1', workspace); + assertFalse(result); + workspace.dispose(); +} + +function test_isNameUsed_False() { + proceduresTest_setUpWithMockBlocks(); + var block = new Blockly.Block(workspace, 'procedure_mock_block'); + block.setFieldValue('name2', 'NAME'); + + var result = Blockly.Procedures.isNameUsed('name1', workspace); + assertFalse(result); + proceduresTest_tearDownWithMockBlocks(); +} + +function test_isNameUsed_True() { + proceduresTest_setUpWithMockBlocks(); + var block = new Blockly.Block(workspace, 'procedure_mock_block'); + block.setFieldValue('name1', 'NAME'); + + var result = Blockly.Procedures.isNameUsed('name1', workspace); + assertTrue(result); + proceduresTest_tearDownWithMockBlocks(); +} +