chore: update procedure map tests to match the refactored API (#6569)

* chore: add tests for the backing variable of parameter models

* chore: update existing procedure map tests

* chore: update block update tests to use refactored API

* chore: update tests to actually use fluent API

* chore: format

* chore: fix tests

* chore: reorganize tests

* chore: format

* chore: add comment

core/blockly.ts

@@ -602,6 +602,7 @@ export {Css};
 export {Events};
 export {Extensions};
 export {Procedures};
+export {Procedures as procedures};
 export {ShortcutItems};
 export {Themes};
 export {Tooltip};

core/procedures.ts

@@ -25,6 +25,9 @@ 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';
@@ -450,3 +453,9 @@ export function getDefinition(name: string, workspace: Workspace): Block|null {
   }
   return null;
 }
+
+export {
+  ObservableProcedureMap,
+  ObservableProcedureModel,
+  ObservableParameterModel,
+};

core/procedures/observable_parameter_model.ts

@@ -25,6 +25,7 @@ export class ObservableParameterModel implements IParameterModel {
    * 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);

core/procedures/observable_procedure_model.ts

@@ -23,6 +23,7 @@ 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;
   }
@@ -33,12 +34,14 @@ 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;
   }
@@ -49,6 +52,7 @@ 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;
   }
@@ -58,6 +62,7 @@ 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;
   }

tests/mocha/procedure_map_test.js

@@ -8,18 +8,19 @@ import {sharedTestSetup, sharedTestTeardown} from './test_helpers/setup_teardown
 
 goog.declareModuleId('Blockly.test.procedureMap');
 
-suite.skip('Procedure Map', function() {
+suite('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);
   });
 
-  suite('triggering block updates', function() {
+  // TODO (#6515): Unskip tests.
+  suite.skip('triggering block updates', function() {
     setup(function() {
       Blockly.Blocks['procedure_mock'] = {
         init: function() { },
@@ -36,8 +37,17 @@ suite.skip('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.addProcedure(
+        this.procedureMap.add(
             new Blockly.procedures.ObservableProcedureModel(this.workspace));
 
         chai.assert.isFalse(
@@ -47,9 +57,9 @@ suite.skip('Procedure Map', function() {
       test('deleting a procedure triggers an update', function() {
         const procedureModel =
             new Blockly.procedures.ObservableProcedureModel(this.workspace);
-        this.procedureMap.addProcedure(procedureModel);
+        this.procedureMap.add(procedureModel);
 
-        this.procedureMap.deleteProcedure(procedureModel.getId());
+        this.procedureMap.delete(procedureModel.getId());
 
         chai.assert.isTrue(
             this.updateSpy.calledOnce, 'Expected an update to be triggered');
@@ -60,7 +70,7 @@ suite.skip('Procedure Map', function() {
       test('setting the name triggers an update', function() {
         const procedureModel =
             new Blockly.procedures.ObservableProcedureModel(this.workspace);
-        this.procedureMap.addProcedure(procedureModel);
+        this.procedureMap.add(procedureModel);
 
         procedureModel.setName('new name');
 
@@ -71,9 +81,9 @@ suite.skip('Procedure Map', function() {
       test('setting the return type triggers an update', function() {
         const procedureModel =
             new Blockly.procedures.ObservableProcedureModel(this.workspace);
-        this.procedureMap.addProcedure(procedureModel);
+        this.procedureMap.add(procedureModel);
 
-        procedureModel.setReturnType(['return type 1', 'return type 2']);
+        procedureModel.setReturnTypes(['return type 1', 'return type 2']);
 
         chai.assert.isTrue(
             this.updateSpy.calledOnce, 'Expected an update to be triggered');
@@ -81,12 +91,12 @@ suite.skip('Procedure Map', function() {
   
       test('removing the return type triggers an update', function() {
         const procedureModel =
-            new Blockly.procedures.ObservableProcedureModel(this.workspace);
-        this.procedureMap.addProcedure(procedureModel);
+            new Blockly.procedures.ObservableProcedureModel(this.workspace)
+                .setReturnTypes(['return type']);
+        this.procedureMap.add(procedureModel);
+        this.updateSpy.resetHistory();
 
-        procedureModel.setReturnType(['return type']);
-        this.updateSpy.reset();
-        procedureModel.setReturnType(null);
+        procedureModel.setReturnTypes(null);
 
         chai.assert.isTrue(
             this.updateSpy.calledOnce, 'Expected an update to be triggered');
@@ -95,7 +105,7 @@ suite.skip('Procedure Map', function() {
       test('disabling the procedure triggers an update', function() {
         const procedureModel =
             new Blockly.procedures.ObservableProcedureModel(this.workspace);
-        this.procedureMap.addProcedure(procedureModel);
+        this.procedureMap.add(procedureModel);
 
         procedureModel.setEnabled(false);
 
@@ -105,10 +115,10 @@ suite.skip('Procedure Map', function() {
   
       test('enabling the procedure triggers an update', function() {
         const procedureModel =
-            new Blockly.procedures.ObservableProcedureModel(this.workspace);
-        this.procedureMap.addProcedure(procedureModel);
-        procedureModel.setEnabled(false);
-        this.updateSpy.reset();
+            new Blockly.procedures.ObservableProcedureModel(this.workspace)
+                .setEnabled(false);
+        this.procedureMap.add(procedureModel);
+        this.updateSpy.resetHistory();
 
         procedureModel.setEnabled(true);
 
@@ -119,7 +129,7 @@ suite.skip('Procedure Map', function() {
       test('inserting a parameter triggers an update', function() {
         const procedureModel =
             new Blockly.procedures.ObservableProcedureModel(this.workspace);
-        this.procedureMap.addProcedure(procedureModel);
+        this.procedureMap.add(procedureModel);
 
         procedureModel.insertParameter(
             new Blockly.procedures.ObservableParameterModel(this.workspace));
@@ -130,11 +140,12 @@ suite.skip('Procedure Map', function() {
   
       test('deleting a parameter triggers an update', function() {
         const procedureModel =
-            new Blockly.procedures.ObservableProcedureModel(this.workspace);
-        this.procedureMap.addProcedure(procedureModel);
-        procedureModel.insertParameter(
-            new Blockly.procedures.ObservableParameterModel(this.workspace));
-        this.updateSpy.reset();
+            new Blockly.procedures.ObservableProcedureModel(this.workspace)
+                .insertParameter(
+                    new Blockly.procedures.ObservableParameterModel(
+                        this.workspace));
+        this.procedureMap.add(procedureModel);
+        this.updateSpy.resetHistory();
 
         procedureModel.deleteParameter(0);
 
@@ -144,34 +155,31 @@ suite.skip('Procedure Map', function() {
     });
 
     suite('parameter model updates', function() {
-      test('setting the variable model triggers an update', function() {
-        const procedureModel =
-            new Blockly.procedures.ObservableProcedureModel(this.workspace);
-        this.procedureMap.addProcedure(procedureModel);
+      test('setting the name triggers an update', function() {
         const parameterModel =
-            new Blockly.procedures.ObservableParameterModel(this.workspace);
-        procedureModel.insertParameter(parameterModel);
-        this.updateSpy.reset();
+            new Blockly.procedures.ObservableParameterModel(
+                this.workspace, 'test1');
+        this.procedureMap.add(
+            new Blockly.procedures.ObservableProcedureModel(this.workspace)
+                .insertParameter(parameterModel));
+        this.updateSpy.resetHistory();
 
-        parameterModel.setVariable(
-            new Blockly.VariableModel(this.workspace, 'variable'));
+        parameterModel.setName('test2');
 
         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);
-        procedureModel.insertParameter(parameterModel);
-        const variableModel =
-            new Blockly.VariableModel(this.workspace, 'variable');
-        parameterModel.setVariable(variableModel);
-        this.updateSpy.reset();
+            new Blockly.procedures.ObservableParameterModel(
+                this.workspace, 'test1');
+        this.procedureMap.add(
+            new Blockly.procedures.ObservableProcedureModel(this.workspace)
+                .insertParameter(parameterModel));
+        this.updateSpy.resetHistory();
 
+        const variableModel = parameterModel.getVariableModel();
         variableModel.name = 'some name';
         variableModel.type = 'some type';
 
@@ -184,4 +192,65 @@ suite.skip('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');
+    });
+  });
 });