[RNE Rewrite] test(cpp): add host GoogleTest suites for the native sources - #1347
Merged
Conversation
The cpp/ sources are entirely JSI-facing, so the tests link a real Hermes runtime and a minimal ExecuTorch host build, install the production module into it, and drive it from JavaScript the way the TS layer does. 103 tests across dtype, conversions, tensor, schema, math, speech and cv, running in ~2s with no simulator or device. Adds scripts/build-native-test-deps.sh (pinned Hermes + ExecuTorch host builds, ~2 min, cached in CI) and scripts/run-native-tests.sh, plus a native-tests CI job. clang-tidy now skips cpp/tests, whose headers are not part of that script's prerequisites.
The v0.0.0-rewrite-libs-test prerelease was a throwaway. v0.10.0-libs carries the same ExecuTorch 1.3.1 C++ headers (byte-identical; it only drops the cpuinfo/pthreadpool and Obj-C headers the host tests never include).
MethodMeta only exists once a program is loaded, which previously left schema::methodSpecFromMetadata/validateSpec/getUsedBackends and all of ModelHostObject's constructor untested. They turn out not to need the XNNPACK delegate: Module::load() and Module::method_meta() both parse the program without initialising backends (only load_method() resolves them). So the whole load path runs on the host against selfie-segmentation, the smallest model the org publishes at ~486 KB, pinned to an exact HF revision and checksum-verified. Adds 18 tests (121 total). The fixture is downloaded rather than committed; when absent the suites are dropped with a CMake warning instead of failing. Execution still needs a host XNNPACK build and stays out of scope.
`libopencv-dev` is a meta-package that hard-depends on the viz and contrib
modules, pulling VTK, OpenMPI, tesseract and ~220 packages. On a throttled
Azure mirror that ran past 50 minutes (libvtk9 alone: 20 MB in ~10 min).
--no-install-recommends would not have helped, since those are hard Depends.
Install libopencv-{core,imgproc}-dev instead. OpenCVConfig.cmake ships only in
the meta-package, so cpp/tests now prefers OpenCV's CMake package when present
and otherwise locates core/imgproc directly; both paths verified to build and
pass all 121 tests.
Also caps the job at 30 minutes so a stalled download fails with logs rather
than running to the 6 h default.
Both were masked on macOS. re2/abseil were linked before libtokenizers.a, their consumer. GNU ld resolves archives left to right and only pulls members satisfying an already-undefined symbol, so nothing was pulled and the re2 symbols came out undefined; Apple's linker searches regardless of order. They are now interface dependencies of et_tokenizers, so CMake emits them after it, wrapped in --start-group on GNU ld to cover the cycles between abseil archives. OpenCV was compiled against the vendored third-party/include/opencv2 headers (4.13) while linking the system library (4.6 on Ubuntu) — cvtColor resolved to the AlgorithmHint overload added in 4.10 and failed at link. Homebrew's 4.12 was new enough to hide it. Vendored headers are now -isystem (matching compile_flags.txt, and silencing ExecuTorch's deprecation warnings) while the installed OpenCV goes on -I, which is always searched first, so headers and library now come from the same install. Also splits the dependency cache into restore/save so a failing test no longer discards the ~9 min build.
…ments The five required options were read inside the try block guarding the OpenCV call, so a missing or mistyped one came back as EXECUTION_FAILED with the message "OpenCV error: ... option 'x' is required" — a caller mistake labelled as an OpenCV failure, with the INVALID_ARGUMENT code lost on the way out. Read them before the try, which is where they belong: nothing about them can throw a cv::Exception.
The host build only covered the sources that existed when it was written. Since then `cpp/extensions/llm` and the phonemizer landed, and neither was compiled here at all. - Build ExecuTorch's LLM runner (plus the sampler, memory allocator and portable-kernel utilities it pulls in) and link `cpp/extensions/llm`. - Compile phonemis from the submodule and gate `phonemizer.cpp` on it, mirroring android/CMakeLists.txt. Its `data/` is Git LFS and nothing here reads it, so a smudge-skipped checkout is enough; a missing submodule drops the suite with a warning instead of failing. - Swap ExecuTorch's tokenizers submodule for software-mansion-labs/pytorch-tokenizers at the commit the shipped libraries use. third-party/include already carries that fork's headers, so upstream's libtokenizers.a linked fine and then read a differently laid out object: HFTokenizer::load segfaulted inside setup_pretokenizer. This is the one header/library drift that does not surface as a link error. - Fetch a tokenizer.json fixture (Whisper tiny.en, ~2.4 MB) alongside the .pte, both pinned by revision and checksum as before.
The branch has grown coded error handling, an llm extension, OCR quad extraction, a phonemizer, gather, rectifyQuad, a bool dtype and isEmulator. None of it was covered, and the coded errors had quietly broken the assumptions the existing suites were built on. Harness: `evalThrowing` reads the thrown value's name, code and etRuntimeErrorCode, and negative tests assert through `isCodedError` / `throwsCoded`. A throw site that loses its code — by raising a bare jsi::JSError, or by escaping the guard — now fails instead of passing on the message alone. New suites: - `ErrorTest` pins the JS-visible error shape, the ExecuTorch code being attached only for runtime failures, JS errors passing through the guard untouched, and the C++ code list matching VALID_ERROR_CODES in src/core/error.ts. - `UtilsTest` covers getExecuTorchRegisteredBackends, isEmulator and every namespace install() is expected to put on the module. - `LlmRunnerTest` covers createLLMRunner's argument contract and the tokenizer-then-model load order. - `OcrOpsTest` covers extractDbnetTextQuads: what each option filters, not only the happy path. - `PhonemizerTest` covers construction, validation and the disposal contract. - `TokenizerTest` closes the gap the README documented, now that a tokenizer fixture is fetched. Existing suites: gather and rectifyQuad added, the bool dtype covered, and the assertions that relied on jsi::JSError or std::invalid_argument moved onto codes. 195 tests pass; 154 with RNE_TESTS_ENABLE_OPENCV=OFF.
#1336 gave the native artifacts their own version (`nativeLibsVersion` in package.json), so download-libs.js resolves the libs release on its own and the RNET_BASE_URL override this job carried is dead weight. The same override was already dropped from the clang-tidy job on the base branch. Also check out the phonemis submodule, which the speech extension now compiles, with GIT_LFS_SKIP_SMUDGE so its LFS `data/` stays as pointer files, and raise the timeout: the dependency build grew the LLM runner and the tokenizers fork.
msluszniak
force-pushed
the
@ms/native-tests
branch
from
August 26, 2026 16:37
59581bc to
efaf4e0
Compare
Two phonemis headers use std::optional and std::u32string without including <optional> / <string>. libc++ pulls both in transitively, so the Android (NDK) and iOS builds never notice, and neither does a local run on macOS. libstdc++ does not, so the member declarations fail to parse and GCC reports a pile of "no declaration matches" errors on their definitions. Force-include the two headers for the phonemis target rather than patch a pinned submodule. The SHELL: prefix keeps each flag paired with its argument; without it CMake de-duplicates the repeated -include and the second header is taken as an input file.
msluszniak
marked this pull request as ready for review
August 26, 2026 17:39
barhanc
approved these changes
Aug 27, 2026
This was referenced Aug 27, 2026
Closed
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
2 participants
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.
Description
Adds host-side GoogleTest suites for the sources under
cpp/, plus anative-testsCI job.Every entry point in
cpp/is JSI-facing so rather than stub that boundary, the tests link a real Hermes runtime and a minimal ExecuTorch host build, install the production module, and drive it from JavaScript. That covers argument parsing, HostObject plumbing, TypedArray handling, and the exact error messages andcodes thatcore/error.hputs on the JS side.All tests pass covering all modules, except model execution and LLM generation, see notes below.
Introduces a breaking change?
Type of change
Tested on
Testing instructions
195 tests pass; 154 with
RNE_TESTS_ENABLE_OPENCV=OFF, which skips thecvandocrsuites.RNE_SKIP_FIXTURES=1works offline,-DRNE_TESTS_ENABLE_PHONEMIS=OFFskips the phonemizer suite.To confirm the tests work, mutate a source and re-run test suite. It should fail.
Screenshots
n/a
Related issues
Relates to #1291. The
RNET_BASE_URLoverride both CI jobs used to need is gone: since #1336 the artifacts carry their ownnativeLibsVersionanddownload-libs.jsresolves the release itself.Checklist
Additional notes
lint:cppnow skipscpp/tests, whose Hermes/GoogleTest headers aren't among that script's prerequisites. Test files can still be passed to it explicitly.extractDbnetTextQuadsread its five required options inside thetryguarding the OpenCV call, so a missing one surfaced asEXECUTION_FAILEDwith the messageOpenCV error: ... option 'x' is required; they are now read before it. Andbuild-native-test-deps.shswaps ExecuTorch's tokenizers submodule forsoftware-mansion-labs/pytorch-tokenizers, which is what the shipped libraries use and whatthird-party/includecarries headers for. Upstream'slibtokenizers.alinks fine and then reads a differently laid out object, soHFTokenizer::loadsegfaults insidesetup_pretokenizer. That is the one header/library drift here that does not show up as a link error.data/, which is Git LFS. The phonemizer suite pins the JSI contract instead: construction, argument checking, the lifecycle, error classification. All of this is documented incpp/tests/README.md.