fix(storage-reverse-image-search): stop native tfjs addon breaking tests in CI#1107
Closed
cabljac wants to merge 1 commit into
Closed
fix(storage-reverse-image-search): stop native tfjs addon breaking tests in CI#1107cabljac wants to merge 1 commit into
cabljac wants to merge 1 commit into
Conversation
…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.
Collaborator
Author
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
storage-reverse-image-search:testfails intermittently in CI with:Suites fail to load (not fail assertions), which bails the lerna test run. Same class of bug as firestore-semantic-search.
Root cause
src/common/feature_vectors.ts, which doesimport * as tf from '@tensorflow/tfjs-node'.@tensorflow/tfjs-nodeloads a platform-specific native addon 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, so it passed/failed at random - and only in CI.Fix - split unit and integration tests
testscript): maps@tensorflow/tfjs-nodeto a lightweight stub viamoduleNameMapper. The many suites that mockfeature_vectorsor never invoke it stop crashing at import time.npm run test:integrationviajest.integration.config.js): the two specs that genuinely exercise TensorFlow -feature_vectors.test.tsandfeature_vectors.memory.test.ts, which load a real model over the network and assert on real tensors/memory - move here. They need the native addon built and network access, so they do not belong in the--ignore-scriptsCI job. They are preserved, not deleted.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 throughmoduleNameMapperkeeps it scoped to the unit config.Verification
Reproduced the CI failure locally by removing the native addon: 10 suites failed to load. With the fix applied and the addon still absent, the unit run (against the Firestore emulator) is green: 15 suites pass, 0 failures. The integration config resolves the real tfjs package.
Companion to the same fix for firestore-semantic-search (#1106).