diff --git a/blocks/procedures.js b/blocks/procedures.js index 565bdf058..c91364f81 100644 --- a/blocks/procedures.js +++ b/blocks/procedures.js @@ -19,7 +19,6 @@ const Events = goog.require('Blockly.Events'); const Extensions = goog.require('Blockly.Extensions'); const Procedures = goog.require('Blockly.Procedures'); const Variables = goog.require('Blockly.Variables'); -const Xml = goog.require('Blockly.Xml'); const xmlUtils = goog.require('Blockly.utils.xml'); const {Align} = goog.require('Blockly.Input'); /* eslint-disable-next-line no-unused-vars */ @@ -100,10 +99,7 @@ const blocks = createBlockDefinitionsFromJsonArray([ 'type': 'procedures_callnoreturn', 'message0': '%1 %2', 'args0': [ - { - 'type': 'field_label', - 'name': 'NAME', - }, + {'type': 'field_label', 'name': 'NAME', 'text': '%{BKY_UNNAMED_KEY}'}, { 'type': 'input_dummy', 'name': 'TOPROW', @@ -177,10 +173,7 @@ const blocks = createBlockDefinitionsFromJsonArray([ 'type': 'procedures_callreturn', 'message0': '%1 %2', 'args0': [ - { - 'type': 'field_label', - 'name': 'NAME', - }, + {'type': 'field_label', 'name': 'NAME', 'text': '%{BKY_UNNAMED_KEY}'}, { 'type': 'input_dummy', 'name': 'TOPROW', @@ -319,8 +312,9 @@ const procedureDefGetDefMixin = function() { }, }; - mixin.model_ = - new ObservableProcedureModel(this.workspace, this.getFieldValue('NAME')); + mixin.model_ = new ObservableProcedureModel( + this.workspace, + Procedures.findLegalName(this.getFieldValue('NAME'), this)); this.workspace.getProcedureMap().add(mixin.getProcedureModel()); this.mixin(mixin, true); @@ -725,7 +719,8 @@ const procedureDefMutator = { Procedures.mutateCallers(this); const model = this.getProcedureModel(); - for (let i = model.getParameters().length; i >= 0; i--) { + const count = this.getProcedureModel().getParameters().length; + for (let i = count - 1; i >= 0; i--) { model.deleteParameter(i); } @@ -996,6 +991,72 @@ Extensions.register( /** @this {Block} */ const procedureCallerGetDefMixin = function() { const mixin = { + model_: null, + + prevParams_: [], + + argsMap_: new Map(), + + /** + * @return {IProcedureModel} The procedure model associated with this + * block. + */ + getProcedureModel() { + return this.model_; + }, + + /** + * @param {string} name The name of the procedure model to find. + * @param {string[]} params The param names of the procedure model to find. + * @return {IProcedureModel} The procedure model that was found. + */ + findProcedureModel_(name, params = []) { + const workspace = this.getTargetWorkspace_(); + const model = workspace.getProcedureMap().getProcedures().find( + (proc) => proc.getName() === name); + if (!model) return null; + + const hasMatchingParams = + model.getParameters().every((p, i) => p.getName() === params[i]); + if (!hasMatchingParams) return null; + + return model; + }, + + /** + * Creates a procedure definition block with the given name and params, + * and returns the procedure model associated with it. + * @param {string} name The name of the procedure to create. + * @param {string[]} params The names of the parameters to create. + * @return {IProcedureModel} The procedure model associated with the new + * procedure definition block. + */ + createDef_(name, params = []) { + const xy = this.getRelativeToSurfaceXY(); + const newName = Procedures.findLegalName(name, this); + + const blockDef = { + 'type': this.defType_, + 'x': xy.x + config.snapRadius * (this.RTL ? -1 : 1), + 'y': xy.y + config.snapRadius * 2, + 'extraState': { + 'params': params.map((p) => ({'name': p})), + }, + 'fields': {'NAME': newName}, + }; + return serialization.blocks.append(blockDef, this.getTargetWorkspace_()) + .getProcedureModel(); + }, + + /** + * @return {Workspace} The main workspace (i.e. not the flyout workspace) + * associated with this block. + */ + getTargetWorkspace_() { + return this.workspace.isFlyout ? this.workspace.targetWorkspace : + this.workspace; + }, + /** * Returns the name of the procedure this block calls. * @return {string} Procedure name. @@ -1012,7 +1073,8 @@ const procedureCallerGetDefMixin = function() { * @this {Block} */ getVars: function() { - return this.arguments_; + return this.getProcedureModel().getParameters().map( + (p) => p.getVariableModel().name); }, /** @@ -1021,7 +1083,8 @@ const procedureCallerGetDefMixin = function() { * @this {Block} */ getVarModels: function() { - return this.argumentVarModels_; + return this.getProcedureModel().getParameters().map( + (p) => p.getVariableModel()); }, }; @@ -1043,6 +1106,8 @@ const procedureCallerMutator = { previousEnabledState_: true, + paramsFromSerializedState_: [], + /** * Create XML to represent the (non-editable) name and arguments. * Backwards compatible serialization implementation. @@ -1068,16 +1133,13 @@ const procedureCallerMutator = { */ domToMutation: function(xmlElement) { const name = xmlElement.getAttribute('name'); - this.renameProcedure(this.getProcedureCall(), name); - const args = []; - const paramIds = []; - for (let i = 0, childNode; (childNode = xmlElement.childNodes[i]); i++) { - if (childNode.nodeName.toLowerCase() === 'arg') { - args.push(childNode.getAttribute('name')); - paramIds.push(childNode.getAttribute('paramId')); + const params = []; + for (const n of xmlElement.childNodes) { + if (n.nodeName.toLowerCase() === 'arg') { + params.push(n.getAttribute('name')); } } - this.setProcedureParameters_(args, paramIds); + this.deserialize_(name, params); }, /** @@ -1100,19 +1162,160 @@ const procedureCallerMutator = { * procedure name. */ loadExtraState: function(state) { - this.renameProcedure(this.getProcedureCall(), state['name']); - const params = state['params']; - if (params) { - const ids = []; - ids.length = params.length; - ids.fill(null); - this.setProcedureParameters_(params, ids); + this.deserialize_(state['name'], state['params'] || []); + }, + + /** + * Applies the given name and params from the serialized state to the block. + * @param {string} name The name to apply to the block. + * @param {!Array} params The parameters to apply to the block. + */ + deserialize_: function(name, params) { + this.setFieldValue(name, 'NAME'); + if (!this.model_) this.model_ = this.findProcedureModel_(name, params); + if (this.getProcedureModel()) { + this.initBlockWithProcedureModel_(); + } else { + // We need to create/find our procedure def in our change listener. + this.paramsFromSerializedState_ = params; + // Create inputs based on the mutation so that children can be connected. + this.createArgInputs_(params); } + + // Temporarily maintained for logic that relies on arguments_ + this.arguments_ = params; }, }; Extensions.registerMutator('procedure_caller_mutator', procedureCallerMutator); const procedureCallerUpdateShapeMixin = { + /** + * Renders the block for the first time based on the procedure model. + */ + initBlockWithProcedureModel_() { + this.prevParams_ = [...this.getProcedureModel().getParameters()]; + this.doProcedureUpdate(); + }, + + /** + * Updates the shape of this block to reflect the state of the data model. + */ + doProcedureUpdate: function() { + if (!this.getProcedureModel()) return; + const id = this.getProcedureModel().getId(); + if (!this.getTargetWorkspace_().getProcedureMap().has(id)) { + this.dispose(); + return; + } + this.updateName_(); + this.updateEnabled_(); + this.updateParameters_(); + }, + + /** + * Updates the name field of this block to match the state of the data model. + */ + updateName_: function() { + const name = this.getProcedureModel().getName(); + this.setFieldValue(name, 'NAME'); + const baseMsg = this.outputConnection ? + Msg['PROCEDURES_CALLRETURN_TOOLTIP'] : + Msg['PROCEDURES_CALLNORETURN_TOOLTIP']; + this.setTooltip(baseMsg.replace('%1', name)); + }, + + /** + * Updates the enabled state of this block to match the state of the data + * model. + */ + updateEnabled_: function() { + if (!this.getProcedureModel().getEnabled()) { + this.previousEnabledState_ = this.isEnabled(); + this.setEnabled(false); + } else { + this.setEnabled(this.previousEnabledState_); + } + }, + + /** + * Updates the parameter fields/inputs of this block to match the state of the + * data model. + */ + updateParameters_: function() { + this.updateArgsMap_(); + this.deleteAllArgInputs_(); + this.addParametersLabel__(); + this.createArgInputs_(); + this.reattachBlocks_(); + this.prevParams_ = [...this.getProcedureModel().getParameters()]; + }, + + /** + * Saves a map of parameter IDs to target blocks attached to the inputs + * of this caller block. + */ + updateArgsMap_: function() { + for (const [i, p] of this.prevParams_.entries()) { + const target = this.getInputTargetBlock(`ARG${i}`); + if (target) this.argsMap_.set(p.getId(), target); + } + }, + + /** + * Deletes all the parameter inputs on this block. + */ + deleteAllArgInputs_: function() { + let i = 0; + while (this.getInput(`ARG${i}`)) { + this.removeInput(`ARG${i}`); + i++; + } + }, + + /** + * Adds or removes the parameter label to match the state of the data model. + */ + addParametersLabel__: function() { + const topRow = this.getInput('TOPROW'); + if (this.getProcedureModel().getParameters().length) { + if (!this.getField('WITH')) { + topRow.appendField(Msg['PROCEDURES_CALL_BEFORE_PARAMS'], 'WITH'); + topRow.init(); + } + } else if (this.getField('WITH')) { + topRow.removeField('WITH'); + } + }, + + /** + * Creates all of the parameter inputs to match the state of the data model. + * @param {Array} params The params to add to the block, or null to + * use the params defined in the procedure model. + */ + createArgInputs_: function(params = null) { + if (!params) { + params = this.getProcedureModel().getParameters().map((p) => p.getName()); + } + for (const [i, p] of params.entries()) { + this.appendValueInput(`ARG${i}`) + .appendField(new FieldLabel(p), `ARGNAME${i}`) + .setAlign(Align.RIGHT); + } + }, + + /** + * Reattaches blocks to this blocks' inputs based on the data saved in the + * argsMap_. + */ + reattachBlocks_: function() { + const params = this.getProcedureModel().getParameters(); + for (const [i, p] of params.entries()) { + if (!this.argsMap_.has(p.getId())) continue; + this.getInput(`ARG${i}`).connection.connect( + this.argsMap_.get(p.getId()).outputConnection); + } + }, + /** * Notification that a procedure is renaming. * If the name matches this block's procedure, rename it. @@ -1291,55 +1494,28 @@ const procedureCallerOnChangeMixin = { // Events not generated by user. Skip handling. return; } + // TODO: Clean this up to call createDef_. if (event.type === Events.BLOCK_CREATE && - event.ids.indexOf(this.id) !== -1) { + (event.blockId === this.id || event.ids.indexOf(this.id) !== -1)) { // Look for the case where a procedure call was created (usually through // paste) and there is no matching definition. In this case, create // an empty definition block with the correct signature. const name = this.getProcedureCall(); let def = Procedures.getDefinition(name, this.workspace); - if (def && - (def.type !== this.defType_ || - JSON.stringify(def.getVars()) !== JSON.stringify(this.arguments_))) { - // The signatures don't match. - def = null; - } + if (!this.defMatches_(def)) def = null; if (!def) { + // We have no def nor procedure model. Events.setGroup(event.group); - /** - * Create matching definition block. - * - * - * - * - * - * test - * - * - */ - const xml = xmlUtils.createElement('xml'); - const block = xmlUtils.createElement('block'); - block.setAttribute('type', this.defType_); - const xy = this.getRelativeToSurfaceXY(); - const x = xy.x + config.snapRadius * (this.RTL ? -1 : 1); - const y = xy.y + config.snapRadius * 2; - block.setAttribute('x', x); - block.setAttribute('y', y); - const mutation = this.mutationToDom(); - block.appendChild(mutation); - const field = xmlUtils.createElement('field'); - field.setAttribute('name', 'NAME'); - const callName = this.getProcedureCall(); - const newName = Procedures.findLegalName(callName, this); - if (callName !== newName) { - this.renameProcedure(callName, newName); - } - field.appendChild(xmlUtils.createTextNode(callName)); - block.appendChild(field); - xml.appendChild(block); - Xml.domToWorkspace(xml, this.workspace); + this.model_ = this.createDef_( + this.getFieldValue('NAME'), this.paramsFromSerializedState_); Events.setGroup(false); } + if (!this.getProcedureModel()) { + // We have a def, but no reference to its model. + this.model_ = this.findProcedureModel_( + this.getFieldValue('NAME'), this.paramsFromSerializedState_); + } + this.initBlockWithProcedureModel_(); } else if (event.type === Events.BLOCK_DELETE && event.blockId != this.id) { // Look for the case where a procedure definition has been deleted, // leaving this block (a procedure call) orphaned. In this case, delete @@ -1351,31 +1527,13 @@ const procedureCallerOnChangeMixin = { this.dispose(true); Events.setGroup(false); } - } else if (event.type === Events.CHANGE && event.element === 'disabled') { - const name = this.getProcedureCall(); - const def = Procedures.getDefinition(name, this.workspace); - if (def && def.id === event.blockId) { - // in most cases the old group should be '' - const oldGroup = Events.getGroup(); - if (oldGroup) { - // This should only be possible programmatically and may indicate a - // problem with event grouping. If you see this message please - // investigate. If the use ends up being valid we may need to reorder - // events in the undo stack. - console.log( - 'Saw an existing group while responding to a definition change'); - } - Events.setGroup(event.group); - if (event.newValue) { - this.previousEnabledState_ = this.isEnabled(); - this.setEnabled(false); - } else { - this.setEnabled(this.previousEnabledState_); - } - Events.setGroup(oldGroup); - } } }, + + defMatches_(defBlock) { + return defBlock && defBlock.type === this.defType_ && + JSON.stringify(defBlock.getVars()) === JSON.stringify(this.arguments_); + }, }; Extensions.registerMixin( 'procedure_caller_onchange_mixin', procedureCallerOnChangeMixin); diff --git a/core/procedures/update_procedures.ts b/core/procedures/update_procedures.ts index 53d06caa5..b355a9c56 100644 --- a/core/procedures/update_procedures.ts +++ b/core/procedures/update_procedures.ts @@ -14,6 +14,7 @@ import {Workspace} from '../workspace.js'; * @internal */ export function triggerProceduresUpdate(workspace: Workspace) { + if (workspace.isClearing) return; for (const block of workspace.getAllBlocks(false)) { if (isProcedureBlock(block)) { block.doProcedureUpdate(); diff --git a/tests/mocha/blocks/procedures_test.js b/tests/mocha/blocks/procedures_test.js index ae137689a..f9b96c61b 100644 --- a/tests/mocha/blocks/procedures_test.js +++ b/tests/mocha/blocks/procedures_test.js @@ -12,15 +12,17 @@ import {assertCallBlockStructure, assertDefBlockStructure, createProcDefBlock, c import {assertEventNotFired, createChangeListenerSpy} from '../test_helpers/events.js'; import {runSerializationTestSuite} from '../test_helpers/serialization.js'; import {createGenUidStubWithReturns, sharedTestSetup, sharedTestTeardown, workspaceTeardown} from '../test_helpers/setup_teardown.js'; +import {defineRowBlock} from '../test_helpers/block_definitions.js'; suite('Procedures', function() { setup(function() { - sharedTestSetup.call(this); + sharedTestSetup.call(this, {fireEventsNow: false}); this.workspace = Blockly.inject('blocklyDiv', {}); this.workspace.createVariable('preCreatedVar', '', 'preCreatedVarId'); this.workspace.createVariable( 'preCreatedTypedVar', 'type', 'preCreatedTypedVarId'); + defineRowBlock(); }); teardown(function() { @@ -47,6 +49,7 @@ suite('Procedures', function() { const defBlock = createProcDefBlock(this.workspace); defBlock.setEnabled(false); + this.clock.runAll(); chai.assert.isFalse( defBlock.getProcedureModel().getEnabled(), @@ -329,7 +332,7 @@ suite('Procedures', function() { }); }); - suite.skip('caller blocks', function() { + suite('caller blocks', function() { test('renaming the procedure data model updates blocks', function() { const defBlock = createProcDefBlock(this.workspace); const callBlock = createProcCallBlock(this.workspace); @@ -351,7 +354,7 @@ suite('Procedures', function() { procModel.setEnabled(false); chai.assert.isFalse( - callBlock.getEnabled(), + callBlock.isEnabled(), 'Expected the procedure block to be disabled'); }); @@ -387,20 +390,58 @@ suite('Procedures', function() { procModel.insertParameter(param2, 0); chai.assert.isNotNull( - callBlock.getInput('ARG0'), - 'Expected the first param input to exist'); + callBlock.getInput('ARG0'), + 'Expected the first param input to exist'); chai.assert.isNotNull( - callBlock.getInput('ARG1'), - 'Expected the second param input to exist'); + callBlock.getInput('ARG1'), + 'Expected the second param input to exist'); chai.assert.equal( - callBlock.getFieldValue('ARGNAME0'), - 'param1', - 'Expected the first params field to match the name of the param'); + callBlock.getFieldValue('ARGNAME0'), + 'param2', + 'Expected the first params field to match the name of the param'); chai.assert.equal( - callBlock.getFieldValue('ARGNAME1'), - 'param2', - 'Expected the second params field to match the name of the param'); + callBlock.getFieldValue('ARGNAME1'), + 'param1', + 'Expected the second params field to match the name of the param'); }); + + test( + 'moving a parameter in the data model moves input blocks', + function() { + const defBlock = createProcDefBlock(this.workspace); + const callBlock = createProcCallBlock(this.workspace); + const procModel = defBlock.getProcedureModel(); + const param1 = + new ObservableParameterModel(this.workspace, 'param1', 'id1'); + const param2 = + new ObservableParameterModel(this.workspace, 'param2', 'id2'); + procModel.insertParameter(param1, 0); + procModel.insertParameter(param2, 1); + const rowBlock1 = this.workspace.newBlock('row_block'); + const rowBlock2 = this.workspace.newBlock('row_block'); + callBlock.getInput('ARG0').connection + .connect(rowBlock1.outputConnection); + callBlock.getInput('ARG1').connection + .connect(rowBlock2.outputConnection); + + procModel.deleteParameter(1); + procModel.insertParameter(param2, 0); + + chai.assert.isNotNull( + callBlock.getInput('ARG0'), + 'Expected the first param input to exist'); + chai.assert.equal( + callBlock.getInputTargetBlock('ARG0'), + rowBlock2, + 'Expected the second row block to be attached to the first input'); + chai.assert.isNotNull( + callBlock.getInput('ARG1'), + 'Expected the second param input to exist'); + chai.assert.equal( + callBlock.getInputTargetBlock('ARG1'), + rowBlock1, + 'Expected the first row block to be attached to the second input'); + }); test( 'deleting a parameter from the data model updates blocks', @@ -702,7 +743,7 @@ suite('Procedures', function() { }); }); - suite('Renaming procedures', function() { + suite('renaming procedures', function() { test('callers are updated to have the new name', function() { const defBlock = createProcDefBlock(this.workspace); const callBlock = createProcCallBlock(this.workspace); @@ -738,7 +779,7 @@ suite('Procedures', function() { }); }); - suite('Adding procedure parameters', function() { + suite('adding procedure parameters', function() { test('no variable create event is fired', function() { const eventSpy = createChangeListenerSpy(this.workspace); const defBlock = createProcDefBlock(this.workspace); @@ -774,6 +815,7 @@ suite('Procedures', function() { }, }, mutatorWorkspace); + this.clock.runAll(); const newFlyoutParamName = mutatorWorkspace.getFlyout().getWorkspace().getTopBlocks(true)[0] @@ -829,7 +871,7 @@ suite('Procedures', function() { }); }); - suite('Renaming procedure parameters', function() { + suite('renaming procedure parameters', function() { test('defs are updated for parameter renames', function() { // Create a stack of container, parameter. const defBlock = createProcDefBlock(this.workspace); @@ -963,7 +1005,7 @@ suite('Procedures', function() { }); }); - suite('Reordering procedure parameters', function() { + suite('reordering procedure parameters', function() { test('reordering procedure parameters updates procedure blocks', function() { // Create a stack of container, parameter, parameter. const defBlock = createProcDefBlock(this.workspace); @@ -1078,7 +1120,7 @@ suite('Procedures', function() { }); }); - suite('Enabling and disabling procedure blocks', function() { + suite('enabling and disabling procedure blocks', function() { test( 'if a procedure definition is disabled, the procedure caller ' + 'is also disabled', @@ -1087,6 +1129,7 @@ suite('Procedures', function() { const callBlock = createProcCallBlock(this.workspace); defBlock.setEnabled(false); + this.clock.runAll(); chai.assert.isFalse( callBlock.isEnabled(), @@ -1125,9 +1168,8 @@ suite('Procedures', function() { }); }); - suite('Deleting procedure blocks', function() { - // Currently fails because of event ordering. - test.skip( + suite('deleting procedure blocks', function() { + test( 'when the procedure definition block is deleted, all of its ' + 'associated callers are deleted as well', function() { @@ -1135,6 +1177,8 @@ suite('Procedures', function() { const callBlock1 = createProcCallBlock(this.workspace); const callBlock2 = createProcCallBlock(this.workspace); + defBlock.dispose(); + this.clock.runAll(); chai.assert.isTrue( callBlock1.disposed, 'Expected the first caller to be disposed'); @@ -1143,6 +1187,232 @@ suite('Procedures', function() { }); }); + suite('caller blocks creating new def blocks', function() { + setup(function() { + this.TEST_VAR_ID = 'test-id'; + this.genUidStub = createGenUidStubWithReturns(this.TEST_VAR_ID); + }); + + suite('xml', function() { + test('callers without defs create new defs', function() { + const callBlock = Blockly.Xml.domToBlock(Blockly.Xml.textToDom( + '' + + '' + + '' + ), this.workspace); + this.clock.runAll(); + assertDefBlockStructure( + this.workspace.getBlocksByType('procedures_defreturn')[0], true); + assertCallBlockStructure(callBlock, [], [], 'do something'); + }); + + test('callers without mutations create unamed defs', function() { + const callBlock = Blockly.Xml.domToBlock(Blockly.Xml.textToDom( + '' + ), this.workspace); + this.clock.runAll(); + assertDefBlockStructure( + this.workspace.getBlocksByType('procedures_defreturn')[0], true); + assertCallBlockStructure(callBlock, [], [], 'unnamed'); + }); + + test('callers with missing args create new defs', function() { + const defBlock = Blockly.Xml.domToBlock(Blockly.Xml.textToDom(` + + do something + + + + + `), this.workspace); + const callBlock = Blockly.Xml.domToBlock(Blockly.Xml.textToDom( + '' + + ' ' + + '' + ), this.workspace); + this.clock.runAll(); + assertDefBlockStructure(defBlock, true, ['x'], ['arg']); + assertCallBlockStructure(callBlock, [], [], 'do something2'); + }); + + test('callers with mismatched args create new defs', function() { + const defBlock = Blockly.Xml.domToBlock(Blockly.Xml.textToDom(` + + do something + + + + + `), this.workspace); + const callBlock = Blockly.Xml.domToBlock(Blockly.Xml.textToDom(` + + + + + + `), this.workspace); + this.clock.runAll(); + assertDefBlockStructure(defBlock, true, ['x'], ['arg']); + assertCallBlockStructure( + callBlock, ['y'], [this.TEST_VAR_ID], 'do something2'); + }); + + test( + 'callers whose defs are deserialized later do not create defs', + function() { + Blockly.Xml.domToWorkspace(Blockly.Xml.textToDom(` + + + + + + + + do something + + + + + + `), this.workspace); + this.clock.runAll(); + const defBlock = + this.workspace.getBlocksByType('procedures_defreturn')[0]; + const callBlock = + this.workspace.getBlocksByType('procedures_callreturn')[0]; + assertDefBlockStructure(defBlock, true, ['x'], ['arg']); + assertCallBlockStructure(callBlock, ['x'], ['arg'], 'do something'); + chai.assert.equal( + defBlock.getProcedureModel(), + callBlock.getProcedureModel(), + 'Expected the blocks to have the same procedure model'); + }); + }); + + suite('json', function() { + test('callers without defs create new defs', function() { + const callBlock = Blockly.serialization.blocks.append({ + 'type': 'procedures_callreturn', + 'extraState': { + 'name': 'do something', + }, + }, this.workspace, {recordUndo: true}); + this.clock.runAll(); + assertDefBlockStructure( + this.workspace.getBlocksByType('procedures_defreturn')[0], true); + assertCallBlockStructure(callBlock, [], [], 'do something'); + }); + + test('callers without extra state create unamed defs', function() { + // recordUndo must be true to trigger change listener. + const callBlock = Blockly.serialization.blocks.append({ + 'type': 'procedures_callreturn', + }, this.workspace, {recordUndo: true}); + this.clock.runAll(); + assertDefBlockStructure( + this.workspace.getBlocksByType('procedures_defreturn')[0], true); + assertCallBlockStructure(callBlock, [], [], 'unnamed'); + }); + + test('callers with missing args create new defs', function() { + const defBlock = Blockly.serialization.blocks.append({ + 'type': 'procedures_defreturn', + 'fields': { + 'NAME': 'do something', + }, + 'extraState': { + 'params': [ + { + 'name': 'x', + 'id': 'arg', + }, + ], + }, + }, this.workspace); + const callBlock = Blockly.serialization.blocks.append({ + 'type': 'procedures_callreturn', + 'extraState': { + 'name': 'do something', + }, + }, this.workspace, {recordUndo: true}); + this.clock.runAll(); + assertDefBlockStructure(defBlock, true, ['x'], ['arg']); + assertCallBlockStructure(callBlock, [], [], 'do something2'); + }); + + test('callers with mismatched args create new defs', function() { + const defBlock = Blockly.serialization.blocks.append({ + 'type': 'procedures_defreturn', + 'fields': { + 'NAME': 'do something', + }, + 'extraState': { + 'params': [ + { + 'name': 'x', + 'id': 'arg', + }, + ], + }, + }, this.workspace); + const callBlock = Blockly.serialization.blocks.append({ + 'type': 'procedures_callreturn', + 'extraState': { + 'name': 'do something', + 'params': ['y'], + }, + }, this.workspace, {recordUndo: true}); + this.clock.runAll(); + assertDefBlockStructure(defBlock, true, ['x'], ['arg']); + assertCallBlockStructure( + callBlock, ['y'], [this.TEST_VAR_ID], 'do something2'); + }); + + test( + 'callers whose defs are deserialized later do not create defs', + function() { + Blockly.serialization.workspaces.load({ + 'blocks': { + 'languageVersion': 0, + 'blocks': [ + { + 'type': 'procedures_callreturn', + 'extraState': { + 'params': ['x'], + }, + }, + { + 'type': 'procedures_defreturn', + 'fields': { + 'NAME': 'do something', + }, + 'extraState': { + 'params': [ + { + 'name': 'x', + 'id': 'arg', + }, + ], + }, + }, + ], + }, + }, this.workspace); + this.clock.runAll(); + const defBlock = + this.workspace.getBlocksByType('procedures_defreturn')[0]; + const callBlock = + this.workspace.getBlocksByType('procedures_callreturn')[0]; + assertDefBlockStructure(defBlock, true, ['x'], ['arg']); + assertCallBlockStructure(callBlock, ['x'], ['arg'], 'do something'); + chai.assert.equal( + defBlock.getProcedureModel(), + callBlock.getProcedureModel(), + 'Expected the blocks to have the same procedure model'); + }); + }); + }); + suite('allProcedures', function() { test('Only Procedures', function() { const noReturnBlock = this.workspace.newBlock('procedures_defnoreturn'); @@ -1197,7 +1467,7 @@ suite('Procedures', function() { }); }); - suite.skip('Multiple block serialization', function() { + suite('Multiple block serialization', function() { function assertDefAndCallBlocks(workspace, noReturnNames, returnNames, hasCallers) { const allProcedures = Blockly.Procedures.allProcedures(workspace); const defNoReturnBlocks = allProcedures[0]; @@ -1255,22 +1525,13 @@ suite('Procedures', function() { this.workspace, ['unnamed2'], ['unnamed'], false); }); - test('callnoreturn (no def in xml)', function() { - const xml = Blockly.Xml.textToDom(` - - - `); - Blockly.Xml.domToWorkspace(xml, this.workspace); - assertDefAndCallBlocks( - this.workspace, ['unnamed'], [], true); - }); - test('callreturn (no def in xml)', function() { const xml = Blockly.Xml.textToDom(` `); Blockly.Xml.domToWorkspace(xml, this.workspace); + this.clock.runAll(); assertDefAndCallBlocks( this.workspace, [], ['unnamed'], true); }); @@ -1282,6 +1543,7 @@ suite('Procedures', function() { `); Blockly.Xml.domToWorkspace(xml, this.workspace); + this.clock.runAll(); assertDefAndCallBlocks( this.workspace, ['unnamed'], ['unnamed2'], true); }); @@ -1293,95 +1555,11 @@ suite('Procedures', function() { `); Blockly.Xml.domToWorkspace(xml, this.workspace); + this.clock.runAll(); assertDefAndCallBlocks( this.workspace, ['unnamed2'], ['unnamed'], true); }); }); - - suite('caller param mismatch', function() { - setup(function() { - this.TEST_VAR_ID = 'test-id'; - this.genUidStub = createGenUidStubWithReturns(this.TEST_VAR_ID); - }); - - test('callreturn with missing args', function() { - const defBlock = Blockly.Xml.domToBlock(Blockly.Xml.textToDom(` - - do something - - - - - `), this.workspace); - const callBlock = Blockly.Xml.domToBlock(Blockly.Xml.textToDom( - '' + - ' ' + - '' - ), this.workspace); - assertDefBlockStructure(defBlock, true, ['x'], ['arg']); - assertCallBlockStructure(callBlock, [], [], 'do something2'); - }); - - test('callreturn with bad args', function() { - const defBlock = Blockly.Xml.domToBlock(Blockly.Xml.textToDom(` - - do something - - - - - `), this.workspace); - const callBlock = Blockly.Xml.domToBlock(Blockly.Xml.textToDom(` - - - - - - `), this.workspace); - assertDefBlockStructure(defBlock, true, ['x'], ['arg']); - assertCallBlockStructure( - callBlock, ['y'], [this.TEST_VAR_ID], 'do something2'); - }); - - test('callnoreturn with missing args', function() { - const defBlock = Blockly.Xml.domToBlock(Blockly.Xml.textToDom(` - - do something - - - - - `), this.workspace); - const callBlock = Blockly.Xml.domToBlock(Blockly.Xml.textToDom( - '' + - ' ' + - '' - ), this.workspace); - assertDefBlockStructure(defBlock, false, ['x'], ['arg']); - assertCallBlockStructure(callBlock, [], [], 'do something2'); - }); - - test('callnoreturn with bad args', function() { - const defBlock = Blockly.Xml.domToBlock(Blockly.Xml.textToDom(` - - do something - - - - - `), this.workspace); - const callBlock = Blockly.Xml.domToBlock(Blockly.Xml.textToDom(` - - - - - - `), this.workspace); - assertDefBlockStructure(defBlock, false, ['x'], ['arg']); - assertCallBlockStructure( - callBlock, ['y'], [this.TEST_VAR_ID], 'do something2'); - }); - }); }); suite('getDefinition - Modified cases', function() { @@ -1448,8 +1626,11 @@ suite('Procedures', function() { }); test('Call block', function() { - this.callBlock = this.workspace.newBlock(testSuite.callType); + this.callBlock = Blockly.serialization.blocks.append({ + 'type': testSuite.callType, + }, this.workspace, {recordUndo: true}); this.callBlock.setFieldValue('proc name', 'NAME'); + this.clock.runAll(); assertCallBlockStructure(this.callBlock); }); }); diff --git a/tests/mocha/test_helpers/procedures.js b/tests/mocha/test_helpers/procedures.js index b409c5c2f..74c8a7fd2 100644 --- a/tests/mocha/test_helpers/procedures.js +++ b/tests/mocha/test_helpers/procedures.js @@ -102,7 +102,7 @@ export function assertCallBlockStructure( assertCallBlockArgsStructure(callBlock, args); assertBlockVarModels(callBlock, varIds); if (name !== undefined) { - chai.assert(callBlock.getFieldValue('NAME'), name); + chai.assert.equal(callBlock.getFieldValue('NAME'), name); } } diff --git a/tests/mocha/test_helpers/serialization.js b/tests/mocha/test_helpers/serialization.js index cc076bfc0..ebc6deb5d 100644 --- a/tests/mocha/test_helpers/serialization.js +++ b/tests/mocha/test_helpers/serialization.js @@ -66,6 +66,7 @@ export const runSerializationTestSuite = (testCases) => { block = Blockly.Xml.domToBlock(Blockly.Xml.textToDom( testCase.xml), this.workspace); } + this.clock.runAll(); testCase.assertBlockStructure(block); }; };