From 85c4e4129f405dd882c17feae65929951ab72b4c Mon Sep 17 00:00:00 2001 From: Beka Westberg Date: Thu, 13 Jun 2019 09:59:32 -0700 Subject: [PATCH 1/6] Added procedure tests. --- blocks/procedures.js | 39 ++- tests/mocha/index.html | 2 + tests/mocha/procedures_test.js | 421 +++++++++++++++++++++++++++++++++ 3 files changed, 455 insertions(+), 7 deletions(-) create mode 100644 tests/mocha/procedures_test.js diff --git a/blocks/procedures.js b/blocks/procedures.js index 1c29e7583..c14fab60d 100644 --- a/blocks/procedures.js +++ b/blocks/procedures.js @@ -164,7 +164,7 @@ Blockly.Blocks['procedures_defnoreturn'] = { * @this Blockly.Block */ decompose: function(workspace) { - var containerBlock = workspace.newBlock('procedures_mutatorcontainer'); + /*var containerBlock = workspace.newBlock('procedures_mutatorcontainer'); containerBlock.initSvg(); // Check/uncheck the allow statement box. @@ -185,7 +185,36 @@ Blockly.Blocks['procedures_defnoreturn'] = { paramBlock.oldLocation = i; connection.connect(paramBlock.previousConnection); connection = paramBlock.nextConnection; + }*/ + + var xml = Blockly.Xml.textToDom( + '' + + ' ' + + ' ' + + ' ' + + '').children[0]; + var node = xml.getElementsByTagName('statement')[0]; + for (var i = 0; i < this.arguments_.length; i++) { + node.appendChild(Blockly.Xml.textToDom( + '' + + ' ' + + ' ' + this.arguments_[i] + '' + + ' ' + + ' ' + + '' + ).children[0]); + node = node.getElementsByTagName('xml'); } + + var containerBlock = Blockly.Xml.domToBlock(xml, workspace); + + if (this.type == 'procedure_defreturn') { + containerBlock.setFieldValue( + this.hasStatements_ ? 'TRUE' : 'FALSE', 'STATEMENTS'); + } else { + containerBlock.removeInput('STATEMENT_INPUT'); + } + // Initialize procedure's callers with blank IDs. Blockly.Procedures.mutateCallers(this); return containerBlock; @@ -205,11 +234,7 @@ Blockly.Blocks['procedures_defnoreturn'] = { var varName = paramBlock.getFieldValue('NAME'); this.arguments_.push(varName); var variable = this.workspace.getVariable(varName, ''); - if (variable != null) { - this.argumentVarModels_.push(variable); - } else { - console.log('Failed to get variable named ' + varName + ', ignoring.'); - } + this.argumentVarModels_.push(variable); this.paramIds_.push(paramBlock.id); paramBlock = paramBlock.nextConnection && @@ -527,7 +552,6 @@ Blockly.Blocks['procedures_mutatorcontainer'] = { } }; - Blockly.Blocks['procedures_mutatorarg'] = { /** * Mutator block for procedure argument. @@ -591,6 +615,7 @@ Blockly.Blocks['procedures_mutatorarg'] = { var model = outerWs.getVariable(varName, ''); if (model && model.name != varName) { // Rename the variable (case change) + // TODO: This function doesn't exist on the workspace. outerWs.renameVarById(model.getId(), varName); } if (!model) { diff --git a/tests/mocha/index.html b/tests/mocha/index.html index 735eaa0bc..ba0ce0789 100644 --- a/tests/mocha/index.html +++ b/tests/mocha/index.html @@ -21,8 +21,10 @@ + + diff --git a/tests/mocha/procedures_test.js b/tests/mocha/procedures_test.js new file mode 100644 index 000000000..944a7b980 --- /dev/null +++ b/tests/mocha/procedures_test.js @@ -0,0 +1,421 @@ +/** + * @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. + */ + +goog.require('Blockly.Blocks.procedures'); +goog.require('Blockly.Msg.en'); + +suite('Procedures', function() { + setup(function() { + Blockly.setTheme(new Blockly.Theme({ + "procedure_blocks": { + "colourPrimary": "290" + } + })); + this.workspace = new Blockly.Workspace(); + + this.callForAllTypes = function(func, startName) { + var typesArray = [ + ['procedures_defnoreturn', 'procedures_callnoreturn'], + ['procedures_defreturn', 'procedures_callreturn'] + ]; + + for (var i = 0, types; types = typesArray[i]; i++) { + var context = Object.create(null); + context.workspace = this.workspace; + context.defType = types[0]; + context.callType = types[1]; + + context.defBlock = new Blockly.Block(this.workspace, types[0]); + context.defBlock.setFieldValue(startName, 'NAME'); + context.callBlock = new Blockly.Block(this.workspace, types[1]); + context.callBlock.setFieldValue(startName, 'NAME'); + func.call(context); + context.defBlock.dispose(); + context.callBlock.dispose(); + } + }; + }); + teardown(function() { + this.workspace.dispose(); + }); + + suite('isNameUsed', function() { + test('No Blocks', function() { + chai.assert.isFalse( + Blockly.Procedures.isNameUsed('name1', this.workspace) + ); + }); + test('True', function() { + this.callForAllTypes(function() { + chai.assert.isTrue( + Blockly.Procedures.isNameUsed('name1', this.workspace) + ); + }, 'name1'); + }); + test('False', function() { + this.callForAllTypes(function() { + chai.assert.isFalse( + Blockly.Procedures.isNameUsed('name2', this.workspace) + ); + }, 'name1'); + }); + }); + suite('rename', function() { + test('Simple, Programmatic', function() { + this.callForAllTypes(function() { + this.defBlock.setFieldValue( + this.defBlock.getFieldValue('NAME') + '2', + 'NAME' + ); + chai.assert.equal(this.defBlock.getFieldValue('NAME'), 'start name2'); + chai.assert.equal(this.callBlock.getFieldValue('NAME'), 'start name2'); + }, 'start name'); + }); + test('Simple, Input', function() { + this.callForAllTypes(function() { + var defInput = this.defBlock.getField('NAME'); + defInput.htmlInput_ = Object.create(null); + defInput.htmlInput_.oldValue_ = 'start name'; + defInput.htmlInput_.untypedDefaultValue_ = 'start name'; + + defInput.htmlInput_.value = defInput.htmlInput_.oldValue_ + '2'; + defInput.onHtmlInputChange_(null); + chai.assert.equal(this.defBlock.getFieldValue('NAME'), 'start name2'); + chai.assert.equal(this.callBlock.getFieldValue('NAME'), 'start name2'); + }, 'start name'); + }); + test('lower -> CAPS', function() { + this.callForAllTypes(function() { + var defInput = this.defBlock.getField('NAME'); + defInput.htmlInput_ = Object.create(null); + defInput.htmlInput_.oldValue_ = 'start name'; + defInput.htmlInput_.untypedDefaultValue_ = 'start name'; + + defInput.htmlInput_.value = 'START NAME'; + defInput.onHtmlInputChange_(null); + chai.assert.equal(this.defBlock.getFieldValue('NAME'), 'START NAME'); + chai.assert.equal(this.callBlock.getFieldValue('NAME'), 'START NAME'); + }, 'start name'); + }); + test('CAPS -> lower', function() { + this.callForAllTypes(function() { + var defInput = this.defBlock.getField('NAME'); + defInput.htmlInput_ = Object.create(null); + defInput.htmlInput_.oldValue_ = 'START NAME'; + defInput.htmlInput_.untypedDefaultValue_ = 'START NAME'; + + defInput.htmlInput_.value = 'start name'; + defInput.onHtmlInputChange_(null); + console.log(this.defType); + chai.assert.equal(this.defBlock.getFieldValue('NAME'), 'start name'); + chai.assert.equal(this.callBlock.getFieldValue('NAME'), 'start name'); + }, 'START NAME'); + }); + test('Whitespace', function() { + this.callForAllTypes(function() { + var defInput = this.defBlock.getField('NAME'); + defInput.htmlInput_ = Object.create(null); + defInput.htmlInput_.oldValue_ = 'start name'; + defInput.htmlInput_.untypedDefaultValue_ = 'start name'; + + defInput.htmlInput_.value = defInput.htmlInput_.oldValue_ + ' '; + defInput.onHtmlInputChange_(null); + chai.assert.equal(this.defBlock.getFieldValue('NAME'), 'start name'); + chai.assert.equal(this.callBlock.getFieldValue('NAME'), 'start name'); + }, 'start name'); + }); + // TODO: Simple fix, oldText needs to be trimmed too, or we just discard + // the checking (params don't check). + test.skip('Whitespace then Text', function() { + this.callForAllTypes(function() { + var defInput = this.defBlock.getField('NAME'); + defInput.htmlInput_ = Object.create(null); + defInput.htmlInput_.oldValue_ = 'start name'; + defInput.htmlInput_.untypedDefaultValue_ = 'start name'; + + defInput.htmlInput_.value = defInput.htmlInput_.oldValue_ + ' '; + defInput.onHtmlInputChange_(null); + defInput.htmlInput_.value = defInput.htmlInput_.oldValue_ + '2'; + defInput.onHtmlInputChange_(null); + chai.assert.equal(this.defBlock.getFieldValue('NAME'), 'start name 2'); + chai.assert.equal(this.callBlock.getFieldValue('NAME'), 'start name 2'); + }, 'start name'); + }); + test('Set Empty', function() { + this.callForAllTypes(function() { + var defInput = this.defBlock.getField('NAME'); + defInput.htmlInput_ = Object.create(null); + defInput.htmlInput_.oldValue_ = 'start name'; + defInput.htmlInput_.untypedDefaultValue_ = 'start name'; + + defInput.htmlInput_.value = ''; + defInput.onHtmlInputChange_(null); + chai.assert.equal(this.defBlock.getFieldValue('NAME'), ''); + chai.assert.equal(this.callBlock.getFieldValue('NAME'), ''); + }, 'start name'); + }); + // TODO: Procedures do not handle having no name correctly. It is a + // problem with newly created procedure blocks having '' as the + // oldName, just like empty procedures. The renameProcedure function + // gets confused when a procedure's name is initially set. This is + // basically #503 again. + test.skip('Set Empty, and Create New', function() { + this.callForAllTypes(function() { + var defInput = this.defBlock.getField('NAME'); + defInput.htmlInput_ = Object.create(null); + defInput.htmlInput_.oldValue_ = 'start name'; + defInput.htmlInput_.untypedDefaultValue_ = 'start name'; + + defInput.htmlInput_.value = ''; + defInput.onHtmlInputChange_(null); + var newDefBlock = new Blockly.Block(this.workspace, this.defType); + newDefBlock.setFieldValue('new name', 'NAME'); + chai.assert.equal(this.defBlock.getFieldValue('NAME'), ''); + chai.assert.equal(this.callBlock.getFieldValue('NAME'), ''); + + newDefBlock.dispose(); + }, 'start name'); + }); + }); + suite('getCallers', function() { + test('Simple', function() { + this.callForAllTypes(function() { + var callers = Blockly.Procedures.getCallers('name1', this.workspace); + chai.assert.equal(callers.length, 1); + chai.assert.equal(callers[0], this.callBlock); + }, 'name1'); + }); + test('Multiple Callers', function() { + this.callForAllTypes(function() { + var caller2 = new Blockly.Block(this.workspace, this.callType); + caller2.setFieldValue('name1', 'NAME'); + var caller3 = new Blockly.Block(this.workspace, this.callType); + caller3.setFieldValue('name1', 'NAME'); + + var callers = Blockly.Procedures.getCallers('name1', this.workspace); + chai.assert.equal(callers.length, 3); + chai.assert.equal(callers[0], this.callBlock); + chai.assert.equal(callers[1], caller2); + chai.assert.equal(callers[2], caller3); + + caller2.dispose(); + caller3.dispose(); + }, 'name1'); + }); + test('Multiple Procedures', function() { + this.callForAllTypes(function() { + var def2 = new Blockly.Block(this.workspace, this.defType); + def2.setFieldValue('name2', 'NAME'); + var caller2 = new Blockly.Block(this.workspace, this.callType); + caller2.setFieldValue('name2', 'NAME'); + + var callers = Blockly.Procedures.getCallers('name1', this.workspace); + chai.assert.equal(callers.length, 1); + chai.assert.equal(callers[0], this.callBlock); + + def2.dispose(); + caller2.dispose(); + }, 'name1'); + }); + // This can happen using the trashcan to retrieve blocks. + test.skip('Call Different Case', function() { + this.callForAllTypes(function() { + this.callBlock.setFieldValue('NAME', 'NAME'); + var callers = Blockly.Procedures.getCallers('name1', this.workspace); + chai.assert.equal(callers.length, 1); + chai.assert.equal(callers[0], this.callBlock); + }, 'name'); + }); + test('Multiple Workspaces', function() { + this.callForAllTypes(function() { + var workspace = new Blockly.Workspace(); + var def2 = new Blockly.Block(workspace, this.defType); + def2.setFieldValue('name', 'NAME'); + var caller2 = new Blockly.Block(workspace, this.callType); + caller2.setFieldValue('name', 'NAME'); + + var callers = Blockly.Procedures.getCallers('name', this.workspace); + chai.assert.equal(callers.length, 1); + chai.assert.equal(callers[0], this.callBlock); + + callers = Blockly.Procedures.getCallers('name', workspace); + chai.assert.equal(callers.length, 1); + chai.assert.equal(callers[0], caller2); + + def2.dispose(); + caller2.dispose(); + }, 'name'); + }); + }); + suite('getDefinition', function() { + test('Simple', function() { + this.callForAllTypes(function() { + var def = Blockly.Procedures.getDefinition('name1', this.workspace); + chai.assert.equal(def, this.defBlock); + }, 'name1'); + }); + test('Multiple Procedures', function() { + this.callForAllTypes(function() { + var def2 = new Blockly.Block(this.workspace, this.defType); + def2.setFieldValue('name2', 'NAME'); + var caller2 = new Blockly.Block(this.workspace, this.callType); + caller2.setFieldValue('name2', 'NAME'); + + var def = Blockly.Procedures.getDefinition('name1', this.workspace); + chai.assert.equal(def, this.defBlock); + + def2.dispose(); + caller2.dispose(); + }, 'name1'); + }); + test('Multiple Workspaces', function() { + this.callForAllTypes(function() { + var workspace = new Blockly.Workspace(); + var def2 = new Blockly.Block(workspace, this.defType); + def2.setFieldValue('name', 'NAME'); + var caller2 = new Blockly.Block(workspace, this.callType); + caller2.setFieldValue('name', 'NAME'); + + var def = Blockly.Procedures.getDefinition('name', this.workspace); + chai.assert.equal(def, this.defBlock); + + def = Blockly.Procedures.getDefinition('name', workspace); + chai.assert.equal(def, def2); + + def2.dispose(); + caller2.dispose(); + }, 'name'); + }); + }); + suite('Arguments', function() { + setup(function() { + this.findParentStub = sinon.stub(Blockly.Mutator, 'findParentWs') + .returns(this.workspace); + }); + teardown(function() { + this.findParentStub.restore(); + }); + suite('Untyped Arguments', function() { + function createMutator(argArray) { + this.mutatorWorkspace = new Blockly.Workspace(); + this.containerBlock = this.defBlock.decompose(this.mutatorWorkspace); + this.connection = this.containerBlock.getInput('STACK').connection; + for (var i = 0; i < argArray.length; i++) { + this.argBlock = new Blockly.Block( + this.mutatorWorkspace, 'procedures_mutatorarg'); + this.argBlock.setFieldValue(argArray[i], 'NAME'); + this.connection.connect(this.argBlock.previousConnection); + this.connection = this.argBlock.nextConnection; + } + this.defBlock.compose(this.containerBlock); + } + function assertArgs(argArray) { + chai.assert.equal(this.defBlock.arguments_.length, argArray.length); + for (var i = 0; i < argArray.length; i++) { + chai.assert.equal(this.defBlock.arguments_[i], argArray[i]); + } + chai.assert.equal(this.callBlock.arguments_.length, argArray.length); + for (var i = 0; i < argArray.length; i++) { + chai.assert.equal(this.callBlock.arguments_[i], argArray[i]); + } + } + function clearVariables() { + // TODO: Update this for typed vars. + var variables = this.workspace.getVariablesOfType(''); + var variableMap = this.workspace.getVariableMap(); + for (var i = 0, variable; variable = variables[i]; i++) { + variableMap.deleteVariable(variable); + } + } + test('Simple Add Arg', function() { + this.callForAllTypes(function() { + var args = ['arg1']; + createMutator.call(this, args); + assertArgs.call(this, args); + clearVariables.call(this); + }, 'name'); + }); + test('Multiple Args', function() { + this.callForAllTypes(function() { + var args = ['arg1', 'arg2', 'arg3']; + createMutator.call(this, args); + assertArgs.call(this, args); + clearVariables.call(this); + }, 'name'); + }); + test('Simple Change Arg', function() { + this.callForAllTypes(function() { + createMutator.call(this, ['arg1']); + this.argBlock.setFieldValue('arg2', 'NAME'); + this.defBlock.compose(this.containerBlock); + assertArgs.call(this, ['arg2']); + clearVariables.call(this); + }, 'name'); + }); + test.skip('lower -> CAPS', function() { + this.callForAllTypes(function() { + createMutator.call(this, ['arg']); + this.argBlock.setFieldValue('ARG', 'NAME'); + this.defBlock.compose(this.containerBlock); + assertArgs.call(this, ['ARG']); + clearVariables.call(this); + }, 'name'); + }); + test.skip('CAPS -> lower', function() { + this.callForAllTypes(function() { + createMutator.call(this, ['ARG']); + this.argBlock.setFieldValue('arg', 'NAME'); + this.defBlock.compose(this.containerBlock); + assertArgs.call(this, ['arg']); + clearVariables.call(this); + }, 'name'); + }); + test('Set Arg Empty (#1958)', function() { + this.callForAllTypes(function() { + var args = ['arg1']; + createMutator.call(this, args); + this.argBlock.setFieldValue('', 'NAME'); + this.defBlock.compose(this.containerBlock); + assertArgs.call(this, args); + clearVariables.call(this); + }, 'name'); + }); + test('Whitespace', function() { + this.callForAllTypes(function() { + var args = ['arg1']; + createMutator.call(this, args); + this.argBlock.setFieldValue(' ', 'NAME'); + this.defBlock.compose(this.containerBlock); + assertArgs.call(this, args); + clearVariables.call(this); + }, 'name'); + }); + test('Whitespace and Text', function() { + this.callForAllTypes(function() { + createMutator.call(this, ['arg1']); + this.argBlock.setFieldValue(' text ', 'NAME'); + this.defBlock.compose(this.containerBlock); + assertArgs.call(this, ['text']); + clearVariables.call(this); + }, 'name'); + }); + }); + }); +}); From 942c7c2a38a1ec341cedc4462334906cc119a013 Mon Sep 17 00:00:00 2001 From: Beka Westberg Date: Thu, 13 Jun 2019 16:39:19 -0700 Subject: [PATCH 2/6] Deleted inaccurate jsunit procedure tests. --- tests/jsunit/index.html | 1 - tests/jsunit/procedures_test.js | 84 --------------------------------- 2 files changed, 85 deletions(-) delete mode 100644 tests/jsunit/procedures_test.js diff --git a/tests/jsunit/index.html b/tests/jsunit/index.html index aa61c8f3f..038d20faf 100644 --- a/tests/jsunit/index.html +++ b/tests/jsunit/index.html @@ -25,7 +25,6 @@ - diff --git a/tests/jsunit/procedures_test.js b/tests/jsunit/procedures_test.js deleted file mode 100644 index 9d1468af2..000000000 --- a/tests/jsunit/procedures_test.js +++ /dev/null @@ -1,84 +0,0 @@ -/** - * @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.getField('NAME').getText(), [], 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(); - workspace.createVariable('name2', '', 'id2'); - var block = new Blockly.Block(workspace, 'procedure_mock_block'); - block.setFieldValue('id2', 'NAME'); - - var result = Blockly.Procedures.isNameUsed('name1', workspace); - assertFalse(result); - proceduresTest_tearDownWithMockBlocks(); -} - -function test_isNameUsed_True() { - proceduresTest_setUpWithMockBlocks(); - workspace.createVariable('name1', '', 'id1'); - var block = new Blockly.Block(workspace, 'procedure_mock_block'); - block.setFieldValue('id1', 'NAME'); - - var result = Blockly.Procedures.isNameUsed('name1', workspace); - assertTrue(result); - proceduresTest_tearDownWithMockBlocks(); -} From 2620a5092ec66c25b7d30c021807a37a8a519a73 Mon Sep 17 00:00:00 2001 From: Beka Westberg Date: Thu, 13 Jun 2019 16:54:37 -0700 Subject: [PATCH 3/6] Added message.js to mocha tests file. --- tests/mocha/index.html | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/mocha/index.html b/tests/mocha/index.html index ba0ce0789..7841f2af1 100644 --- a/tests/mocha/index.html +++ b/tests/mocha/index.html @@ -22,6 +22,7 @@ + From 427291c05de6bbd50c6653426f91e7c7d58a7ad3 Mon Sep 17 00:00:00 2001 From: Beka Westberg Date: Fri, 14 Jun 2019 09:41:40 -0700 Subject: [PATCH 4/6] Added skipping bad variable test. --- tests/mocha/field_variable_test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/mocha/field_variable_test.js b/tests/mocha/field_variable_test.js index d38117e85..0bfa75b45 100644 --- a/tests/mocha/field_variable_test.js +++ b/tests/mocha/field_variable_test.js @@ -68,7 +68,7 @@ suite('Variable Fields', function() { sinon.restore(); }); - test('Dropdown contains variables', function() { + test.skip('Dropdown contains variables', function() { // Expect that the dropdown options will contain the variables that exist this.workspace.createVariable('name1', '', 'id1'); this.workspace.createVariable('name2', '', 'id2'); From 347519b2a11f25fbe6484c9f5d5b676452a7d6f3 Mon Sep 17 00:00:00 2001 From: Beka Westberg Date: Fri, 14 Jun 2019 09:43:17 -0700 Subject: [PATCH 5/6] Added all block files to mocha index. --- tests/mocha/index.html | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/tests/mocha/index.html b/tests/mocha/index.html index 7841f2af1..0c731e0cf 100644 --- a/tests/mocha/index.html +++ b/tests/mocha/index.html @@ -6,6 +6,16 @@ + + + + + + + + + + @@ -21,8 +31,6 @@ - - From b34ca2f93b83f6bd36cc60d247921e4bac6503aa Mon Sep 17 00:00:00 2001 From: Beka Westberg Date: Tue, 25 Jun 2019 15:07:07 -0700 Subject: [PATCH 6/6] Fixed review comments. --- blocks/procedures.js | 43 ++++++------------------------ tests/mocha/field_variable_test.js | 4 +-- tests/mocha/index.html | 8 ------ tests/mocha/procedures_test.js | 22 ++++++++++----- 4 files changed, 25 insertions(+), 52 deletions(-) diff --git a/blocks/procedures.js b/blocks/procedures.js index c14fab60d..45fd1709b 100644 --- a/blocks/procedures.js +++ b/blocks/procedures.js @@ -164,45 +164,18 @@ Blockly.Blocks['procedures_defnoreturn'] = { * @this Blockly.Block */ decompose: function(workspace) { - /*var containerBlock = workspace.newBlock('procedures_mutatorcontainer'); - containerBlock.initSvg(); - - // Check/uncheck the allow statement box. - if (this.getInput('RETURN')) { - containerBlock.setFieldValue( - this.hasStatements_ ? 'TRUE' : 'FALSE', 'STATEMENTS'); - } else { - containerBlock.removeInput('STATEMENT_INPUT'); - } - - // Parameter list. - var connection = containerBlock.getInput('STACK').connection; - for (var i = 0; i < this.arguments_.length; i++) { - var paramBlock = workspace.newBlock('procedures_mutatorarg'); - paramBlock.initSvg(); - paramBlock.setFieldValue(this.arguments_[i], 'NAME'); - // Store the old location. - paramBlock.oldLocation = i; - connection.connect(paramBlock.previousConnection); - connection = paramBlock.nextConnection; - }*/ - var xml = Blockly.Xml.textToDom( - '' + - ' ' + - ' ' + - ' ' + - '').children[0]; + '' + + ' ' + + ''); var node = xml.getElementsByTagName('statement')[0]; for (var i = 0; i < this.arguments_.length; i++) { node.appendChild(Blockly.Xml.textToDom( - '' + - ' ' + - ' ' + this.arguments_[i] + '' + - ' ' + - ' ' + - '' - ).children[0]); + '' + + ' ' + this.arguments_[i] + '' + + ' ' + + '' + )); node = node.getElementsByTagName('xml'); } diff --git a/tests/mocha/field_variable_test.js b/tests/mocha/field_variable_test.js index 0bfa75b45..f83f06718 100644 --- a/tests/mocha/field_variable_test.js +++ b/tests/mocha/field_variable_test.js @@ -68,7 +68,7 @@ suite('Variable Fields', function() { sinon.restore(); }); - test.skip('Dropdown contains variables', function() { + test('Dropdown contains variables', function() { // Expect that the dropdown options will contain the variables that exist this.workspace.createVariable('name1', '', 'id1'); this.workspace.createVariable('name2', '', 'id2'); @@ -81,7 +81,7 @@ suite('Variable Fields', function() { fieldVariable); // Expect three variable options and a rename option. - assertEquals(result_options.length, 4); + assertEquals(result_options.length, 5); isEqualArrays(result_options[0], ['name1', 'id1']); isEqualArrays(result_options[1], ['name2', 'id2']); isEqualArrays(result_options[2], ['name3', 'id3']); diff --git a/tests/mocha/index.html b/tests/mocha/index.html index 0c731e0cf..0025120a1 100644 --- a/tests/mocha/index.html +++ b/tests/mocha/index.html @@ -7,14 +7,6 @@ - - - - - - - - diff --git a/tests/mocha/procedures_test.js b/tests/mocha/procedures_test.js index 944a7b980..4b73fcfca 100644 --- a/tests/mocha/procedures_test.js +++ b/tests/mocha/procedures_test.js @@ -2,7 +2,7 @@ * @license * Blockly Tests * - * Copyright 2017 Google Inc. + * Copyright 2019 Google Inc. * https://developers.google.com/blockly/ * * Licensed under the Apache License, Version 2.0 (the "License"); @@ -42,9 +42,9 @@ suite('Procedures', function() { context.defType = types[0]; context.callType = types[1]; - context.defBlock = new Blockly.Block(this.workspace, types[0]); + context.defBlock = new Blockly.Block(this.workspace, context.defType); context.defBlock.setFieldValue(startName, 'NAME'); - context.callBlock = new Blockly.Block(this.workspace, types[1]); + context.callBlock = new Blockly.Block(this.workspace, context.callType); context.callBlock.setFieldValue(startName, 'NAME'); func.call(context); context.defBlock.dispose(); @@ -234,10 +234,17 @@ suite('Procedures', function() { caller2.dispose(); }, 'name1'); }); - // This can happen using the trashcan to retrieve blocks. - test.skip('Call Different Case', function() { + // This can occur if you: + // 1) Create an uppercase definition and call block. + // 2) Delete both blocks. + // 3) Create a lowercase definition and call block. + // 4) Retrieve the uppercase call block from the trashcan. + // (And vise versa for creating lowercase blocks first) + // When converted to code all function names will be lowercase, so a + // caller should still be returned for a differently-cased procedure. + test('Call Different Case', function() { this.callForAllTypes(function() { - this.callBlock.setFieldValue('NAME', 'NAME'); + this.callBlock.setFieldValue('NAME1', 'NAME'); var callers = Blockly.Procedures.getCallers('name1', this.workspace); chai.assert.equal(callers.length, 1); chai.assert.equal(callers[0], this.callBlock); @@ -387,7 +394,8 @@ suite('Procedures', function() { clearVariables.call(this); }, 'name'); }); - test('Set Arg Empty (#1958)', function() { + // Test case for #1958 + test('Set Arg Empty', function() { this.callForAllTypes(function() { var args = ['arg1']; createMutator.call(this, args);