[triggers] Support Item as parameter & Add arg type checks (#194)

* [test] Improve `typeOfArguments` test * [test] Globally mock `@runtime` & OSGi services * [triggers] Support Item as argument & Add type checks Unit tests for triggers can not be extended for Item as argument, because we need to mock Item first. * Remove old & obsolete type defs * Update CHANGELOG * Revert removal of our custom written type defs * Refactor mocking for test of triggers.js. (#1) * Cosmetic changes to comments * Update type defs * [test] triggers: Also test with Item as argument * [test] Fix typo in typeOfArguments.spec.js Also-by: Stefan Friedle <stefan@friedle.de> Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
openhab · Dec 19, 2022 · 0f05a60 · 0f05a60
1 parent 269b769
commit 0f05a60
Show file tree

Hide file tree

Showing 25 changed files with 260 additions and 279 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,8 +2,9 @@
 
 ## to be released
 
-| Type        | Namespace | Description                                 | Reference                                                                                               | Breaking |
-|-------------|-----------|---------------------------------------------|---------------------------------------------------------------------------------------------------------|----------|
+| Type        | Namespace  | Description                                              | Reference                                              | Breaking |
+|-------------|------------|----------------------------------------------------------|--------------------------------------------------------|----------|
+| Enhancement | `triggers` | Add support for `Item` as argument & Add arg type checks | [#194](https://github.com/openhab/openhab-js/pull/194) | No       |
 
 Also see the [Release Milestone](https://github.com/openhab/openhab-js/milestone/10).
 

diff --git a/jest.config.js b/jest.config.js
@@ -130,7 +130,7 @@ module.exports = {
   // runner: "jest-runner",
 
   // The paths to modules that run some code to configure or set up the testing environment before each test
-  setupFiles: ['./test/java.mock.js'],
+  setupFiles: ['./test/java.mock.js', './test/@runtime.mock.js'],
 
   // A list of paths to modules that run some code to configure or set up the testing framework before each test
   // setupFilesAfterEnv: [],

diff --git a/test/@runtime.mock.js b/test/@runtime.mock.js
@@ -0,0 +1,20 @@
+jest.mock('@runtime', () => ({
+  DateTimeType: jest.fn(),
+  DecimalType: jest.fn(),
+  StringType: jest.fn(),
+  QuantityType: jest.fn()
+}), { virtual: true });
+
+jest.mock('@runtime/Defaults', () => ({}), { virtual: true });
+
+jest.mock('@runtime/osgi', () => ({
+  bundleContext: {
+    getServiceReference: jest.fn(),
+    getService: jest.fn(),
+    getAllServiceReferences: jest.fn(),
+    registerService: jest.fn()
+  },
+  lifecycle: {
+    addDisposeHook: jest.fn()
+  }
+}), { virtual: true });
diff --git a/test/actions.spec.js b/test/actions.spec.js
@@ -4,8 +4,6 @@ const { ScriptExecution, Transformation } = require('../actions');
 const { JavaScriptExecution, JavaTransformation } = require('./openhab.mock');
 
 jest.mock('../osgi');
-jest.mock('@runtime/osgi', () => ({}), { virtual: true });
-jest.mock('@runtime/Defaults', () => ({}), { virtual: true });
 
 describe('actions.js', () => {
   describe('ScriptExecution', () => {

diff --git a/test/openhab.mock.js b/test/openhab.mock.js
@@ -1,3 +1,4 @@
+// org.openhab.core.automation.util.ModuleBuilder (https://www.openhab.org/javadoc/latest/org/openhab/core/automation/util/modulebuilder)
 class ModuleBuilder {
   constructor () {
     this.withId = jest.fn(() => this);
@@ -9,19 +10,29 @@ class ModuleBuilder {
 
 ModuleBuilder.createTrigger = jest.fn(() => new ModuleBuilder());
 
+// org.openhab.core.config.core.Configuration (https://www.openhab.org/javadoc/latest/org/openhab/core/config/core/configuration)
 class Configuration {
   constructor (config) {
     this.config = config;
   }
 }
 
+// org.openhab.core.items.MetadataRegistry (https://www.openhab.org/javadoc/latest/org/openhab/core/items/metadataregistry)
+class MetadataRegistry {
+  add () {}
+  get () {}
+  update () {}
+}
+
+// org.openhab.core.model.script.actions.ScriptExecution (https://www.openhab.org/javadoc/latest/org/openhab/core/model/script/actions/scriptexecution)
 class JavaScriptExecution {
   static callScript () {}
   static createTimer () {}
 }
 
+// org.openhab.core.transform.actions.Transformation (https://www.openhab.org/javadoc/latest/org/openhab/core/transform/actions/transformation)
 class JavaTransformation {}
 JavaTransformation.transform = jest.fn(() => 'on');
 JavaTransformation.transformRaw = jest.fn(() => 'on');
 
-module.exports = { Configuration, ModuleBuilder, JavaScriptExecution, JavaTransformation };
+module.exports = { Configuration, MetadataRegistry, ModuleBuilder, JavaScriptExecution, JavaTransformation };
diff --git a/test/osgi.spec.js b/test/osgi.spec.js
@@ -3,18 +3,6 @@ const { Hashtable } = require('./java.mock');
 const bundleContext = require('@runtime/osgi').bundleContext;
 const lifecycle = require('@runtime/osgi').lifecycle;
 
-jest.mock('@runtime/osgi', () => ({
-  bundleContext: {
-    getServiceReference: jest.fn(),
-    getService: jest.fn(),
-    getAllServiceReferences: jest.fn(),
-    registerService: jest.fn()
-  },
-  lifecycle: {
-    addDisposeHook: jest.fn()
-  }
-}), { virtual: true });
-
 describe('osgi.js', () => {
   describe('getService', () => {
     describe('looks up given service from bundle context', () => {

diff --git a/test/triggers.spec.js b/test/triggers.spec.js
@@ -18,6 +18,15 @@ const {
   PIDTrigger
 } = require('../triggers');
 
+jest.mock('../items', () => ({
+  Item: class {
+    constructor (name) {
+      this.name = name;
+    }
+  }
+}));
+const Item = require('../items').Item;
+
 describe('triggers.js', () => {
   const moduleBuilderSpy = new ModuleBuilder();
   ModuleBuilder.createTrigger.mockImplementation(() => moduleBuilderSpy);
@@ -32,11 +41,13 @@ describe('triggers.js', () => {
   });
 
   describe('ItemCommandTrigger', () => {
-    it('creates trigger.', () => {
-      const itemName = 'itemName';
-      const command = 'command';
-      const triggerName = 'triggerName';
-      ItemCommandTrigger(itemName, command, triggerName);
+    const itemName = 'itemName';
+    const command = 'command';
+    const triggerName = 'triggerName';
+    const item = new Item(itemName);
+
+    it.each([[itemName], [item]])('creates trigger from %s.', (itemOrName) => {
+      ItemCommandTrigger(itemOrName, command, triggerName);
 
       expect(moduleBuilderSpy.withTypeUID).toHaveBeenCalledWith(
         'core.ItemCommandTrigger'
@@ -51,12 +62,14 @@ describe('triggers.js', () => {
   });
 
   describe('ItemStateChangeTrigger', () => {
-    it('creates trigger.', () => {
-      const itemName = 'itemName';
-      const previousState = 'previousState';
-      const state = 'state';
-      const triggerName = 'triggerName';
-      ItemStateChangeTrigger(itemName, previousState, state, triggerName);
+    const itemName = 'itemName';
+    const previousState = 'previousState';
+    const state = 'state';
+    const triggerName = 'triggerName';
+    const item = new Item(itemName);
+
+    it.each([[itemName], [item]])('creates trigger from %s.', (itemOrName) => {
+      ItemStateChangeTrigger(itemOrName, previousState, state, triggerName);
 
       expect(moduleBuilderSpy.withTypeUID).toHaveBeenCalledWith(
         'core.ItemStateChangeTrigger'
@@ -90,11 +103,13 @@ describe('triggers.js', () => {
   });
 
   describe('ItemStateUpdateTrigger', () => {
-    it('creates trigger.', () => {
-      const itemName = 'itemName';
-      const state = 'state';
-      const triggerName = 'triggerName';
-      ItemStateUpdateTrigger(itemName, state, triggerName);
+    const itemName = 'itemName';
+    const state = 'state';
+    const triggerName = 'triggerName';
+    const item = new Item(itemName);
+
+    it.each([[itemName], [item]])('creates trigger from %s.', (itemOrName) => {
+      ItemStateUpdateTrigger(itemOrName, state, triggerName);
 
       expect(moduleBuilderSpy.withTypeUID).toHaveBeenCalledWith(
         'core.ItemStateUpdateTrigger'
@@ -109,12 +124,14 @@ describe('triggers.js', () => {
   });
 
   describe('GroupStateChangeTrigger', () => {
-    it('creates trigger.', () => {
-      const groupName = 'groupName';
-      const state = 'state';
-      const previousState = 'previousState';
-      const triggerName = 'triggerName';
-      GroupStateChangeTrigger(groupName, previousState, state, triggerName);
+    const groupName = 'groupName';
+    const state = 'state';
+    const previousState = 'previousState';
+    const triggerName = 'triggerName';
+    const group = new Item(groupName);
+
+    it.each([[groupName], [group]])('creates trigger from %s.', (groupOrName) => {
+      GroupStateChangeTrigger(groupOrName, previousState, state, triggerName);
 
       expect(moduleBuilderSpy.withTypeUID).toHaveBeenCalledWith(
         'core.GroupStateChangeTrigger'
@@ -129,11 +146,13 @@ describe('triggers.js', () => {
   });
 
   describe('GroupStateUpdateTrigger', () => {
-    it('creates trigger.', () => {
-      const groupName = 'groupName';
-      const state = 'state';
-      const triggerName = 'triggerName';
-      GroupStateUpdateTrigger(groupName, state, triggerName);
+    const groupName = 'groupName';
+    const state = 'state';
+    const triggerName = 'triggerName';
+    const group = new Item(groupName);
+
+    it.each([[groupName], [group]])('creates trigger from %s.', (groupOrName) => {
+      GroupStateUpdateTrigger(groupOrName, state, triggerName);
 
       expect(moduleBuilderSpy.withTypeUID).toHaveBeenCalledWith(
         'core.GroupStateUpdateTrigger'
@@ -148,11 +167,13 @@ describe('triggers.js', () => {
   });
 
   describe('GroupCommandTrigger', () => {
-    it('creates trigger.', () => {
-      const groupName = 'groupName';
-      const command = 'command';
-      const triggerName = 'triggerName';
-      GroupCommandTrigger(groupName, command, triggerName);
+    const groupName = 'groupName';
+    const command = 'command';
+    const triggerName = 'triggerName';
+    const group = new Item(groupName);
+
+    it.each([[groupName], [group]])('creates trigger from %s.', (groupOrName) => {
+      GroupCommandTrigger(groupOrName, command, triggerName);
 
       expect(moduleBuilderSpy.withTypeUID).toHaveBeenCalledWith(
         'core.GroupCommandTrigger'
@@ -262,7 +283,7 @@ describe('triggers.js', () => {
   describe('DateTimeTrigger', () => {
     it('creates trigger.', () => {
       const itemName = 'itemName';
-      const timeOnly = 'timeOnly';
+      const timeOnly = true;
       const triggerName = 'triggerName';
       DateTimeTrigger(itemName, timeOnly, triggerName);
 

diff --git a/test/typeOfArguments.spec.js b/test/typeOfArguments.spec.js
@@ -1,15 +1,27 @@
 const typeOfArguments = require('../typeOfArguments');
 
 describe('typeOfArguments.js', () => {
-  describe('supports primitive types (without "undefined")', () => {
-    const expectedArray = ['string', 'number', 'bigint', 'boolean', 'symbol', 'null'];
+  describe('supports primitive types', () => {
+    const expectedArray = ['string', 'number', 'bigint', 'boolean', 'symbol', 'null', 'undefined'];
 
     it('does not throw an error if arguments match.', () => {
-      expect(() => typeOfArguments(['string', 5, BigInt(5), true, Symbol('foo'), null], expectedArray)).not.toThrowError();
+      expect(() => typeOfArguments(['string', 5, BigInt(5), true, Symbol('foo'), null, undefined], expectedArray)).not.toThrowError();
     });
 
     it('throws TypeError if an argument does not match.', () => {
-      expect(() => typeOfArguments([5, BigInt(5), true, Symbol('foo'), null, 'string'], expectedArray)).toThrowError(TypeError);
+      expect(() => typeOfArguments([5, BigInt(5), true, Symbol('foo'), null, undefined, 'string'], expectedArray)).toThrowError(TypeError);
+    });
+  });
+
+  describe('supports classname checking', () => {
+    const expectedArray = ['Car', 'Bus'];
+    class Car {}
+    class Bus {}
+    it('does not throw an error if arguments match.', () => {
+      expect(() => typeOfArguments([new Car(), new Bus()], expectedArray)).not.toThrowError();
+    });
+    it('throws TypeError if an argument does not match.', () => {
+      expect(() => typeOfArguments([new Bus(), new Car()], expectedArray)).toThrowError(TypeError);
     });
   });
 
@@ -24,15 +36,10 @@ describe('typeOfArguments.js', () => {
     expect(() => typeOfArguments([{}, new Object()], ['object', 'object'])).not.toThrowError(); // eslint-disable-line
   });
 
-  describe('supports classname checking', () => {
-    const expectedArray = ['Car', 'Bus'];
-    class Car {}
-    class Bus {}
-    it('does not trow an error if arguments match.', () => {
-      expect(() => typeOfArguments([new Car(), new Bus()], expectedArray)).not.toThrowError();
-    });
-    it('throws TypeError if an argument does not match.', () => {
-      expect(() => typeOfArguments([new Bus(), new Car()], expectedArray)).toThrowError(TypeError);
-    });
+  it('throws TypeError if a required argument is missing.', () => {
+    function testFn (x, y) {
+      typeOfArguments([x, y], ['string', 'string']);
+    }
+    expect(() => testFn()).toThrowError(TypeError);
   });
 });