test: Add a virtual device harness and move every test onto it

AGENTS.md

@@ -18,7 +18,8 @@ npm run generate-types  # regenerate index.d.ts after changing cluster definitio
 npm run build           # JSDoc site into build/, open build/index.html in a browser
 ```
 
-`npm test` globs `test/*.js` only, so nothing under `test/manual/` runs with it.
+`npm test` globs `test/*.js` only, so a test file must sit directly in `test/`; `test/util/` holds
+the harness.
 
 `index.d.ts` is generated from the cluster definitions and committed. Never edit it by hand; run
 `npm run generate-types`, which CI does too.
@@ -51,19 +52,8 @@ Refer to a cluster id as `OnOffCluster.ID`, never as `6` or `0x0006`.
 
 ## Tests
 
-Tests run on `node:test`, so `describe`, `it` and `mock` are imported, not globals. A callback-style
-test takes the test context first: `it('...', function (t, done) { ... })`. Written as
-`function (done)` node:test sees a synchronous test, passes it immediately and never runs the
-assertions.
+Tests run on `node:test`, so `describe`, `it` and `mock` are imported, not globals.
 
-Build mock nodes with the helpers in `test/util/index.js` instead of by hand:
-
-- `createMockNode({ loopback: true, ... })` sends a command straight back into the same node's
-  `BoundCluster`, which covers most cases. See `test/onOff.js`.
-- `createConnectedNodePair()` wires two nodes together when sender and receiver must differ. See
-  `test/testNode.js`.
-- `MOCK_DEVICES` holds preset sensors. See `test/iasZone.js`.
-
-Server-to-client traffic such as attribute reports and notifications has no helper at the time of
-writing, so check `test/util/index.js` first and otherwise build a `ZCLStandardHeader` by hand and
-pass it to `node.handleFrame()`, as `test/attributeReport.js` does.
+Every test that needs a node uses `createVirtualDevice()` from `test/util/virtualDevice.js`.
+Read [docs/testing.md](docs/testing.md) before writing a test, above all its rules: an assertion
+inside a `BoundCluster` or cluster handler becomes an error Default Response instead of a failure.

README.md

@@ -340,7 +340,7 @@ The workflow commits updated types back to that branch if changes are detected.
 Great if you'd like to contribute to this project, a few things to take note of before submitting a PR:
 
 - This project enforces ESLint and Prettier: check with `npm run lint` and `npm run format:check`, and fix formatting with `npm run format:write`.
-- Tests use the built-in node:test runner, see the [test](https://github.com/athombv/node-zigbee-clusters/blob/production/test) directory.
+- Tests use the built-in node:test runner and one virtual device harness, see [Writing tests](docs/testing.md).
 - Adding or updating a cluster definition follows its own conventions, see [Adding or updating a cluster definition](docs/adding-clusters.md).
 - Pull request titles follow [Conventional Commits](https://www.conventionalcommits.org/), for example `fix(onOff): Handle a missing attribute`. The title becomes a line in the release notes, so write it for someone using the library. See [Release notes](#release-notes).
 - [GitHub Action workflows](https://github.com/athombv/node-zigbee-clusters/blob/production/.github/workflows) run ESLint, Prettier and the tests, and publish releases.

docs/adding-clusters.md

@@ -127,7 +127,7 @@ lists. It sits at module level because `imageBlockRequest`, `imagePageRequest` a
 
 A definition is worth a test when it encodes or decodes something a plain attribute list does not:
 command arguments, a variant payload, a notification. `test/onOff.js` is the smallest example,
-`test/ota.js` covers variant fields. The mock node helpers live in `test/util/index.js`.
+`test/ota.js` covers variant fields. [Writing tests](testing.md) explains the test harness.
 
 ## Before opening a pull request
 

docs/testing.md

@@ -0,0 +1,48 @@
+# Writing tests
+
+## The idea
+
+A test plays both sides of a Zigbee conversation. `createVirtualDevice()` in
+[`test/util/virtualDevice.js`](../test/util/virtualDevice.js) creates two nodes linked in memory:
+
+- `zclNode`, the code under test, as a Homey driver gets it.
+- `virtualDevice`, the device it talks to, scripted by the test with `BoundCluster`s.
+
+Both run the library's real encoding, parsing and response matching; only the radio is replaced.
+The link behaves like Homey's stack where it matters for this library: frames arrive after
+`sendFrame` resolves, so response timeouts run, and `zclNode` cannot send more than 82 bytes. The
+link also logs every frame and lets a test drop, rewrite or inject frames, which is how a test makes
+a device misbehave in ways real hardware rarely does.
+
+The options and return values are documented on the functions in
+[`test/util/virtualDevice.js`](../test/util/virtualDevice.js).
+
+## Examples
+
+| To test                                                     | See                                  |
+| ----------------------------------------------------------- | ------------------------------------ |
+| A command or attribute read                                 | `test/onOff.js`                      |
+| The bytes on the wire against the specification             | `test/colorControl.js`               |
+| A report or notification from the device                    | `test/attributeReport.js`            |
+| A device that answers wrongly, or never; known bugs as todo | `test/virtualDevice.js`              |
+| Colliding TSNs, duplicate responses, failed sends           | `test/testClusterResponseTimeout.js` |
+| Group and broadcast frames                                  | `test/testNode.js`                   |
+| Homey serving the device, such as OTA                       | `test/ota.js`                        |
+| A cluster with its own data type                            | `test/xiaomiLifeline.js`             |
+
+## Rules
+
+A test is only worth having if it fails when the code it covers breaks.
+
+- **Never assert inside a handler.** The endpoint turns a thrown assertion into an error Default
+  Response, so the test may pass anyway. Store what the handler got and assert after the `await`.
+- **No `done` callbacks.** Await the command, or `settle()`.
+- **`await settle()`** before asserting that something did not happen, or on what a frame from the
+  device triggered.
+- **Fake time** with `t.mock.timers`, never wait for a real timeout.
+- **One device per test**, created inside the `it`.
+- **Assert specific errors and real bytes.** A round trip through the same definition passes when
+  the definition is wrong.
+- **Prove the failure path.** Break the library code under test and check the test fails.
+- **A known bug** is a test for the correct behavior marked `{ todo: '...' }`. Check it fails on the
+  bug, not on the test; the fix removes the `todo`.

test/attributeReport.js

@@ -1,36 +1,34 @@
 'use strict';
 
-const { describe, it } = require('node:test');
+const { describe, it, mock } = require('node:test');
 const assert = require('node:assert');
 const OnOffCluster = require('../lib/clusters/onOff');
-const { createMockNode } = require('./util');
-const { ZCLStandardHeader } = require('../lib/zclFrames');
+const { createVirtualDevice, frameToClient } = require('./util/virtualDevice');
 
 describe('Attribute report', function () {
-  it('should emit the transaction sequence number with the attribute value', function (t, done) {
-    const node = createMockNode({
-      endpoints: [
-        {
-          endpointId: 1,
-          inputClusters: [OnOffCluster.ID],
-        },
-      ],
+  it('should emit the transaction sequence number with the attribute value', async function () {
+    const { zclNode, virtualDevice, settle } = createVirtualDevice({
+      endpoints: [{ endpointId: 1, inputClusters: [OnOffCluster.ID] }],
     });
+    const onOnOff = mock.fn();
+    zclNode.endpoints[1].clusters.onOff.on('attr.onOff', onOnOff);
 
-    node.endpoints[1].clusters.onOff.on('attr.onOff', (value, meta) => {
-      assert.strictEqual(value, true);
-      // Only whitelisted fields, not the stack's frame meta
-      assert.deepStrictEqual(meta, { transactionSequenceNumber: 42 });
-      done();
+    const report = frameToClient({
+      cmdId: 0x0a, // reportAttributes
+      trxSequenceNumber: 42,
+      // attributeId 0x0000 (onOff), dataType 0x10 (bool), value true
+      data: Buffer.from([0x00, 0x00, 0x10, 0x01]),
     });
+    await virtualDevice.sendFrame(1, OnOffCluster.ID, report, {
+      dstEndpoint: 1,
+      linkQuality: 200,
+    });
+    await settle();
 
-    const frame = new ZCLStandardHeader();
-    frame.cmdId = 0x0a; // reportAttributes
-    frame.trxSequenceNumber = 42;
-    frame.frameControl.directionToClient = true;
-    // attributeId 0x0000 (onOff), dataType 0x10 (bool), value true
-    frame.data = Buffer.from([0x00, 0x00, 0x10, 0x01]);
-
-    node.handleFrame(1, OnOffCluster.ID, frame.toBuffer(), { dstEndpoint: 1, linkQuality: 200 });
+    // Only whitelisted fields, not the stack's frame meta
+    assert.deepStrictEqual(
+      onOnOff.mock.calls.map((call) => call.arguments),
+      [[true, { transactionSequenceNumber: 42 }]],
+    );
   });
 });

test/colorControl.js

@@ -1,18 +1,16 @@
 'use strict';
 
-const { describe, it, afterEach, mock } = require('node:test');
+const { describe, it } = require('node:test');
 const assert = require('node:assert');
 
 const BoundCluster = require('../lib/BoundCluster');
 const ColorControlCluster = require('../lib/clusters/colorControl');
-const Endpoint = require('../lib/Endpoint');
-const { createMockNode } = require('./util');
+const { createVirtualDevice } = require('./util/virtualDevice');
 
 // Sends colorLoopSet on a fresh node and returns the parsed outgoing frame, so tests can assert
 // the payload bytes against the ZCL spec instead of round-tripping through our own definition
 async function sendColorLoopSet(args) {
-  const node = createMockNode({
-    loopback: true,
+  const { zclNode, virtualDevice, frames } = createVirtualDevice({
     endpoints: [
       {
         endpointId: 1,
@@ -21,7 +19,7 @@ async function sendColorLoopSet(args) {
     ],
   });
 
-  node.endpoints[1].bind(
+  virtualDevice.endpoints[1].bind(
     'colorControl',
     new (class extends BoundCluster {
       async colorLoopSet() {
@@ -30,20 +28,15 @@ async function sendColorLoopSet(args) {
     })(),
   );
 
-  const sendFrameSpy = mock.method(node, 'sendFrame');
-  await node.endpoints[1].clusters.colorControl.colorLoopSet(args);
+  await zclNode.endpoints[1].clusters.colorControl.colorLoopSet(args);
 
-  return Endpoint.parseFrame(sendFrameSpy.mock.calls[0].arguments[2]);
+  assert.strictEqual(frames[0].from, 'zclNode');
+  return frames[0].frame;
 }
 
 describe('Color Control', function () {
-  afterEach(function () {
-    mock.restoreAll();
-  });
-
   it('should receive moveToColor', async function () {
-    const node = createMockNode({
-      loopback: true,
+    const { zclNode, virtualDevice } = createVirtualDevice({
       endpoints: [
         {
           endpointId: 1,
@@ -53,7 +46,7 @@ describe('Color Control', function () {
     });
 
     let receivedData = null;
-    node.endpoints[1].bind(
+    virtualDevice.endpoints[1].bind(
       'colorControl',
       new (class extends BoundCluster {
         async moveToColor(data) {
@@ -62,7 +55,7 @@ describe('Color Control', function () {
       })(),
     );
 
-    await node.endpoints[1].clusters.colorControl.moveToColor({
+    await zclNode.endpoints[1].clusters.colorControl.moveToColor({
       colorX: 0x5000,
       colorY: 0x3000,
       transitionTime: 20,
@@ -74,8 +67,7 @@ describe('Color Control', function () {
   });
 
   it('should receive moveToColorTemperature', async function () {
-    const node = createMockNode({
-      loopback: true,
+    const { zclNode, virtualDevice } = createVirtualDevice({
       endpoints: [
         {
           endpointId: 1,
@@ -85,7 +77,7 @@ describe('Color Control', function () {
     });
 
     let receivedData = null;
-    node.endpoints[1].bind(
+    virtualDevice.endpoints[1].bind(
       'colorControl',
       new (class extends BoundCluster {
         async moveToColorTemperature(data) {
@@ -94,7 +86,7 @@ describe('Color Control', function () {
       })(),
     );
 
-    await node.endpoints[1].clusters.colorControl.moveToColorTemperature({
+    await zclNode.endpoints[1].clusters.colorControl.moveToColorTemperature({
       colorTemperature: 370,
       transitionTime: 15,
     });
@@ -104,8 +96,7 @@ describe('Color Control', function () {
   });
 
   it('should receive moveToHueAndSaturation', async function () {
-    const node = createMockNode({
-      loopback: true,
+    const { zclNode, virtualDevice } = createVirtualDevice({
       endpoints: [
         {
           endpointId: 1,
@@ -115,7 +106,7 @@ describe('Color Control', function () {
     });
 
     let receivedData = null;
-    node.endpoints[1].bind(
+    virtualDevice.endpoints[1].bind(
       'colorControl',
       new (class extends BoundCluster {
         async moveToHueAndSaturation(data) {
@@ -124,7 +115,7 @@ describe('Color Control', function () {
       })(),
     );
 
-    await node.endpoints[1].clusters.colorControl.moveToHueAndSaturation({
+    await zclNode.endpoints[1].clusters.colorControl.moveToHueAndSaturation({
       hue: 180,
       saturation: 200,
       transitionTime: 10,
@@ -136,8 +127,7 @@ describe('Color Control', function () {
   });
 
   it('should receive colorLoopSet', async function () {
-    const node = createMockNode({
-      loopback: true,
+    const { zclNode, virtualDevice } = createVirtualDevice({
       endpoints: [
         {
           endpointId: 1,
@@ -147,7 +137,7 @@ describe('Color Control', function () {
     });
 
     let receivedData = null;
-    node.endpoints[1].bind(
+    virtualDevice.endpoints[1].bind(
       'colorControl',
       new (class extends BoundCluster {
         async colorLoopSet(data) {
@@ -156,7 +146,7 @@ describe('Color Control', function () {
       })(),
     );
 
-    await node.endpoints[1].clusters.colorControl.colorLoopSet({
+    await zclNode.endpoints[1].clusters.colorControl.colorLoopSet({
       updateFlags: ['updateAction', 'updateDirection', 'updateTime', 'updateStartHue'],
       action: 'activateFromColorLoopStartEnhancedHue',
       direction: 'incrementHue',
@@ -176,7 +166,7 @@ describe('Color Control', function () {
     assert.strictEqual(receivedData.startHue, 0x2300);
   });
 
-  // The loopback test above encodes and decodes with the same definition, so it passes even when
+  // The round-trip test above encodes and decodes with the same definition, so it passes even when
   // that definition is wrong on the wire. These bytes come from the spec, not from our types:
   // ZCL r8 Figure 5-17 (payload layout), Figure 5-18 (updateFlags bits), Table 5.25 (action
   // values), Table 5.26 (direction values), Table 5.13 (command id).