Repository navigation
test: Add a virtual device harness and move every test onto it - #260
Merged
Merged
Conversation
Real devices behave too well to reproduce bugs like the readAttributes retry loop. createVirtualDevice puts the device on its own node behind an in-memory link that records every frame, enforces Homey's 82-byte ZCL frame limit, and lets a test drop, rewrite or inject frames. test/virtualDevice.js keeps four bugs that reproduce on master as todo tests: the readAttributes loop on unrequested ids (#258), readAttributes([]) on metering overflowing a frame (#259), Endpoint.unbind leaving a Cluster in bindings, and attr.undefined emitted for unknown ids in a report. Refs #258 Refs #259
The suite had four ways to fake a node: createMockNode, createConnectedNodePair, MOCK_DEVICES and hand-built Node stubs. All of them are now createVirtualDevice, and test/util/mockNode.js is gone. Migrating exposed tests that could not fail. They now can, each checked by breaking the library code it covers: - waitForResponse: false was passed as command args, so the test waited anyway. - The mixed-manufacturer tests accepted any error, including one from a binding left over from an earlier test. - Not answering a default response was never reached, because our own default responses set Disable Default Response; the device now sends one without it. - Assertions inside handlers became error responses or hung done callbacks; tests now assert after await or settle(), and on wire frames instead of spies. - The attribute 0x0000 test asserted nothing; it now decodes a noData report. Duplicates of the ZCLError and Default Response tests are removed, the four tests of MOCK_DEVICES itself are dropped, and OTA runs the way Homey uses it: the device requests, the node serves. The harness gains frame meta, eager parsing and a 1000-frame cap that turns a request/response loop into a failure.
docs/testing.md explains both sides of createVirtualDevice, its options and return values, recipes for misbehaving devices, and the rules that keep a test able to fail. AGENTS.md and the README point to it.
test/manual/xiaomiHacks.js registered a Basic cluster that declares its own data type for Xiaomi's attribute 0xff01, but never decoded a frame, so it checked nothing. The new test has the virtual device send a report shaped like the ones Xiaomi devices send and asserts the decoded records. It fails when the library decodes with the type on the wire instead of the declared one, which is what such a custom cluster relies on.
Frames were delivered while sendFrame was still running, so every reply arrived before the command armed its response timeout: setting that timeout to 0 failed one test. Frames now arrive on the next turn of the event loop, as on Homey where sendFrame resolves on the APS ACK and the response comes later. The same mutation now fails 33 tests. A loop between two handlers only debug-logged the frame cap error, so the test passed. settle() now rethrows a failed delivery, including the cap. An interceptor that returns a non-Buffer throws instead of being dropped silently. frameToClient() and createSingleClusterDevice() replace the hand-built headers and per-file single-cluster helpers. frames[].error, which the node never let through, is gone, and docs/testing.md matches the code.
github-actions
Bot
added
the
internal
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The review identifies no active issues with the virtual-device test-harness migration.
No open issues 🎉
What changed in this PR
This PR unifies node-based tests around an in-memory virtual device harness, improving coverage of Zigbee frame handling without changing production library code.
Changes:
- Adds paired nodes with asynchronous delivery, frame recording and interception, outgoing frame-size enforcement, and loop detection.
- Migrates tests from legacy mocks, moves assertions outside handlers, and adds Xiaomi decoding coverage plus four known-bug tests.
- Documents the harness and removes obsolete helpers and manual test code.
| File | Description |
|---|---|
test/xiaomiLifeline.js |
Adds an automated Xiaomi lifeline report-decoding test. |
test/virtualDevice.js |
Tests harness behavior and records four known bugs as todos. |
test/util/virtualDevice.js |
Implements the shared virtual device harness and frame helpers. |
test/util/mockNode.js |
Removes legacy mock-node factories and device presets. |
test/util/index.js |
Exports the new harness utilities. |
test/testZCLError.js |
Consolidates error-response tests with explicit status-byte assertions. |
test/testTrxSequenceNumber.js |
Migrates sequence-number tests to the harness. |
test/testNode.js |
Migrates node tests and strengthens observable assertions. |
test/testMfgSpecificCommands.js |
Checks manufacturer headers and mixed-manufacturer rejection. |
test/testClusterResponseTimeout.js |
Migrates timeout and response-matching scenarios to the harness. |
test/ota.js |
Models device-initiated OTA requests with the node serving responses. |
test/onOff.js |
Migrates On/Off command tests to paired nodes. |
test/manual/xiaomiHacks.js |
Removes the manual Xiaomi example replaced by automated coverage. |
test/levelControl.js |
Migrates Level Control command tests. |
test/iasZone.js |
Migrates commands and notifications; removes preset-only tests. |
test/doorLock.js |
Migrates command and notification tests; moves assertions outside handlers. |
test/defaultResponse.js |
Checks response behavior through the virtual link. |
test/colorControl.js |
Migrates tests while retaining specification-byte checks. |
test/attributeReport.js |
Verifies emitted values and metadata after delivery completes. |
README.md |
Links contributor guidance to the testing documentation. |
docs/testing.md |
Documents harness APIs, recipes, rules, and limitations. |
AGENTS.md |
Updates test placement and harness guidance. |
Review effort: Balanced
The index only re-exported test/util/virtualDevice.js. docs/adding-clusters.md still pointed at the removed mock node helpers there; it now links the testing guide.
The diagram repeated the bullets under it, frameToClient was explained three times, and the misbehaving-device snippet was plain BoundCluster use.
Option and return value details now live as JSDoc on createVirtualDevice, next to the code they describe, so they cannot drift from it. The guide keeps the idea, a table of example tests per scenario, and the rules.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
One test harness,
createVirtualDevice(), replaces the four ways the suite faked a node. It puts the code under test (zclNode) and a scripted device (virtualDevice) on two nodes linked in memory, logs every frame, lets a test drop, rewrite or inject frames, and enforces Homey's 82-byte ZCL frame limit. Every test now uses it, anddocs/testing.mdexplains how.Why
Real devices behave too well to reproduce bugs like the
readAttributesretry loop. Several existing tests also could not fail: breaking the code they covered left them green.How
sendFrameresolves, as on Homey, so response timeouts really run.settle()waits for frames in flight and fails on a request/response loop between the nodes.todotests intest/virtualDevice.js. #258 and #259 remove theirs.waitForResponse: false, mixed manufacturer ids, not answering a Default Response, and assertions inside handlers.MOCK_DEVICEShelper are gone.test/manual/xiaomiHacks.jsnever decoded a frame and is now a real test.Anything Else
Per-cluster command tests still encode and decode with the same definition, so a wrong command id or argument width passes. Asserting spec bytes per cluster is a follow-up.