From d58844d23cb41974ad72e428e299fe7e4dfd17c1 Mon Sep 17 00:00:00 2001 From: Beka Westberg Date: Wed, 19 Oct 2022 10:42:32 -0700 Subject: [PATCH] Revert "chore: update procedure map tests to match the refactored API (#6562)" (#6568) This reverts commit 5f70fc415bfde65dcc2c9832b2b680cefcfb39f7. --- core/blockly.ts | 1 - core/procedures.ts | 9 - core/procedures/observable_parameter_model.ts | 10 +- core/procedures/observable_procedure_map.ts | 9 +- core/procedures/observable_procedure_model.ts | 5 - tests/mocha/procedure_map_test.js | 155 +++++------------- 6 files changed, 50 insertions(+), 139 deletions(-) diff --git a/core/blockly.ts b/core/blockly.ts index 41946d4d4..71c19f77d 100644 --- a/core/blockly.ts +++ b/core/blockly.ts @@ -602,7 +602,6 @@ export {Css}; export {Events}; export {Extensions}; export {Procedures}; -export {Procedures as procedures}; export {ShortcutItems}; export {Themes}; export {Tooltip}; diff --git a/core/procedures.ts b/core/procedures.ts index e31d77d31..7fad925e6 100644 --- a/core/procedures.ts +++ b/core/procedures.ts @@ -25,9 +25,6 @@ import * as eventUtils from './events/utils.js'; import {Field, UnattachedFieldError} from './field.js'; import {Msg} from './msg.js'; import {Names} from './names.js'; -import {ObservableProcedureMap} from './procedures/observable_procedure_map.js'; -import {ObservableProcedureModel} from './procedures/observable_procedure_model.js'; -import {ObservableParameterModel} from './procedures/observable_parameter_model.js'; import * as utilsXml from './utils/xml.js'; import * as Variables from './variables.js'; import type {Workspace} from './workspace.js'; @@ -453,9 +450,3 @@ export function getDefinition(name: string, workspace: Workspace): Block|null { } return null; } - -export { - ObservableProcedureMap, - ObservableProcedureModel, - ObservableParameterModel, -}; diff --git a/core/procedures/observable_parameter_model.ts b/core/procedures/observable_parameter_model.ts index 407b0ad6d..4c74a0505 100644 --- a/core/procedures/observable_parameter_model.ts +++ b/core/procedures/observable_parameter_model.ts @@ -17,15 +17,13 @@ export class ObservableParameterModel implements IParameterModel { constructor( private readonly workspace: Workspace, name: string, id?: string) { this.id = id ?? genUid(); - this.variable = - this.workspace.getVariable(name) ?? workspace.createVariable(name); + this.variable = workspace.createVariable(name); } /** * Sets the name of this parameter to the given name. */ setName(name: string): this { - // TODO(#6516): Fire events. if (name == this.variable.name) return this; this.variable = this.workspace.getVariable(name) ?? this.workspace.createVariable(name); @@ -36,14 +34,12 @@ export class ObservableParameterModel implements IParameterModel { * Unimplemented. The built-in ParameterModel does not support typing. * If you want your procedure blocks to have typed parameters, you need to * implement your own ParameterModel. - * - * @throws Throws for the ObservableParameterModel specifically because this - * method is unimplemented. */ setTypes(_types: string[]): this { - throw new Error( + console.warn( 'The built-in ParameterModel does not support typing. You need to ' + 'implement your own custom ParameterModel.'); + return this; } /** diff --git a/core/procedures/observable_procedure_map.ts b/core/procedures/observable_procedure_map.ts index 35ee4a5e7..db5edfe68 100644 --- a/core/procedures/observable_procedure_map.ts +++ b/core/procedures/observable_procedure_map.ts @@ -17,7 +17,7 @@ export class ObservableProcedureMap extends Map { * Adds the given procedure model to the procedure map. */ override set(id: string, proc: IProcedureModel): this { - // TODO(#6516): Fire events. + // TODO(#6156): Fire events. super.set(id, proc); return this; } @@ -27,7 +27,7 @@ export class ObservableProcedureMap extends Map { * exists). */ override delete(id: string): boolean { - // TODO(#6516): Fire events. + // TODO(#6156): Fire events. return super.delete(id); } @@ -35,7 +35,7 @@ export class ObservableProcedureMap extends Map { * Removes all ProcedureModels from the procedure map. */ override clear() { - // TODO(#6516): Fire events. + // TODO(#6156): Fire events. super.clear(); } @@ -44,8 +44,7 @@ export class ObservableProcedureMap extends Map { * blocks can find it. */ add(proc: IProcedureModel): this { - // TODO(#6516): Fire events. - // TODO(#6526): See if this method is actually useful. + // TODO(#6156): Fire events. return this.set(proc.getId(), proc); } } diff --git a/core/procedures/observable_procedure_model.ts b/core/procedures/observable_procedure_model.ts index aa51bb7f9..335667ab9 100644 --- a/core/procedures/observable_procedure_model.ts +++ b/core/procedures/observable_procedure_model.ts @@ -23,7 +23,6 @@ export class ObservableProcedureModel implements IProcedureModel { /** Sets the human-readable name of the procedure. */ setName(name: string): this { - // TODO(#6516): Fire events. this.name = name; return this; } @@ -34,14 +33,12 @@ export class ObservableProcedureModel implements IProcedureModel { * To move a parameter, first delete it, and then re-insert. */ insertParameter(parameterModel: IParameterModel, index: number): this { - // TODO(#6516): Fire events. this.parameters.splice(index, 0, parameterModel); return this; } /** Removes the parameter at the given index from the parameter list. */ deleteParameter(index: number): this { - // TODO(#6516): Fire events. this.parameters.splice(index, 1); return this; } @@ -52,7 +49,6 @@ export class ObservableProcedureModel implements IProcedureModel { * Pass null to represent a procedure that does not return. */ setReturnTypes(types: string[]|null): this { - // TODO(#6516): Fire events. this.returnTypes = types; return this; } @@ -62,7 +58,6 @@ export class ObservableProcedureModel implements IProcedureModel { * all procedure caller blocks should be disabled as well. */ setEnabled(enabled: boolean): this { - // TODO(#6516): Fire events. this.enabled = enabled; return this; } diff --git a/tests/mocha/procedure_map_test.js b/tests/mocha/procedure_map_test.js index 60a84ad37..08e5f8137 100644 --- a/tests/mocha/procedure_map_test.js +++ b/tests/mocha/procedure_map_test.js @@ -8,19 +8,18 @@ import {sharedTestSetup, sharedTestTeardown} from './test_helpers/setup_teardown goog.declareModuleId('Blockly.test.procedureMap'); -suite('Procedure Map', function() { +suite.skip('Procedure Map', function() { setup(function() { sharedTestSetup.call(this); this.workspace = new Blockly.Workspace(); - // this.procedureMap = this.workspace.getProcedureMap(); + this.procedureMap = this.workspace.getProcedureMap(); }); teardown(function() { sharedTestTeardown.call(this); }); - // TODO (#6515): Unskip tests. - suite.skip('triggering block updates', function() { + suite('triggering block updates', function() { setup(function() { Blockly.Blocks['procedure_mock'] = { init: function() { }, @@ -37,17 +36,8 @@ suite('Procedure Map', function() { }); suite('procedure map updates', function() { - test('inserting a procedure does not trigger an update', function() { - const procedureModel = - new Blockly.procedures.ObservableProcedureModel(this.workspace); - this.procedureMap.set(procedureModel.getId(), procedureModel); - - chai.assert.isFalse( - this.updateSpy.called, 'Expected no update to be triggered'); - }); - test('adding a procedure does not trigger an update', function() { - this.procedureMap.add( + this.procedureMap.addProcedure( new Blockly.procedures.ObservableProcedureModel(this.workspace)); chai.assert.isFalse( @@ -57,9 +47,9 @@ suite('Procedure Map', function() { test('deleting a procedure triggers an update', function() { const procedureModel = new Blockly.procedures.ObservableProcedureModel(this.workspace); - this.procedureMap.add(procedureModel); + this.procedureMap.addProcedure(procedureModel); - this.procedureMap.delete(procedureModel.getId()); + this.procedureMap.deleteProcedure(procedureModel.getId()); chai.assert.isTrue( this.updateSpy.calledOnce, 'Expected an update to be triggered'); @@ -70,7 +60,7 @@ suite('Procedure Map', function() { test('setting the name triggers an update', function() { const procedureModel = new Blockly.procedures.ObservableProcedureModel(this.workspace); - this.procedureMap.add(procedureModel); + this.procedureMap.addProcedure(procedureModel); procedureModel.setName('new name'); @@ -81,9 +71,9 @@ suite('Procedure Map', function() { test('setting the return type triggers an update', function() { const procedureModel = new Blockly.procedures.ObservableProcedureModel(this.workspace); - this.procedureMap.add(procedureModel); + this.procedureMap.addProcedure(procedureModel); - procedureModel.setReturnTypes(['return type 1', 'return type 2']); + procedureModel.setReturnType(['return type 1', 'return type 2']); chai.assert.isTrue( this.updateSpy.calledOnce, 'Expected an update to be triggered'); @@ -91,12 +81,12 @@ suite('Procedure Map', function() { test('removing the return type triggers an update', function() { const procedureModel = - new Blockly.procedures.ObservableProcedureModel(this.workspace) - .setReturnTypes(['return type']); - this.procedureMap.add(procedureModel); - this.updateSpy.resetHistory(); + new Blockly.procedures.ObservableProcedureModel(this.workspace); + this.procedureMap.addProcedure(procedureModel); - procedureModel.setReturnTypes(null); + procedureModel.setReturnType(['return type']); + this.updateSpy.reset(); + procedureModel.setReturnType(null); chai.assert.isTrue( this.updateSpy.calledOnce, 'Expected an update to be triggered'); @@ -105,7 +95,7 @@ suite('Procedure Map', function() { test('disabling the procedure triggers an update', function() { const procedureModel = new Blockly.procedures.ObservableProcedureModel(this.workspace); - this.procedureMap.add(procedureModel); + this.procedureMap.addProcedure(procedureModel); procedureModel.setEnabled(false); @@ -115,10 +105,10 @@ suite('Procedure Map', function() { test('enabling the procedure triggers an update', function() { const procedureModel = - new Blockly.procedures.ObservableProcedureModel(this.workspace) - .setEnabled(false); - this.procedureMap.add(procedureModel); - this.updateSpy.resetHistory(); + new Blockly.procedures.ObservableProcedureModel(this.workspace); + this.procedureMap.addProcedure(procedureModel); + procedureModel.setEnabled(false); + this.updateSpy.reset(); procedureModel.setEnabled(true); @@ -129,7 +119,7 @@ suite('Procedure Map', function() { test('inserting a parameter triggers an update', function() { const procedureModel = new Blockly.procedures.ObservableProcedureModel(this.workspace); - this.procedureMap.add(procedureModel); + this.procedureMap.addProcedure(procedureModel); procedureModel.insertParameter( new Blockly.procedures.ObservableParameterModel(this.workspace)); @@ -140,12 +130,11 @@ suite('Procedure Map', function() { test('deleting a parameter triggers an update', function() { const procedureModel = - new Blockly.procedures.ObservableProcedureModel(this.workspace) - .insertParameter( - new Blockly.procedures.ObservableParameterModel( - this.workspace)); - this.procedureMap.add(procedureModel); - this.updateSpy.resetHistory(); + new Blockly.procedures.ObservableProcedureModel(this.workspace); + this.procedureMap.addProcedure(procedureModel); + procedureModel.insertParameter( + new Blockly.procedures.ObservableParameterModel(this.workspace)); + this.updateSpy.reset(); procedureModel.deleteParameter(0); @@ -155,31 +144,34 @@ suite('Procedure Map', function() { }); suite('parameter model updates', function() { - test('setting the name triggers an update', function() { + test('setting the variable model triggers an update', function() { + const procedureModel = + new Blockly.procedures.ObservableProcedureModel(this.workspace); + this.procedureMap.addProcedure(procedureModel); const parameterModel = - new Blockly.procedures.ObservableParameterModel( - this.workspace, 'test1'); - this.procedureMap.add( - new Blockly.procedures.ObservableProcedureModel(this.workspace) - .insertParameter(parameterModel)); - this.updateSpy.resetHistory(); + new Blockly.procedures.ObservableParameterModel(this.workspace); + procedureModel.insertParameter(parameterModel); + this.updateSpy.reset(); - parameterModel.setName('test2'); + parameterModel.setVariable( + new Blockly.VariableModel(this.workspace, 'variable')); chai.assert.isTrue( this.updateSpy.calledOnce, 'Expected an update to be triggered'); }); test('modifying the variable model does not trigger an update', function() { + const procedureModel = + new Blockly.procedures.ObservableProcedureModel(this.workspace); + this.procedureMap.addProcedure(procedureModel); const parameterModel = - new Blockly.procedures.ObservableParameterModel( - this.workspace, 'test1'); - this.procedureMap.add( - new Blockly.procedures.ObservableProcedureModel(this.workspace) - .insertParameter(parameterModel)); - this.updateSpy.resetHistory(); + new Blockly.procedures.ObservableParameterModel(this.workspace); + procedureModel.insertParameter(parameterModel); + const variableModel = + new Blockly.VariableModel(this.workspace, 'variable'); + parameterModel.setVariable(variableModel); + this.updateSpy.reset(); - const variableModel = parameterModel.getVariableModel(); variableModel.name = 'some name'; variableModel.type = 'some type'; @@ -192,65 +184,4 @@ suite('Procedure Map', function() { suite('event firing', function() { // TBA after the procedure map is implemented }); - - suite('backing variable to parameters', function() { - test( - 'construction references an existing variable if available', - function() { - const variable = this.workspace.createVariable('test1'); - const param = new Blockly.procedures.ObservableParameterModel( - this.workspace, 'test1'); - - chai.assert.equal( - variable, - param.getVariableModel(), - 'Expected the parameter model to reference the existing variable'); - }); - - test('construction creates a variable if none exists', function() { - const param = new Blockly.procedures.ObservableParameterModel( - this.workspace, 'test1'); - - chai.assert.equal( - this.workspace.getVariable('test1'), - param.getVariableModel(), - 'Expected the parameter model to create a variable'); - }); - - test('setName references an existing variable if available', function() { - const variable = this.workspace.createVariable('test2'); - const param = new Blockly.procedures.ObservableParameterModel( - this.workspace, 'test1'); - - param.setName('test2'); - - chai.assert.equal( - variable, - param.getVariableModel(), - 'Expected the parameter model to reference the existing variable'); - }); - - test('setName creates a variable if none exits', function() { - const param = new Blockly.procedures.ObservableParameterModel( - this.workspace, 'test1'); - - param.setName('test2'); - - chai.assert.equal( - this.workspace.getVariable('test2'), - param.getVariableModel(), - 'Expected the parameter model to create a variable'); - }); - - test('setTypes is unimplemented', function() { - const param = new Blockly.procedures.ObservableParameterModel( - this.workspace, 'test1'); - - chai.assert.throws( - () => { - param.setTypes(['some', 'types']); - }, - 'The built-in ParameterModel does not support typing'); - }); - }); });