W-23517404: Add integration tests + charset decode fix#132
Open
mlischetti wants to merge 1 commit into
Open
Conversation
…de fix Add integration-lane coverage for gaps F3 (concurrency / FFI edge cases) and F5 (error & edge-input matrix) from TESTING_ASSESSMENT.md, and fix a real charset bug surfaced while writing the F5 charset tests. Fix (src/result.ts): - ExecutionResult.getString() passed the runtime's IANA charset name (e.g. UTF-16, US-ASCII, ISO-8859-1) straight to Buffer.toString, which threw ERR_UNKNOWN_ENCODING on a successful result. Replace the unchecked `as BufferEncoding` cast with decodeBytes(), which maps common charsets to valid Node encodings, handles UTF-16 byte order (BOM detection + big-endian byte-swap, since Node only decodes LE), and falls back to UTF-8 for unknown charsets instead of throwing. - 10 new unit tests for decodeBytes (BE/LE/BOM, latin1, ascii, unknown fallback, no caller-buffer mutation). Group B integration tests (tests/integration/edge-cases.test.ts, 20): - Concurrency / FFI: runTransform async-iterable input (pre-buffered path), empty and throwing async inputs, multi-instance lifecycle, idempotent initialize, double-cleanup, re-init after cleanup, run-before-initialize guard, concurrent runs on the singleton, interleaved concurrent streaming. - Error & edge inputs: binary output getBytes round-trip, output charset matrix (UTF-8/16/16LE/16BE/ISO-8859-1 + US-ASCII substitution), unknown input/output mime types, missing input reference, compilation/syntax error, malformed input content. Documented (not asserted): a bad-library-path initialize() throws only as the first init in a process (the native runtime is ref-counted / loaded process-globally), so it can't be asserted in the shared lane without an isolated child process — verified manually, tracked as a follow-up. Verified: tsc clean, 70 unit tests pass, 34 integration tests pass against the real native library. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
Closes gaps F3 (concurrency / FFI edge cases) and F5 (error & edge-input matrix) from
TESTING_ASSESSMENT.mdfor the@dataweave/nativeNode.js binding.While writing the F5 charset tests, this also fixes a real bug in
getString().Bug fix —
src/result.tsExecutionResult.getString()passed the runtime's IANA charset name (UTF-16,US-ASCII,ISO-8859-1) straight toBuffer.toString, which threwERR_UNKNOWN_ENCODINGon a successful result. Replaced the uncheckedas BufferEncodingcast with a newdecodeBytes()that:Backed by 10 new unit tests.
Integration tests —
tests/integration/edge-cases.test.ts(20)runTransformasync-iterable input (pre-buffered path), empty & throwing async inputs, multi-instance lifecycle, idempotentinitialize, double-cleanup, re-init after cleanup, run-before-init guard, 25 concurrent runs, interleaved concurrent streaming.getBytesround-trip, output charset matrix, unknown input/output mime types, missing input reference, compilation/syntax error, malformed input content.Known limitation (tracked)
A bad-library-path
initialize()throwsDataWeaveErroronly as the first init in a process (the native runtime is ref-counted / process-global), so it can't be asserted in the shared lane without test-order coupling.Documented in-file and verified manually; tracked as follow-up W-23517830.
Testing
tsc --noEmitcleannpm run test:unit→ 70 passednpm run test:integration→ 34 passed against the real native library🤖 Generated with Claude Code