fix: stop native tfjs addon breaking tests in CI (semantic-search + reverse-image-search)#1106
Merged
Merged
Conversation
… in CI
Unit test suites failed to run in CI with 'tfjs_binding.node can not be
found'. The cause is a require-time dependency: several suites transitively
import use_embeddings.ts, which calls require('@tensorflow/tfjs-node'). That
package loads a platform-specific native addon at import time, but CI installs
dependencies with 'npm ci --ignore-scripts', so tfjs-node's postinstall never
downloads the addon. Whether a run passed depended on stale cached node_modules,
making it flaky and CI-only.
Mock @tensorflow/tfjs-node and the Universal Sentence Encoder via jest
moduleNameMapper so unit tests never load the native binary. The only tests
that exercise real embeddings are already skipped (they require live model
access), so no coverage is lost.
Verified against the emulator with the native addon removed: all suites now
run and pass instead of failing to load.
…sts in CI Test suites failed to run in CI with 'tfjs_binding.node can not be found'. Same root cause as firestore-semantic-search: suites transitively import feature_vectors.ts, which imports @tensorflow/tfjs-node, and that package loads a platform-specific native addon at import time. CI installs dependencies with 'npm ci --ignore-scripts', so tfjs-node's postinstall never downloads the addon, so any suite touching feature_vectors fails to load. Whether a run passed depended on stale cached node_modules, making it flaky and CI-only. Split unit and integration tests: - Unit run (default 'test' script) maps @tensorflow/tfjs-node to a lightweight stub via moduleNameMapper, so the many suites that only mock or never invoke feature_vectors stop crashing at import time. - The two specs that genuinely exercise TensorFlow (feature_vectors.test.ts and feature_vectors.memory.test.ts load a real model over the network and assert on real tensors) move to jest.integration.config.js, run via 'npm run test:integration'. They need the native addon built and network access, so they do not belong in the --ignore-scripts CI job. The stub lives outside a __mocks__ directory on purpose: manual mocks for node_modules packages are auto-applied by jest and cannot be disabled per config, which would also break the integration run. Wiring it through moduleNameMapper keeps it scoped to the unit config. Verified with the native addon removed and the emulator running: the unit run previously had 10 suites fail to load; it now runs 15 suites green with 0 failures. The integration config resolves the real tfjs package. Fixes #355.
2f7b5af to
8150388
Compare
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.
Fixes #355.
Problem
The
node.js_22_testCI job fails intermittently with:Suites fail to load (not fail assertions), which bails the whole
lerna run testjob. It affects two extensions that depend on TensorFlow: firestore-semantic-search and storage-reverse-image-search. Both are fixed here in one PR becauselerna run test --concurrency 1with nx-bail stops at the first failing extension, so CI can only go green once both are fixed together.Root cause
require('@tensorflow/tfjs-node')(use_embeddings.ts/feature_vectors.ts).@tensorflow/tfjs-nodeloads a platform-specific native addon (tfjs_binding.node) at import time.npm ci --ignore-scripts, so tfjs-node's postinstall that downloads the addon never runs - the addon is absent.node_moduleswas restored from the Lerna cache (keyed on the lockfile hash), so it passed/failed at random, and only in CI.Fix
firestore-semantic-search: every test that exercises real embeddings is already skipped (needs live model access), so
@tensorflow/tfjs-nodeand the Universal Sentence Encoder are mapped to lightweight stubs via jestmoduleNameMapper. No coverage lost.storage-reverse-image-search: two specs genuinely exercise TensorFlow (load a real model over the network, assert on real tensors/memory), so a blanket stub is not possible. Split unit vs integration:
testscript) stubs@tensorflow/tfjs-nodeviamoduleNameMapper, fixing the many suites that only mock or never invokefeature_vectors.jest.integration.config.js(npm run test:integration), which uses the real package. They need the native addon built and network access, so they do not belong in the--ignore-scriptsCI job. Preserved, not deleted.__mocks__directory on purpose: manual mocks for node_modules packages are auto-applied by jest and cannot be disabled per-config, which would also break the integration run.moduleNameMapperkeeps it scoped to the unit config.Verification
Reproduced the CI failure locally by removing the native addon: suites failed to load exactly as in CI. With the fix applied and the addon still absent, both extensions' unit suites run green against the Firestore emulator (semantic-search: 11 suites pass, was 5 failing; reverse-image-search: 15 suites pass, was 10 failing). Confirmed in CI that
firestore-semantic-search:testnow passes.