Skip to content

Commit 37dd404

Browse files
GijsWeteringsmeta-codesync[bot]
authored andcommitted
Run the ReactAndroid JNI gtests as instrumentation tests (#57976)
Summary: Pull Request resolved: #57976 The `ReactAndroid` JNI gtests in `react/fabric/test` and `react/jni/test` were configured as plain host C++ tests, but they link against the Android-only JNI libraries. The resulting binary is an Android aarch64 ELF that needs `/system/bin/linker64` and so cannot run on a Linux host: ``` qemu-aarch64: Could not open '/system/bin/linker64': No such file or directory Test failed to produce the expected output! ... IO error: No result xml files found. ``` They were not failing their assertions — they were never executing. The runner enumerated case names statically out of the binary without running it, so all 6 cases across the two targets were known and reported as failing, with no stack traces at all. The absence of stack traces was itself the tell: the process never started, so no gtest XML was ever produced. Both targets move to the established pattern for gtests against Android-only native libs: build them as Android instrumentation tests, packaged into an APK and run on a device or emulator. `FabricMountingManagerTest` can drop its deliberate leak as a result. It previously allocated the manager with a no-op deleter, because `~FabricMountingManager()` calls `jni::ThreadScope::WithClassLoader`, which throws without an attached `JavaVM`. Under instrumentation the gtest runs inside a native method registered via fbjni `makeNativeMethod`, so `cachedOrNull()` is non-null and the closure runs inline — destruction is safe. The fixture now holds a default-constructed (null) `jni::global_ref` member and returns a `std::unique_ptr`; releasing a null `global_ref` is a no-op. All four test bodies are byte-identical. `ModuleRegistryBuilderTest.cpp` is a comment-only correction. Changelog: [Internal] Differential Revision: D116286866 fbshipit-source-id: a283deae18e42b0caad330bd3a27307428cfcec4
1 parent 5cb6524 commit 37dd404

2 files changed

Lines changed: 17 additions & 25 deletions

File tree

packages/react-native/ReactAndroid/src/main/jni/react/fabric/test/FabricMountingManagerTest.cpp

Lines changed: 15 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -32,32 +32,24 @@ namespace facebook::react {
3232
* `allocatedViewsMutex_`; they never call into Java. That is what makes them
3333
* unit-testable here.
3434
*
35-
* NOTE: `~FabricMountingManager()` calls `jni::ThreadScope::WithClassLoader`,
36-
* which throws `std::runtime_error` when no JavaVM has been attached (see
37-
* `xplat/spectrum/androidLibs/fbjni/cxx/fbjni/detail/Environment.h`). Because
38-
* no JVM is available in this host-side test, the manager instance is
39-
* intentionally never destroyed: each test allocates a single
40-
* `FabricMountingManager` on the heap and wraps it in a `std::shared_ptr`
41-
* with a no-op deleter. The resulting per-test leak is bounded (one ~100-byte
42-
* instance) and does not cross test boundaries.
35+
* `react/fabric:jni` is an Android-only native library, so this target is
36+
* built as a JNI instrumentation test: the gtest binary is packaged into an
37+
* APK and executed on a device/emulator. A JavaVM is therefore attached, which
38+
* `~FabricMountingManager()` requires — it calls
39+
* `jni::ThreadScope::WithClassLoader`, which throws `std::runtime_error` when
40+
* fbjni has not been initialized (see
41+
* `xplat/spectrum/androidLibs/fbjni/cxx/fbjni/detail/Environment.h`).
4342
*/
4443
class FabricMountingManagerTest : public ::testing::Test {
4544
protected:
46-
// Returns a `FabricMountingManager` whose destructor is suppressed.
47-
// See the class-level note above for why this is necessary.
48-
static std::shared_ptr<FabricMountingManager> makeManager() {
49-
// `jni::global_ref<>` default-constructs to an empty (null) reference,
50-
// so no JNI calls are performed during construction. The empty
51-
// reference is safe to copy into the manager because the
52-
// surface-registry methods under test never dereference
53-
// `javaUIManager_`.
54-
auto* emptyRef = new jni::global_ref<JFabricUIManager::javaobject>();
55-
auto* raw = new FabricMountingManager(*emptyRef);
56-
// `emptyRef` is intentionally leaked: resetting it would also engage
57-
// `WithClassLoader`, which requires an attached JavaVM.
58-
return {raw, [](FabricMountingManager* /*unused*/) noexcept {
59-
// No-op deleter: see class-level note.
60-
}};
45+
// An empty (null) reference: `jni::global_ref<>` default-constructs to null,
46+
// so no JNI call happens when it is created, copied into the manager, or
47+
// released. The surface-registry methods under test never dereference
48+
// `javaUIManager_`.
49+
jni::global_ref<JFabricUIManager::javaobject> emptyUIManager_;
50+
51+
std::unique_ptr<FabricMountingManager> makeManager() {
52+
return std::make_unique<FabricMountingManager>(emptyUIManager_);
6153
}
6254
};
6355

packages/react-native/ReactAndroid/src/main/jni/react/jni/test/ModuleRegistryBuilderTest.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ namespace facebook::react {
2525
* exercised by the Robolectric / instrumentation tests that run against a
2626
* real JVM.
2727
*
28-
* The one branch that can be validated host-side without an attached JavaVM
28+
* The one branch that can be validated without calling into Java at all
2929
* is `buildNativeModuleList`'s null-collection guard: when the incoming
3030
* `alias_ref<JCollection<...>>` is a null reference, the function must
3131
* short-circuit and return an empty vector rather than dereferencing the
@@ -47,7 +47,7 @@ namespace facebook::react {
4747
* bring-up paths that legitimately pass no legacy Java modules). Because
4848
* the crash would only surface once the process actually reaches this
4949
* code with a null collection, catching it here — instead of relying on
50-
* a device-side smoke test — is the earliest signal available.
50+
* an app-level smoke test — is the earliest signal available.
5151
*
5252
* The `Instance` weak_ptr and `MessageQueueThread` shared_ptr are supplied
5353
* as empty on purpose: the guard runs before either is dereferenced, so

0 commit comments

Comments
 (0)