From a6d26fb60d4c88aaf1b6baf65e769a847a887706 Mon Sep 17 00:00:00 2001 From: Beka Westberg Date: Mon, 19 Dec 2022 21:42:59 +0000 Subject: [PATCH 01/10] fix: undoing and redoing parameters creating dupe parameters --- blocks/procedures.js | 29 +++++++++++---------------- core/serialization/procedures.ts | 4 ++-- tests/mocha/jso_serialization_test.js | 2 +- 3 files changed, 15 insertions(+), 20 deletions(-) diff --git a/blocks/procedures.js b/blocks/procedures.js index c91364f81e4..31474f573c0 100644 --- a/blocks/procedures.js +++ b/blocks/procedures.js @@ -528,11 +528,13 @@ const procedureDefMutator = { if (opt_paramIds) { container.setAttribute('name', this.getFieldValue('NAME')); } - for (let i = 0; i < this.argumentVarModels_.length; i++) { + + const params = this.getProcedureModel().getParameters(); + for (let i = 0; i < params.length; i++) { const parameter = xmlUtils.createElement('arg'); - const argModel = this.argumentVarModels_[i]; - parameter.setAttribute('name', argModel.name); - parameter.setAttribute('varid', argModel.getId()); + const varModel = params[i].getVariableModel(); + parameter.setAttribute('name', varModel.name); + parameter.setAttribute('varid', varModel.getId()); if (opt_paramIds && this.paramIds_) { parameter.setAttribute('paramId', this.paramIds_[i]); } @@ -596,20 +598,13 @@ const procedureDefMutator = { * parameters and statements. */ saveExtraState: function() { - if (!this.argumentVarModels_.length && this.hasStatements_) { - return null; - } + const params = this.getProcedureModel().getParameters(); + if (!params.length && this.hasStatements_) return; + const state = Object.create(null); - if (this.argumentVarModels_.length) { - state['params'] = []; - for (let i = 0; i < this.argumentVarModels_.length; i++) { - state['params'].push({ - // We don't need to serialize the name, but just in case we decide - // to separate params from variables. - 'name': this.argumentVarModels_[i].name, - 'id': this.argumentVarModels_[i].getId(), - }); - } + if (params.length) { + state['params'] = + params.map((p) => ({'name': p.getName(), 'id': p.getId()})); } if (!this.hasStatements_) { state['hasStatements'] = false; diff --git a/core/serialization/procedures.ts b/core/serialization/procedures.ts index 05f70b03b80..8e18dda664d 100644 --- a/core/serialization/procedures.ts +++ b/core/serialization/procedures.ts @@ -10,7 +10,7 @@ import type {ISerializer} from '../interfaces/i_serializer.js'; import {ObservableProcedureModel} from '../procedures/observable_procedure_model.js'; import {ObservableParameterModel} from '../procedures/observable_parameter_model.js'; import * as priorities from './priorities.js'; -// import * as serializationRegistry from './registry.js'; +import * as serializationRegistry from './registry.js'; import type {Workspace} from '../workspace.js'; @@ -171,4 +171,4 @@ export class ProcedureSerializer Date: Tue, 20 Dec 2022 20:22:05 +0000 Subject: [PATCH 02/10] chore: add tests for undoing and redoing adding parameters --- blocks/procedures.js | 1 + core/procedures/observable_parameter_model.ts | 2 +- tests/mocha/blocks/procedures_test.js | 128 +++++++++++++++++- 3 files changed, 128 insertions(+), 3 deletions(-) diff --git a/blocks/procedures.js b/blocks/procedures.js index 31474f573c0..52aa9d9dc18 100644 --- a/blocks/procedures.js +++ b/blocks/procedures.js @@ -930,6 +930,7 @@ const validateProcedureParamMixin = { this.createdVariables_.push(model); } } + return varName; }, diff --git a/core/procedures/observable_parameter_model.ts b/core/procedures/observable_parameter_model.ts index 91f624f951d..89b805a0c41 100644 --- a/core/procedures/observable_parameter_model.ts +++ b/core/procedures/observable_parameter_model.ts @@ -25,7 +25,7 @@ export class ObservableParameterModel implements IParameterModel { private readonly workspace: Workspace, name: string, id?: string) { this.id = id ?? genUid(); this.variable = this.workspace.getVariable(name) ?? - workspace.createVariable(name, '', id); + workspace.createVariable(name, ''); } /** diff --git a/tests/mocha/blocks/procedures_test.js b/tests/mocha/blocks/procedures_test.js index f9b96c61b5c..b8dcc94caed 100644 --- a/tests/mocha/blocks/procedures_test.js +++ b/tests/mocha/blocks/procedures_test.js @@ -15,7 +15,7 @@ import {createGenUidStubWithReturns, sharedTestSetup, sharedTestTeardown, worksp import {defineRowBlock} from '../test_helpers/block_definitions.js'; -suite('Procedures', function() { +suite.only('Procedures', function() { setup(function() { sharedTestSetup.call(this, {fireEventsNow: false}); this.workspace = Blockly.inject('blocklyDiv', {}); @@ -869,9 +869,76 @@ suite('Procedures', function() { 'param1', 'Expected the params field to match the name of the new param'); }); + + test('undoing adding a procedure parameter removes it', function() { + // Create a stack of container, parameter. + const defBlock = createProcDefBlock(this.workspace); + defBlock.mutator.setVisible(true); + const mutatorWorkspace = defBlock.mutator.getWorkspace(); + const containerBlock = + mutatorWorkspace.newBlock('procedures_mutatorcontainer'); + const paramBlock = mutatorWorkspace.newBlock('procedures_mutatorarg'); + paramBlock.setFieldValue('param1', 'NAME'); + containerBlock.getInput('STACK').connection.connect(paramBlock.previousConnection); + defBlock.compose(containerBlock); + + this.workspace.undo(); + + chai.assert.isFalse( + defBlock.getFieldValue('PARAMS').includes('param1'), + 'Expected the params field to not contain the name of the new param'); + }); + + test( + 'undoing and redoing adding a procedure parameter maintains ' + + 'the same state', + function() { + // Create a stack of container, parameter. + const defBlock = createProcDefBlock(this.workspace); + defBlock.mutator.setVisible(true); + const mutatorWorkspace = defBlock.mutator.getWorkspace(); + const containerBlock = + mutatorWorkspace.newBlock('procedures_mutatorcontainer'); + const paramBlock = mutatorWorkspace.newBlock('procedures_mutatorarg'); + paramBlock.setFieldValue('param1', 'NAME'); + containerBlock.getInput('STACK').connection.connect( + paramBlock.previousConnection); + defBlock.compose(containerBlock); + + this.workspace.undo(); + this.workspace.undo(/* redo= */ true); + + chai.assert.isNotNull( + defBlock.getField('PARAMS'), + 'Expected the params field to exist'); + chai.assert.isTrue( + defBlock.getFieldValue('PARAMS').includes('param1'), + 'Expected the params field to contain the name of the new param'); + }); }); - suite('renaming procedure parameters', function() { + suite.only('renaming procedure parameters', function() { + test('no variable event is fired', function() { + const eventSpy = createChangeListenerSpy(this.workspace); + // Create a stack of container, parameter. + const defBlock = createProcDefBlock(this.workspace); + defBlock.mutator.setVisible(true); + const mutatorWorkspace = defBlock.mutator.getWorkspace(); + const containerBlock = + mutatorWorkspace.newBlock('procedures_mutatorcontainer'); + const paramBlock = mutatorWorkspace.newBlock('procedures_mutatorarg'); + paramBlock.setFieldValue('param1', 'NAME'); + containerBlock.getInput('STACK').connection.connect(paramBlock.previousConnection); + defBlock.compose(containerBlock); + + eventSpy.resetHistory(); + paramBlock.setFieldValue('new name', 'NAME'); + defBlock.compose(containerBlock); + + assertEventNotFired( + eventSpy, Blockly.Events.VarCreate, {}, this.workspace.id); + }); + test('defs are updated for parameter renames', function() { // Create a stack of container, parameter. const defBlock = createProcDefBlock(this.workspace); @@ -1003,6 +1070,63 @@ suite('Procedures', function() { function() { }); + + test( + 'undoing renaming a procedure parameter reverts the change', + function() { + // Create a stack of container, parameter. + const defBlock = createProcDefBlock(this.workspace); + defBlock.mutator.setVisible(true); + const mutatorWorkspace = defBlock.mutator.getWorkspace(); + const containerBlock = mutatorWorkspace.getTopBlocks()[0]; + const paramBlock = mutatorWorkspace.newBlock('procedures_mutatorarg'); + paramBlock.setFieldValue('param1', 'NAME'); + containerBlock.getInput('STACK').connection.connect(paramBlock.previousConnection); + this.clock.runAll(); + Blockly.Events.setGroup(true); + paramBlock.setFieldValue('n', 'NAME'); + this.clock.runAll(); + paramBlock.setFieldValue('ne', 'NAME'); + this.clock.runAll(); + paramBlock.setFieldValue('new', 'NAME'); + this.clock.runAll(); + Blockly.Events.setGroup(false); + + this.workspace.undo(); + + chai.assert.isTrue( + defBlock.getFieldValue('PARAMS').includes('param1'), + 'Expected the params field to contain the old name of the param'); + }); + + test( + 'undoing and redoing renaming a procedure maintains the same state', + function() { + // Create a stack of container, parameter. + const defBlock = createProcDefBlock(this.workspace); + defBlock.mutator.setVisible(true); + const mutatorWorkspace = defBlock.mutator.getWorkspace(); + const containerBlock = mutatorWorkspace.getTopBlocks()[0]; + const paramBlock = mutatorWorkspace.newBlock('procedures_mutatorarg'); + paramBlock.setFieldValue('param1', 'NAME'); + containerBlock.getInput('STACK').connection.connect(paramBlock.previousConnection); + this.clock.runAll(); + Blockly.Events.setGroup(true); + paramBlock.setFieldValue('n', 'NAME'); + this.clock.runAll(); + paramBlock.setFieldValue('ne', 'NAME'); + this.clock.runAll(); + paramBlock.setFieldValue('new', 'NAME'); + this.clock.runAll(); + Blockly.Events.setGroup(false); + + this.workspace.undo(); + this.workspace.undo(/* redo= */ true); + + chai.assert.isTrue( + defBlock.getFieldValue('PARAMS').includes('new'), + 'Expected the params field to contain the new name of the param'); + }); }); suite('reordering procedure parameters', function() { From 403a13b2f144236034eb89b82f56ffddc6404f3e Mon Sep 17 00:00:00 2001 From: Beka Westberg Date: Wed, 21 Dec 2022 19:10:27 +0000 Subject: [PATCH 03/10] fix: undoing and redoing renaming parameters --- blocks/procedures.js | 18 +++++++++---- core/procedures/observable_parameter_model.ts | 4 +-- tests/mocha/blocks/procedures_test.js | 27 +++---------------- 3 files changed, 18 insertions(+), 31 deletions(-) diff --git a/blocks/procedures.js b/blocks/procedures.js index 52aa9d9dc18..74caa0405f3 100644 --- a/blocks/procedures.js +++ b/blocks/procedures.js @@ -558,10 +558,10 @@ const procedureDefMutator = { for (let i = 0; i < xmlElement.childNodes.length; i++) { const node = xmlElement.childNodes[i]; if (node.nodeName.toLowerCase() !== 'arg') continue; + const varId = node.getAttribute('varid'); this.getProcedureModel().insertParameter( new ObservableParameterModel( - this.workspace, node.getAttribute('name'), - node.getAttribute('varid')), + this.workspace, node.getAttribute('name'), undefined, varId), i); } @@ -604,7 +604,15 @@ const procedureDefMutator = { const state = Object.create(null); if (params.length) { state['params'] = - params.map((p) => ({'name': p.getName(), 'id': p.getId()})); + params.map((p) => { + return { + 'name': p.getName(), + 'id': p.getVariableModel().getId(), + // Ideally this would be id, and the other would be varId, + // but backwards compatibility :/ + 'paramId': p.getId(), + }; + }); } if (!this.hasStatements_) { state['hasStatements'] = false; @@ -620,9 +628,9 @@ const procedureDefMutator = { loadExtraState: function(state) { if (state['params']) { for (let i = 0; i < state['params'].length; i++) { - const param = state['params'][i]; + const {name, id, paramId} = state['params'][i]; this.getProcedureModel().insertParameter( - new ObservableParameterModel(this.workspace, param.name, param.id), + new ObservableParameterModel(this.workspace, name, paramId, id), i); } } diff --git a/core/procedures/observable_parameter_model.ts b/core/procedures/observable_parameter_model.ts index 89b805a0c41..c58463375cd 100644 --- a/core/procedures/observable_parameter_model.ts +++ b/core/procedures/observable_parameter_model.ts @@ -22,10 +22,10 @@ export class ObservableParameterModel implements IParameterModel { private procedureModel: IProcedureModel|null = null; constructor( - private readonly workspace: Workspace, name: string, id?: string) { + private readonly workspace: Workspace, name: string, id?: string, varId?: string) { this.id = id ?? genUid(); this.variable = this.workspace.getVariable(name) ?? - workspace.createVariable(name, ''); + workspace.createVariable(name, '', varId); } /** diff --git a/tests/mocha/blocks/procedures_test.js b/tests/mocha/blocks/procedures_test.js index b8dcc94caed..15d2607e91d 100644 --- a/tests/mocha/blocks/procedures_test.js +++ b/tests/mocha/blocks/procedures_test.js @@ -15,7 +15,7 @@ import {createGenUidStubWithReturns, sharedTestSetup, sharedTestTeardown, worksp import {defineRowBlock} from '../test_helpers/block_definitions.js'; -suite.only('Procedures', function() { +suite('Procedures', function() { setup(function() { sharedTestSetup.call(this, {fireEventsNow: false}); this.workspace = Blockly.inject('blocklyDiv', {}); @@ -917,28 +917,7 @@ suite.only('Procedures', function() { }); }); - suite.only('renaming procedure parameters', function() { - test('no variable event is fired', function() { - const eventSpy = createChangeListenerSpy(this.workspace); - // Create a stack of container, parameter. - const defBlock = createProcDefBlock(this.workspace); - defBlock.mutator.setVisible(true); - const mutatorWorkspace = defBlock.mutator.getWorkspace(); - const containerBlock = - mutatorWorkspace.newBlock('procedures_mutatorcontainer'); - const paramBlock = mutatorWorkspace.newBlock('procedures_mutatorarg'); - paramBlock.setFieldValue('param1', 'NAME'); - containerBlock.getInput('STACK').connection.connect(paramBlock.previousConnection); - defBlock.compose(containerBlock); - - eventSpy.resetHistory(); - paramBlock.setFieldValue('new name', 'NAME'); - defBlock.compose(containerBlock); - - assertEventNotFired( - eventSpy, Blockly.Events.VarCreate, {}, this.workspace.id); - }); - + suite('renaming procedure parameters', function() { test('defs are updated for parameter renames', function() { // Create a stack of container, parameter. const defBlock = createProcDefBlock(this.workspace); @@ -1093,7 +1072,7 @@ suite.only('Procedures', function() { Blockly.Events.setGroup(false); this.workspace.undo(); - + chai.assert.isTrue( defBlock.getFieldValue('PARAMS').includes('param1'), 'Expected the params field to contain the old name of the param'); From 3145f8b193fdc55b3af449a4d0218767523a9d95 Mon Sep 17 00:00:00 2001 From: Beka Westberg Date: Wed, 21 Dec 2022 23:41:56 +0000 Subject: [PATCH 04/10] chore: change tests to tick clock --- tests/mocha/blocks/procedures_test.js | 169 ++++++++++++-------------- 1 file changed, 75 insertions(+), 94 deletions(-) diff --git a/tests/mocha/blocks/procedures_test.js b/tests/mocha/blocks/procedures_test.js index 15d2607e91d..79ee41c776d 100644 --- a/tests/mocha/blocks/procedures_test.js +++ b/tests/mocha/blocks/procedures_test.js @@ -63,13 +63,11 @@ suite('Procedures', function() { const defBlock = createProcDefBlock(this.workspace); defBlock.mutator.setVisible(true); const mutatorWorkspace = defBlock.mutator.getWorkspace(); - const containerBlock = - mutatorWorkspace.newBlock('procedures_mutatorcontainer'); + const containerBlock = mutatorWorkspace.getTopBlocks()[0]; const paramBlock = mutatorWorkspace.newBlock('procedures_mutatorarg'); paramBlock.setFieldValue('param name', 'NAME'); containerBlock.getInput('STACK').connection.connect(paramBlock.previousConnection); - - defBlock.compose(containerBlock); + this.clock.runAll(); chai.assert.equal( defBlock.getProcedureModel().getParameter(0).getName(), @@ -82,14 +80,12 @@ suite('Procedures', function() { const defBlock = createProcDefBlock(this.workspace); defBlock.mutator.setVisible(true); const mutatorWorkspace = defBlock.mutator.getWorkspace(); - const containerBlock = - mutatorWorkspace.newBlock('procedures_mutatorcontainer'); + const containerBlock = mutatorWorkspace.getTopBlocks()[0]; const paramBlock = mutatorWorkspace.newBlock('procedures_mutatorarg'); paramBlock.setFieldValue('param name', 'NAME'); containerBlock.getInput('STACK').connection .connect(paramBlock.previousConnection); - - defBlock.compose(containerBlock); + this.clock.runAll(); chai.assert.isTrue( this.workspace.getVariableMap().getVariablesOfType('') @@ -105,8 +101,7 @@ suite('Procedures', function() { const defBlock = createProcDefBlock(this.workspace); defBlock.mutator.setVisible(true); const mutatorWorkspace = defBlock.mutator.getWorkspace(); - const containerBlock = - mutatorWorkspace.newBlock('procedures_mutatorcontainer'); + const containerBlock = mutatorWorkspace.getTopBlocks()[0]; const paramBlock1 = mutatorWorkspace.newBlock('procedures_mutatorarg'); paramBlock1.setFieldValue('param name1', 'NAME'); const paramBlock2 = mutatorWorkspace.newBlock('procedures_mutatorarg'); @@ -114,7 +109,7 @@ suite('Procedures', function() { containerBlock.getInput('STACK').connection .connect(paramBlock1.previousConnection); paramBlock1.nextConnection.connect(paramBlock2.previousConnection); - defBlock.compose(containerBlock); + this.clock.runAll(); const id1 = defBlock.getProcedureModel().getParameter(0).getId(); const id2 = defBlock.getProcedureModel().getParameter(1).getId(); @@ -124,7 +119,7 @@ suite('Procedures', function() { containerBlock.getInput('STACK').connection .connect(paramBlock2.previousConnection); paramBlock2.nextConnection.connect(paramBlock1.previousConnection); - defBlock.compose(containerBlock); + this.clock.runAll(); chai.assert.equal( defBlock.getProcedureModel().getParameter(0).getName(), @@ -149,13 +144,12 @@ suite('Procedures', function() { const defBlock = createProcDefBlock(this.workspace); defBlock.mutator.setVisible(true); const mutatorWorkspace = defBlock.mutator.getWorkspace(); - const containerBlock = - mutatorWorkspace.newBlock('procedures_mutatorcontainer'); + const containerBlock = mutatorWorkspace.getTopBlocks()[0]; const paramBlock = mutatorWorkspace.newBlock('procedures_mutatorarg'); paramBlock.setFieldValue('param name', 'NAME'); containerBlock.getInput('STACK').connection .connect(paramBlock.previousConnection); - defBlock.compose(containerBlock); + this.clock.runAll(); const paramBlockId = defBlock.getProcedureModel().getParameter(0).getId(); Blockly.Events.disable(); @@ -177,16 +171,15 @@ suite('Procedures', function() { const defBlock = createProcDefBlock(this.workspace); defBlock.mutator.setVisible(true); const mutatorWorkspace = defBlock.mutator.getWorkspace(); - const containerBlock = - mutatorWorkspace.newBlock('procedures_mutatorcontainer'); + const containerBlock = mutatorWorkspace.getTopBlocks()[0]; const paramBlock = mutatorWorkspace.newBlock('procedures_mutatorarg'); paramBlock.setFieldValue('param name', 'NAME'); containerBlock.getInput('STACK').connection .connect(paramBlock.previousConnection); - defBlock.compose(containerBlock); + this.clock.runAll(); containerBlock.getInput('STACK').connection.disconnect(); - defBlock.compose(containerBlock); + this.clock.runAll(); chai.assert.isEmpty( defBlock.getProcedureModel().getParameters(), @@ -198,16 +191,15 @@ suite('Procedures', function() { const defBlock = createProcDefBlock(this.workspace); defBlock.mutator.setVisible(true); const mutatorWorkspace = defBlock.mutator.getWorkspace(); - const containerBlock = - mutatorWorkspace.newBlock('procedures_mutatorcontainer'); + const containerBlock = mutatorWorkspace.getTopBlocks()[0]; const paramBlock = mutatorWorkspace.newBlock('procedures_mutatorarg'); paramBlock.setFieldValue('param name', 'NAME'); containerBlock.getInput('STACK').connection .connect(paramBlock.previousConnection); - defBlock.compose(containerBlock); + this.clock.runAll(); paramBlock.setFieldValue('new param name', 'NAME'); - defBlock.compose(containerBlock); + this.clock.runAll(); chai.assert.equal( defBlock.getProcedureModel().getParameter(0).getName(), @@ -780,24 +772,6 @@ suite('Procedures', function() { }); suite('adding procedure parameters', function() { - test('no variable create event is fired', function() { - const eventSpy = createChangeListenerSpy(this.workspace); - const defBlock = createProcDefBlock(this.workspace); - defBlock.mutator.setVisible(true); - const mutatorWorkspace = defBlock.mutator.getWorkspace(); - const containerBlock = - mutatorWorkspace.newBlock('procedures_mutatorcontainer'); - const paramBlock = mutatorWorkspace.newBlock('procedures_mutatorarg'); - paramBlock.setFieldValue('param name', 'NAME'); - containerBlock.getInput('STACK').connection.connect(paramBlock.previousConnection); - - eventSpy.resetHistory(); - defBlock.compose(containerBlock); - - assertEventNotFired( - eventSpy, Blockly.Events.VarCreate, {}, this.workspace.id); - }); - test( 'the mutator flyout updates to avoid parameter name conflicts', function() { @@ -831,13 +805,11 @@ suite('Procedures', function() { const defBlock = createProcDefBlock(this.workspace); defBlock.mutator.setVisible(true); const mutatorWorkspace = defBlock.mutator.getWorkspace(); - const containerBlock = - mutatorWorkspace.newBlock('procedures_mutatorcontainer'); + const containerBlock = mutatorWorkspace.getTopBlocks()[0]; const paramBlock = mutatorWorkspace.newBlock('procedures_mutatorarg'); paramBlock.setFieldValue('param1', 'NAME'); containerBlock.getInput('STACK').connection.connect(paramBlock.previousConnection); - - defBlock.compose(containerBlock); + this.clock.runAll(); chai.assert.isNotNull( defBlock.getField('PARAMS'), @@ -853,13 +825,11 @@ suite('Procedures', function() { const callBlock = createProcCallBlock(this.workspace); defBlock.mutator.setVisible(true); const mutatorWorkspace = defBlock.mutator.getWorkspace(); - const containerBlock = - mutatorWorkspace.newBlock('procedures_mutatorcontainer'); + const containerBlock = mutatorWorkspace.getTopBlocks()[0]; const paramBlock = mutatorWorkspace.newBlock('procedures_mutatorarg'); paramBlock.setFieldValue('param1', 'NAME'); containerBlock.getInput('STACK').connection.connect(paramBlock.previousConnection); - - defBlock.compose(containerBlock); + this.clock.runAll(); chai.assert.isNotNull( callBlock.getInput('ARG0'), @@ -875,12 +845,11 @@ suite('Procedures', function() { const defBlock = createProcDefBlock(this.workspace); defBlock.mutator.setVisible(true); const mutatorWorkspace = defBlock.mutator.getWorkspace(); - const containerBlock = - mutatorWorkspace.newBlock('procedures_mutatorcontainer'); + const containerBlock = mutatorWorkspace.getTopBlocks()[0]; const paramBlock = mutatorWorkspace.newBlock('procedures_mutatorarg'); paramBlock.setFieldValue('param1', 'NAME'); containerBlock.getInput('STACK').connection.connect(paramBlock.previousConnection); - defBlock.compose(containerBlock); + this.clock.runAll(); this.workspace.undo(); @@ -897,13 +866,12 @@ suite('Procedures', function() { const defBlock = createProcDefBlock(this.workspace); defBlock.mutator.setVisible(true); const mutatorWorkspace = defBlock.mutator.getWorkspace(); - const containerBlock = - mutatorWorkspace.newBlock('procedures_mutatorcontainer'); + const containerBlock = mutatorWorkspace.getTopBlocks()[0]; const paramBlock = mutatorWorkspace.newBlock('procedures_mutatorarg'); paramBlock.setFieldValue('param1', 'NAME'); containerBlock.getInput('STACK').connection.connect( paramBlock.previousConnection); - defBlock.compose(containerBlock); + this.clock.runAll(); this.workspace.undo(); this.workspace.undo(/* redo= */ true); @@ -923,15 +891,14 @@ suite('Procedures', function() { const defBlock = createProcDefBlock(this.workspace); defBlock.mutator.setVisible(true); const mutatorWorkspace = defBlock.mutator.getWorkspace(); - const containerBlock = - mutatorWorkspace.newBlock('procedures_mutatorcontainer'); + const containerBlock = mutatorWorkspace.getTopBlocks()[0]; const paramBlock = mutatorWorkspace.newBlock('procedures_mutatorarg'); paramBlock.setFieldValue('param1', 'NAME'); containerBlock.getInput('STACK').connection.connect(paramBlock.previousConnection); - defBlock.compose(containerBlock); + this.clock.runAll(); paramBlock.setFieldValue('new name', 'NAME'); - defBlock.compose(containerBlock); + this.clock.runAll(); chai.assert.isNotNull( defBlock.getField('PARAMS'), @@ -947,15 +914,14 @@ suite('Procedures', function() { const callBlock = createProcCallBlock(this.workspace); defBlock.mutator.setVisible(true); const mutatorWorkspace = defBlock.mutator.getWorkspace(); - const containerBlock = - mutatorWorkspace.newBlock('procedures_mutatorcontainer'); + const containerBlock = mutatorWorkspace.getTopBlocks()[0]; const paramBlock = mutatorWorkspace.newBlock('procedures_mutatorarg'); paramBlock.setFieldValue('param1', 'NAME'); containerBlock.getInput('STACK').connection.connect(paramBlock.previousConnection); - defBlock.compose(containerBlock); + this.clock.runAll(); paramBlock.setFieldValue('new name', 'NAME'); - defBlock.compose(containerBlock); + this.clock.runAll(); chai.assert.isNotNull( callBlock.getInput('ARG0'), @@ -974,15 +940,14 @@ suite('Procedures', function() { const callBlock = createProcCallBlock(this.workspace); defBlock.mutator.setVisible(true); const mutatorWorkspace = defBlock.mutator.getWorkspace(); - const containerBlock = - mutatorWorkspace.newBlock('procedures_mutatorcontainer'); + const containerBlock = mutatorWorkspace.getTopBlocks()[0]; const paramBlock = mutatorWorkspace.newBlock('procedures_mutatorarg'); paramBlock.setFieldValue('param1', 'NAME'); containerBlock.getInput('STACK').connection.connect(paramBlock.previousConnection); - defBlock.compose(containerBlock); + this.clock.runAll(); paramBlock.setFieldValue('param2', 'NAME'); - defBlock.compose(containerBlock); + this.clock.runAll(); chai.assert.isNotNull( this.workspace.getVariable('param1', ''), @@ -996,12 +961,11 @@ suite('Procedures', function() { const defBlock = createProcDefBlock(this.workspace); defBlock.mutator.setVisible(true); const mutatorWorkspace = defBlock.mutator.getWorkspace(); - const containerBlock = - mutatorWorkspace.newBlock('procedures_mutatorcontainer'); + const containerBlock = mutatorWorkspace.getTopBlocks()[0]; const paramBlock = mutatorWorkspace.newBlock('procedures_mutatorarg'); paramBlock.setFieldValue('param1', 'NAME'); containerBlock.getInput('STACK').connection.connect(paramBlock.previousConnection); - defBlock.compose(containerBlock); + this.clock.runAll(); defBlock.mutator.setVisible(false); const variable = this.workspace.getVariable('param1', ''); @@ -1023,12 +987,11 @@ suite('Procedures', function() { const callBlock = createProcCallBlock(this.workspace); defBlock.mutator.setVisible(true); const mutatorWorkspace = defBlock.mutator.getWorkspace(); - const containerBlock = - mutatorWorkspace.newBlock('procedures_mutatorcontainer'); + const containerBlock = mutatorWorkspace.getTopBlocks()[0]; const paramBlock = mutatorWorkspace.newBlock('procedures_mutatorarg'); paramBlock.setFieldValue('param1', 'NAME'); containerBlock.getInput('STACK').connection.connect(paramBlock.previousConnection); - defBlock.compose(containerBlock); + this.clock.runAll(); defBlock.mutator.setVisible(false); const variable = this.workspace.getVariable('param1', ''); @@ -1114,22 +1077,21 @@ suite('Procedures', function() { const defBlock = createProcDefBlock(this.workspace); defBlock.mutator.setVisible(true); const mutatorWorkspace = defBlock.mutator.getWorkspace(); - const containerBlock = - mutatorWorkspace.newBlock('procedures_mutatorcontainer'); + const containerBlock = mutatorWorkspace.getTopBlocks()[0]; const paramBlock1 = mutatorWorkspace.newBlock('procedures_mutatorarg'); paramBlock1.setFieldValue('param1', 'NAME'); const paramBlock2 = mutatorWorkspace.newBlock('procedures_mutatorarg'); paramBlock2.setFieldValue('param2', 'NAME'); containerBlock.getInput('STACK').connection.connect(paramBlock1.previousConnection); paramBlock1.nextConnection.connect(paramBlock2.previousConnection); - defBlock.compose(containerBlock); + this.clock.runAll(); // Reorder the parameters. paramBlock2.previousConnection.disconnect(); paramBlock1.previousConnection.disconnect(); containerBlock.getInput('STACK').connection.connect(paramBlock2.previousConnection); paramBlock2.nextConnection.connect(paramBlock1.previousConnection); - defBlock.compose(containerBlock); + this.clock.runAll(); chai.assert.isNotNull( defBlock.getField('PARAMS'), @@ -1145,22 +1107,21 @@ suite('Procedures', function() { const callBlock = createProcCallBlock(this.workspace); defBlock.mutator.setVisible(true); const mutatorWorkspace = defBlock.mutator.getWorkspace(); - const containerBlock = - mutatorWorkspace.newBlock('procedures_mutatorcontainer'); + const containerBlock = mutatorWorkspace.getTopBlocks()[0]; const paramBlock1 = mutatorWorkspace.newBlock('procedures_mutatorarg'); paramBlock1.setFieldValue('param1', 'NAME'); const paramBlock2 = mutatorWorkspace.newBlock('procedures_mutatorarg'); paramBlock2.setFieldValue('param2', 'NAME'); containerBlock.getInput('STACK').connection.connect(paramBlock1.previousConnection); paramBlock1.nextConnection.connect(paramBlock2.previousConnection); - defBlock.compose(containerBlock); + this.clock.runAll(); // Reorder the parameters. paramBlock2.previousConnection.disconnect(); paramBlock1.previousConnection.disconnect(); containerBlock.getInput('STACK').connection.connect(paramBlock2.previousConnection); paramBlock2.nextConnection.connect(paramBlock1.previousConnection); - defBlock.compose(containerBlock); + this.clock.runAll(); chai.assert.isNotNull( callBlock.getInput('ARG0'), @@ -1187,15 +1148,14 @@ suite('Procedures', function() { const callBlock = createProcCallBlock(this.workspace); defBlock.mutator.setVisible(true); const mutatorWorkspace = defBlock.mutator.getWorkspace(); - const containerBlock = - mutatorWorkspace.newBlock('procedures_mutatorcontainer'); + const containerBlock = mutatorWorkspace.getTopBlocks()[0]; const paramBlock1 = mutatorWorkspace.newBlock('procedures_mutatorarg'); paramBlock1.setFieldValue('param1', 'NAME'); const paramBlock2 = mutatorWorkspace.newBlock('procedures_mutatorarg'); paramBlock2.setFieldValue('param2', 'NAME'); containerBlock.getInput('STACK').connection.connect(paramBlock1.previousConnection); paramBlock1.nextConnection.connect(paramBlock2.previousConnection); - defBlock.compose(containerBlock); + this.clock.runAll(); // Add args to the parameter inputs on the caller. const block1 = this.workspace.newBlock('text'); @@ -1210,7 +1170,7 @@ suite('Procedures', function() { paramBlock1.previousConnection.disconnect(); containerBlock.getInput('STACK').connection.connect(paramBlock2.previousConnection); paramBlock2.nextConnection.connect(paramBlock1.previousConnection); - defBlock.compose(containerBlock); + this.clock.runAll(); chai.assert.equal( callBlock.getInputTargetBlock('ARG0'), @@ -1246,8 +1206,10 @@ suite('Procedures', function() { const defBlock = createProcDefBlock(this.workspace); const callBlock = createProcCallBlock(this.workspace); defBlock.setEnabled(false); + this.clock.runAll(); defBlock.setEnabled(true); + this.clock.runAll(); chai.assert.isTrue( callBlock.isEnabled(), @@ -1261,9 +1223,12 @@ suite('Procedures', function() { const defBlock = createProcDefBlock(this.workspace); const callBlock = createProcCallBlock(this.workspace); callBlock.setEnabled(false); + this.clock.runAll(); defBlock.setEnabled(false); + this.clock.runAll(); defBlock.setEnabled(true); + this.clock.runAll(); chai.assert.isFalse( callBlock.isEnabled(), @@ -1281,8 +1246,8 @@ suite('Procedures', function() { const callBlock2 = createProcCallBlock(this.workspace); defBlock.dispose(); - this.clock.runAll(); + chai.assert.isTrue( callBlock1.disposed, 'Expected the first caller to be disposed'); chai.assert.isTrue( @@ -1574,25 +1539,37 @@ suite('Procedures', function() { function assertDefAndCallBlocks(workspace, noReturnNames, returnNames, hasCallers) { const allProcedures = Blockly.Procedures.allProcedures(workspace); const defNoReturnBlocks = allProcedures[0]; - chai.assert.lengthOf(defNoReturnBlocks, noReturnNames.length); + chai.assert.lengthOf( + defNoReturnBlocks, + noReturnNames.length, + `Expected the number of no return blocks to be ${noReturnNames.length}`); for (let i = 0; i < noReturnNames.length; i++) { const expectedName = noReturnNames[i]; chai.assert.equal(defNoReturnBlocks[i][0], expectedName); if (hasCallers) { const callers = Blockly.Procedures.getCallers(expectedName, workspace); - chai.assert.lengthOf(callers, 1); + chai.assert.lengthOf( + callers, + 1, + `Expected there to be one caller of the ${expectedName} block`); } } const defReturnBlocks = allProcedures[1]; - chai.assert.lengthOf(defReturnBlocks, returnNames.length); + chai.assert.lengthOf( + defReturnBlocks, + returnNames.length, + `Expected the number of return blocks to be ${returnNames.length}`); for (let i = 0; i < returnNames.length; i++) { const expectedName = returnNames[i]; chai.assert.equal(defReturnBlocks[i][0], expectedName); if (hasCallers) { const callers = Blockly.Procedures.getCallers(expectedName, workspace); - chai.assert.lengthOf(callers, 1); + chai.assert.lengthOf( + callers, + 1, + `Expected there to be one caller of the ${expectedName} block`); } } @@ -1613,6 +1590,8 @@ suite('Procedures', function() { `); Blockly.Xml.domToWorkspace(xml, this.workspace); + this.clock.runAll(); + assertDefAndCallBlocks( this.workspace, ['unnamed'], ['unnamed2'], false); }); @@ -1624,6 +1603,8 @@ suite('Procedures', function() { `); Blockly.Xml.domToWorkspace(xml, this.workspace); + this.clock.runAll(); + assertDefAndCallBlocks( this.workspace, ['unnamed2'], ['unnamed'], false); }); @@ -1639,11 +1620,11 @@ suite('Procedures', function() { this.workspace, [], ['unnamed'], true); }); - test('callnoreturn and callreturn (no def in xml)', function() { + test.skip('callnoreturn and callreturn (no def in xml)', function() { const xml = Blockly.Xml.textToDom(` - - + + `); Blockly.Xml.domToWorkspace(xml, this.workspace); this.clock.runAll(); @@ -1651,7 +1632,7 @@ suite('Procedures', function() { this.workspace, ['unnamed'], ['unnamed2'], true); }); - test('callreturn and callnoreturn (no def in xml)', function() { + test.skip('callreturn and callnoreturn (no def in xml)', function() { const xml = Blockly.Xml.textToDom(` From 179535cc979386dbbdd84fa380d3936c691e5fa0 Mon Sep 17 00:00:00 2001 From: Beka Westberg Date: Thu, 22 Dec 2022 18:20:21 +0000 Subject: [PATCH 05/10] chore: format --- blocks/procedures.js | 22 +++++++++---------- core/procedures/observable_parameter_model.ts | 3 ++- 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/blocks/procedures.js b/blocks/procedures.js index 74caa0405f3..d1f4bd5f6ba 100644 --- a/blocks/procedures.js +++ b/blocks/procedures.js @@ -603,16 +603,15 @@ const procedureDefMutator = { const state = Object.create(null); if (params.length) { - state['params'] = - params.map((p) => { - return { - 'name': p.getName(), - 'id': p.getVariableModel().getId(), - // Ideally this would be id, and the other would be varId, - // but backwards compatibility :/ - 'paramId': p.getId(), - }; - }); + state['params'] = params.map((p) => { + return { + 'name': p.getName(), + 'id': p.getVariableModel().getId(), + // Ideally this would be id, and the other would be varId, + // but backwards compatibility :/ + 'paramId': p.getId(), + }; + }); } if (!this.hasStatements_) { state['hasStatements'] = false; @@ -630,8 +629,7 @@ const procedureDefMutator = { for (let i = 0; i < state['params'].length; i++) { const {name, id, paramId} = state['params'][i]; this.getProcedureModel().insertParameter( - new ObservableParameterModel(this.workspace, name, paramId, id), - i); + new ObservableParameterModel(this.workspace, name, paramId, id), i); } } diff --git a/core/procedures/observable_parameter_model.ts b/core/procedures/observable_parameter_model.ts index c58463375cd..e5adb53ba4f 100644 --- a/core/procedures/observable_parameter_model.ts +++ b/core/procedures/observable_parameter_model.ts @@ -22,7 +22,8 @@ export class ObservableParameterModel implements IParameterModel { private procedureModel: IProcedureModel|null = null; constructor( - private readonly workspace: Workspace, name: string, id?: string, varId?: string) { + private readonly workspace: Workspace, name: string, id?: string, + varId?: string) { this.id = id ?? genUid(); this.variable = this.workspace.getVariable(name) ?? workspace.createVariable(name, '', varId); From 2facdad15856da27ad88e5d8c5e3f086e9b7ab7e Mon Sep 17 00:00:00 2001 From: Beka Westberg Date: Thu, 22 Dec 2022 18:58:47 +0000 Subject: [PATCH 06/10] chore: add tests for deleting procedure parameters --- tests/mocha/blocks/procedures_test.js | 85 +++++++++++++++++++++++++++ 1 file changed, 85 insertions(+) diff --git a/tests/mocha/blocks/procedures_test.js b/tests/mocha/blocks/procedures_test.js index 79ee41c776d..a62496eff03 100644 --- a/tests/mocha/blocks/procedures_test.js +++ b/tests/mocha/blocks/procedures_test.js @@ -885,6 +885,91 @@ suite('Procedures', function() { }); }); + suite('deleting procedure parameters', function() { + test('deleting a parameter from the procedure updates procedure defs', function() { + // Create a stack of container, parameter. + const defBlock = createProcDefBlock(this.workspace); + defBlock.mutator.setVisible(true); + const mutatorWorkspace = defBlock.mutator.getWorkspace(); + const containerBlock = mutatorWorkspace.getTopBlocks()[0]; + const paramBlock = mutatorWorkspace.newBlock('procedures_mutatorarg'); + paramBlock.setFieldValue('param1', 'NAME'); + containerBlock.getInput('STACK').connection.connect(paramBlock.previousConnection); + this.clock.runAll(); + + paramBlock.checkAndDelete(); + this.clock.runAll(); + + chai.assert.isFalse( + defBlock.getFieldValue('PARAMS').includes('param1'), + 'Expected the params field to not contain the name of the new param'); + }); + + test('deleting a parameter from the procedure udpates procedure callers', function() { + // Create a stack of container, parameter. + const defBlock = createProcDefBlock(this.workspace); + const callBlock = createProcCallBlock(this.workspace); + defBlock.mutator.setVisible(true); + const mutatorWorkspace = defBlock.mutator.getWorkspace(); + const containerBlock = mutatorWorkspace.getTopBlocks()[0]; + const paramBlock = mutatorWorkspace.newBlock('procedures_mutatorarg'); + paramBlock.setFieldValue('param1', 'NAME'); + containerBlock.getInput('STACK').connection.connect(paramBlock.previousConnection); + this.clock.runAll(); + + paramBlock.checkAndDelete(); + this.clock.runAll(); + + chai.assert.isNull( + callBlock.getInput('ARG0'), + 'Expected the param input to not exist'); + }); + + test('undoing deleting a procedure parameter adds it', function() { + // Create a stack of container, parameter. + const defBlock = createProcDefBlock(this.workspace); + defBlock.mutator.setVisible(true); + const mutatorWorkspace = defBlock.mutator.getWorkspace(); + const containerBlock = mutatorWorkspace.getTopBlocks()[0]; + const paramBlock = mutatorWorkspace.newBlock('procedures_mutatorarg'); + paramBlock.setFieldValue('param1', 'NAME'); + containerBlock.getInput('STACK').connection.connect(paramBlock.previousConnection); + this.clock.runAll(); + paramBlock.checkAndDelete(); + this.clock.runAll(); + + this.workspace.undo(); + + chai.assert.isTrue( + defBlock.getFieldValue('PARAMS').includes('param1'), + 'Expected the params field to contain the name of the new param'); + }); + + test('undoing and redoing deleting a procedure parameter maintains ' + + 'the same state', + function() { + // Create a stack of container, parameter. + const defBlock = createProcDefBlock(this.workspace); + defBlock.mutator.setVisible(true); + const mutatorWorkspace = defBlock.mutator.getWorkspace(); + const containerBlock = mutatorWorkspace.getTopBlocks()[0]; + const paramBlock = mutatorWorkspace.newBlock('procedures_mutatorarg'); + paramBlock.setFieldValue('param1', 'NAME'); + containerBlock.getInput('STACK').connection + .connect(paramBlock.previousConnection); + this.clock.runAll(); + paramBlock.checkAndDelete(); + this.clock.runAll(); + + this.workspace.undo(); + this.workspace.undo(/* redo= */ true); + + chai.assert.isFalse( + defBlock.getFieldValue('PARAMS').includes('param1'), + 'Expected the params field to not contain the name of the new param'); + }); + }); + suite('renaming procedure parameters', function() { test('defs are updated for parameter renames', function() { // Create a stack of container, parameter. From ac744b4dda6ef310ae60fcce623954cc9cc14271 Mon Sep 17 00:00:00 2001 From: Beka Westberg Date: Fri, 6 Jan 2023 18:56:28 +0000 Subject: [PATCH 07/10] chore: fix tests --- blocks/procedures.js | 6 +++- tests/mocha/blocks/procedures_test.js | 43 ++++++++++++++++----------- 2 files changed, 30 insertions(+), 19 deletions(-) diff --git a/blocks/procedures.js b/blocks/procedures.js index d1f4bd5f6ba..ff93cadf172 100644 --- a/blocks/procedures.js +++ b/blocks/procedures.js @@ -720,7 +720,7 @@ const procedureDefMutator = { Procedures.mutateCallers(this); const model = this.getProcedureModel(); - const count = this.getProcedureModel().getParameters().length; + const count = model.getParameters().length; for (let i = count - 1; i >= 0; i--) { model.deleteParameter(i); } @@ -1212,6 +1212,10 @@ const procedureCallerUpdateShapeMixin = { this.updateName_(); this.updateEnabled_(); this.updateParameters_(); + + // Temporarily maintained for code that relies on arguments_ + this.arguments_ = + this.getProcedureModel().getParameters().map((p) => p.getName()); }, /** diff --git a/tests/mocha/blocks/procedures_test.js b/tests/mocha/blocks/procedures_test.js index a62496eff03..831556ee6b8 100644 --- a/tests/mocha/blocks/procedures_test.js +++ b/tests/mocha/blocks/procedures_test.js @@ -9,7 +9,6 @@ goog.declareModuleId('Blockly.test.procedures'); import * as Blockly from '../../../build/src/core/blockly.js'; import {ObservableParameterModel} from '../../../build/src/core/procedures.js'; import {assertCallBlockStructure, assertDefBlockStructure, createProcDefBlock, createProcCallBlock} from '../test_helpers/procedures.js'; -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'; @@ -1307,6 +1306,7 @@ suite('Procedures', function() { function() { const defBlock = createProcDefBlock(this.workspace); const callBlock = createProcCallBlock(this.workspace); + this.clock.runAll(); callBlock.setEnabled(false); this.clock.runAll(); defBlock.setEnabled(false); @@ -2049,15 +2049,18 @@ suite('Procedures', function() { suite('Mutation', function() { setup(function() { - this.defBlock = this.workspace.newBlock(testSuite.defType); - this.defBlock.setFieldValue('proc name', 'NAME'); - this.callBlock = this.workspace.newBlock(testSuite.callType); - this.callBlock.setFieldValue('proc name', 'NAME'); - this.findParentStub = sinon.stub(Blockly.Mutator, 'findParentWs') - .returns(this.workspace); - }); - teardown(function() { - this.findParentStub.restore(); + this.defBlock = Blockly.serialization.blocks.append({ + 'type': testSuite.defType, + 'fields': { + 'NAME': 'proc name', + }, + }, this.workspace); + this.callBlock = Blockly.serialization.blocks.append({ + 'type': testSuite.callType, + 'extraState': { + 'name': 'proc name', + }, + }, this.workspace); }); suite('Composition', function() { suite('Statements', function() { @@ -2104,11 +2107,9 @@ suite('Procedures', function() { }); suite('Untyped Arguments', function() { function createMutator(argArray) { - this.mutatorWorkspace = new Blockly.Workspace( - new Blockly.Options({ - parentWorkspace: this.workspace, - })); - this.containerBlock = this.defBlock.decompose(this.mutatorWorkspace); + this.defBlock.mutator.setVisible(true); + this.mutatorWorkspace = this.defBlock.mutator.getWorkspace(); + this.containerBlock = this.mutatorWorkspace.getTopBlocks()[0]; this.connection = this.containerBlock.getInput('STACK').connection; for (let i = 0; i < argArray.length; i++) { this.argBlock = this.mutatorWorkspace.newBlock('procedures_mutatorarg'); @@ -2116,14 +2117,20 @@ suite('Procedures', function() { this.connection.connect(this.argBlock.previousConnection); this.connection = this.argBlock.nextConnection; } - this.defBlock.compose(this.containerBlock); + this.clock.runAll(); } function assertArgs(argArray) { - chai.assert.equal(this.defBlock.arguments_.length, argArray.length); + chai.assert.equal( + this.defBlock.arguments_.length, + argArray.length, + 'Expected the def to have the right number of arguments'); for (let i = 0; i < argArray.length; i++) { chai.assert.equal(this.defBlock.arguments_[i], argArray[i]); } - chai.assert.equal(this.callBlock.arguments_.length, argArray.length); + chai.assert.equal( + this.callBlock.arguments_.length, + argArray.length, + 'Expected the call to have the right number of arguments'); for (let i = 0; i < argArray.length; i++) { chai.assert.equal(this.callBlock.arguments_[i], argArray[i]); } From 75792424fefd2f22dc7b9cbec7d08d6faaaf1e0f Mon Sep 17 00:00:00 2001 From: Beka Westberg Date: Fri, 6 Jan 2023 22:34:32 +0000 Subject: [PATCH 08/10] chore: unskip tests --- tests/mocha/blocks/procedures_test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/mocha/blocks/procedures_test.js b/tests/mocha/blocks/procedures_test.js index 831556ee6b8..7e4f9a3fa98 100644 --- a/tests/mocha/blocks/procedures_test.js +++ b/tests/mocha/blocks/procedures_test.js @@ -1705,7 +1705,7 @@ suite('Procedures', function() { this.workspace, [], ['unnamed'], true); }); - test.skip('callnoreturn and callreturn (no def in xml)', function() { + test('callnoreturn and callreturn (no def in xml)', function() { const xml = Blockly.Xml.textToDom(` @@ -1717,7 +1717,7 @@ suite('Procedures', function() { this.workspace, ['unnamed'], ['unnamed2'], true); }); - test.skip('callreturn and callnoreturn (no def in xml)', function() { + test('callreturn and callnoreturn (no def in xml)', function() { const xml = Blockly.Xml.textToDom(` From 2b66e5e9a755f6c9ae454ee0540f63d174e454d9 Mon Sep 17 00:00:00 2001 From: Beka Westberg Date: Fri, 6 Jan 2023 23:05:38 +0000 Subject: [PATCH 09/10] chore: fix return type of saveExtraState --- blocks/procedures.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/blocks/procedures.js b/blocks/procedures.js index ff93cadf172..419631d5289 100644 --- a/blocks/procedures.js +++ b/blocks/procedures.js @@ -599,7 +599,7 @@ const procedureDefMutator = { */ saveExtraState: function() { const params = this.getProcedureModel().getParameters(); - if (!params.length && this.hasStatements_) return; + if (!params.length && this.hasStatements_) return null; const state = Object.create(null); if (params.length) { From cbdcc4035db51f00e7e911ca93dd57511cecc9d6 Mon Sep 17 00:00:00 2001 From: Beka Westberg Date: Fri, 6 Jan 2023 23:22:38 +0000 Subject: [PATCH 10/10] chore: increase mocha timeout --- tests/mocha/webdriver.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/mocha/webdriver.js b/tests/mocha/webdriver.js index e7eb99bc769..e469212348c 100644 --- a/tests/mocha/webdriver.js +++ b/tests/mocha/webdriver.js @@ -53,7 +53,7 @@ async function runMochaTestsInBrowser() { const text = await elem.getAttribute('tests_failed'); return text !== 'unset'; }, { - timeout: 50000, + timeout: 100000, }); const elem = await browser.$('#failureCount');