From 6b0512c8196da5356272cbdfc1f16f23891d4f9e Mon Sep 17 00:00:00 2001 From: David Vacca Date: Wed, 19 Sep 2018 07:59:12 -0700 Subject: [PATCH] Enable Fabric test using Fabric C++ implementation Summary: This diff: - Disables all tests but one of FabricViewTest - Disables all tests but one of FabricBenchmarkTest - Changes ReactAppTestActivity to run with Hermes The reason there is only one test running in each test class, is because the tear down process of Fabric is still flaky and it produces crashes when restarting RN. We are working on this right now and we will enable the rest of the tests after that's fixed. Reviewed By: achen1 Differential Revision: D9890700 fbshipit-source-id: a8716481eff15b77bd12b38aaaefd4e282c71f3b --- .../react/testing/ReactAppTestActivity.java | 3 +++ .../react/testing/ReactInstanceSpecForTest.java | 13 +++++++++++++ .../react/testing/ReactInstrumentationTest.java | 14 ++++++++++++-- .../react/bridge/CatalystInstanceImpl.java | 1 - .../react/bridge/ProxyJavaScriptExecutor.java | 3 +-- 5 files changed, 29 insertions(+), 5 deletions(-) diff --git a/ReactAndroid/src/androidTest/java/com/facebook/react/testing/ReactAppTestActivity.java b/ReactAndroid/src/androidTest/java/com/facebook/react/testing/ReactAppTestActivity.java index 043f2faf1..edb099f87 100644 --- a/ReactAndroid/src/androidTest/java/com/facebook/react/testing/ReactAppTestActivity.java +++ b/ReactAndroid/src/androidTest/java/com/facebook/react/testing/ReactAppTestActivity.java @@ -208,6 +208,9 @@ public class ReactAppTestActivity extends FragmentActivity .getReactInstanceManagerBuilder() .setApplication(getApplication()) .setBundleAssetName(bundleName); + if (spec.getJavaScriptExecutorFactory() != null) { + builder.setJavaScriptExecutorFactory(spec.getJavaScriptExecutorFactory()); + } if (!spec.getAlternativeReactPackagesForTest().isEmpty()) { builder.addPackages(spec.getAlternativeReactPackagesForTest()); } else { diff --git a/ReactAndroid/src/androidTest/java/com/facebook/react/testing/ReactInstanceSpecForTest.java b/ReactAndroid/src/androidTest/java/com/facebook/react/testing/ReactInstanceSpecForTest.java index 29809ccf7..8a638c03a 100644 --- a/ReactAndroid/src/androidTest/java/com/facebook/react/testing/ReactInstanceSpecForTest.java +++ b/ReactAndroid/src/androidTest/java/com/facebook/react/testing/ReactInstanceSpecForTest.java @@ -9,12 +9,14 @@ package com.facebook.react.testing; import android.annotation.SuppressLint; import com.facebook.react.ReactPackage; +import com.facebook.react.bridge.JavaScriptExecutorFactory; import com.facebook.react.bridge.JavaScriptModule; import com.facebook.react.bridge.NativeModule; import com.facebook.react.uimanager.ViewManager; import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import javax.annotation.Nullable; /** * A spec that allows a test to add additional NativeModules/JS modules to the ReactInstance. This @@ -29,12 +31,18 @@ public class ReactInstanceSpecForTest { private final List> mJSModuleSpecs = new ArrayList<>(); private final List mViewManagers = new ArrayList<>(); private final ArrayList mReactPackages = new ArrayList<>(); + @Nullable private JavaScriptExecutorFactory mJavaScriptExecutorFactory = null; public ReactInstanceSpecForTest addNativeModule(NativeModule module) { mNativeModules.add(module); return this; } + public ReactInstanceSpecForTest setJavaScriptExecutorFactory(JavaScriptExecutorFactory javaScriptExecutorFactory) { + mJavaScriptExecutorFactory = javaScriptExecutorFactory; + return this; + } + public ReactInstanceSpecForTest setPackage(ReactPackage reactPackage) { if (!mReactPackages.isEmpty()) { throw new IllegalStateException( @@ -66,6 +74,11 @@ public class ReactInstanceSpecForTest { return mReactPackages.get(0); } + @Nullable + public JavaScriptExecutorFactory getJavaScriptExecutorFactory() { + return mJavaScriptExecutorFactory; + } + public List getAlternativeReactPackagesForTest() { return mReactPackages; } diff --git a/ReactAndroid/src/androidTest/java/com/facebook/react/testing/ReactInstrumentationTest.java b/ReactAndroid/src/androidTest/java/com/facebook/react/testing/ReactInstrumentationTest.java index c8c914f3e..dcfd33e01 100644 --- a/ReactAndroid/src/androidTest/java/com/facebook/react/testing/ReactInstrumentationTest.java +++ b/ReactAndroid/src/androidTest/java/com/facebook/react/testing/ReactInstrumentationTest.java @@ -11,9 +11,11 @@ import android.content.Intent; import android.test.ActivityInstrumentationTestCase2; import android.view.View; import android.view.ViewGroup; +import com.facebook.react.bridge.JavaScriptExecutorFactory; import com.facebook.react.bridge.JavaScriptModule; import com.facebook.react.bridge.ReactContext; import com.facebook.react.testing.idledetection.IdleWaiter; +import javax.annotation.Nullable; /** * Base class for instrumentation tests that runs React based application. @@ -27,6 +29,9 @@ public abstract class ReactInstrumentationTest extends protected StringRecordingModule mRecordingModule; + @Nullable + protected JavaScriptExecutorFactory mJavaScriptExecutorFactory = null; + public ReactInstrumentationTest() { super(ReactAppTestActivity.class); } @@ -49,7 +54,7 @@ public abstract class ReactInstrumentationTest extends /** * Renders this component within this test's activity */ - public void renderComponent(final String componentName) throws Exception { + public void renderComponent(final String componentName) { getActivity().renderComponent(componentName, null); waitForBridgeAndUIIdle(); } @@ -98,7 +103,12 @@ public abstract class ReactInstrumentationTest extends * Override this method to provide extra native modules to be loaded before the app starts */ protected ReactInstanceSpecForTest createReactInstanceSpecForTest() { - return new ReactInstanceSpecForTest().addNativeModule(mRecordingModule); + ReactInstanceSpecForTest reactInstanceSpecForTest = + new ReactInstanceSpecForTest().addNativeModule(mRecordingModule); + if (mJavaScriptExecutorFactory != null) { + reactInstanceSpecForTest.setJavaScriptExecutorFactory(mJavaScriptExecutorFactory); + } + return reactInstanceSpecForTest; } /** diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstanceImpl.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstanceImpl.java index 3961083f7..ba257c840 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstanceImpl.java +++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstanceImpl.java @@ -25,7 +25,6 @@ import com.facebook.react.common.ReactConstants; import com.facebook.react.common.annotations.VisibleForTesting; import com.facebook.systrace.Systrace; import com.facebook.systrace.TraceListener; -import java.lang.annotation.Native; import java.lang.ref.WeakReference; import java.util.ArrayList; import java.util.Collection; diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/ProxyJavaScriptExecutor.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/ProxyJavaScriptExecutor.java index feba9bc1a..ea5d126f7 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/ProxyJavaScriptExecutor.java +++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/ProxyJavaScriptExecutor.java @@ -7,10 +7,9 @@ package com.facebook.react.bridge; -import javax.annotation.Nullable; - import com.facebook.jni.HybridData; import com.facebook.proguard.annotations.DoNotStrip; +import javax.annotation.Nullable; /** * JavaScript executor that delegates JS calls processed by native code back to a java version