[RNE Rewrite] test(cpp): add host GoogleTest suites for the native sources - #1347
Open
msluszniak wants to merge 6 commits into
Open
[RNE Rewrite] test(cpp): add host GoogleTest suites for the native sources#1347msluszniak wants to merge 6 commits into
msluszniak wants to merge 6 commits into
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).
Keeps it on the same headers source as the native-tests job instead of the throwaway v0.0.0-rewrite-libs-test prerelease. The C++ headers are identical, and the full cpp/ set clang-tidy sweeps compiles against them.
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.
12 tasks
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
1 participant
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 exactJSErrormessages.All tests passes covering all modules, except model execution, see notes below.
Introduces a breaking change?
Type of change
Tested on
Testing instructions
Pass
RNE_TESTS_ENABLE_OPENCV=OFFto skip thecvsuite,RNE_SKIP_FIXTURES=1to work offline.To confirm the tests work, mutate a source and re-run test suite. It should fail.
Screenshots
n/a
Related issues
Relates to #1291 — both CI jobs still need the
RNET_BASE_URLoverride while the package version is0.0.0.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.execute()needs the delegate the program was exported against, i.e. a host XNNPACK build. Argument validation ahead of it is. That plustokenizer.cpp(compiled and linked, but no suite yet — needs fixture files) are the remaining gaps, documented incpp/tests/README.md.