[T-004] Add comprehensive test suite for flux-js VM#1
[T-004] Add comprehensive test suite for flux-js VM#1SuperInstance wants to merge 1 commit intomainfrom
Conversation
- Added Vitest framework (previously 0 tests)
- Created 6 test files with 161 tests covering:
- VM core: 45 tests (all opcodes, registers, stack, jumps, loops, errors)
- Assembler: 32 tests (encoding, labels, comments, case insensitivity)
- Disassembler: 22 tests (all instruction formats, round-trip)
- Vocabulary/Interpreter: 37 tests (10 NL patterns, result format)
- A2A/Swarm: 21 tests (agents, messaging, consensus, integration)
- Opcodes: 4 tests (constants verification)
- Fixed 3 source bugs found during testing:
- BUG 1: JMP opcode used compound assignment this.pc+=this._i16()
which is unsafe in V8 — _i16() modifies this.pc before the
compound assignment reads it, causing incorrect offset base
- BUG 2: Assembler label resolution for JMP was off by 2
(used bc.length instead of bc.length + 2 as currentPc)
- BUG 3: Assembler label resolution for JNZ/JZ was off by 1
(used bc.length + 1 instead of bc.length + 2)
- All 161 tests passing
There was a problem hiding this comment.
🟡 Incomplete fix: MOVI label resolution has same off-by-one that was fixed in JNZ/JZ
The PR correctly fixes the resolveValue currentPc offset for JNZ/JZ (line 179: bc.length + 1 → bc.length + 2) and JMP (line 183: bc.length → bc.length + 2), but the identical pattern in MOVI (line 175) was left unfixed at bc.length + 1. MOVI has the same 4-byte format as JNZ/JZ: after bc.push(op, register), bc.length = S + 2. The VM reads MOVI as opcode(1) + _u8()(1) + _i16()(2) = 4 bytes, so PC ends at S + 4. The correct currentPc should be bc.length + 2 = S + 4, but the code passes bc.length + 1 = S + 3, producing an off-by-one error when a label is used as the MOVI operand (e.g., MOVI R0, some_label).
(Refers to line 175)
Was this helpful? React with 👍 or 👎 to provide feedback.
Summary
Bug Fixes
BUG 1: JMP opcode compound assignment (V8 unsafe)
this.pc += this._i16()readsthis.pcbefore callingthis._i16(), but_i16()modifiesthis.pc. V8 caches the stale value, applying the offset to the wrong base.Fix: Store offset in variable first.
BUG 2: JMP label resolution off by 2
BUG 3: JNZ/JZ label resolution off by 1
Part of T-004