Skip to content

[e2e-testing][Appium] Adding support for android:id #9942

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions Libraries/Components/View/View.js
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,30 @@ const View = React.createClass({
* Used to locate this view in end-to-end tests.
*
* > This disables the 'layout-only view removal' optimization for this view!
*
* ### Android Specifics
*
* While React Native does *not* utilize XML based layouts for android Views it
* is still possible to add [android:id](https://developer.android.com/reference/android/view/View.html#attr_android:id)
* to the underlying View in order to support
* [findViewById](https://developer.android.com/reference/android/app/Activity.html#findViewById(int)).
*
* This is achieved by:
*
* 1. Defining a resource id in your android project's `res` folder (typically at
* `./android/app/src/main/res/values/ids.xml`).
*
* 2. Adding your resource ids to `ids.xml` e.g.
*
* ```xml
* <?xml version="1.0" encoding="utf-8"?>
* <resources>
* <item name="login_button" type="id"/>
* </resources>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting approach!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I couldn't think of a better one. The resource ids need to be created at compile time in order for uiautomatorviewer to recognize them.

*
* ```
* 3. Using the resource id as `testID` e.g. `<View testID="login_button">`.
*
*/
testID: PropTypes.string,

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ protected void onCreate(Bundle savedInstanceState) {
setContentView(rootView);

mScreenshotingFrameLayout = new ScreenshotingFrameLayout(this);
mScreenshotingFrameLayout.setId(ROOT_VIEW_ID);
mScreenshotingFrameLayout.setTag(ROOT_VIEW_ID);
rootView.addView(mScreenshotingFrameLayout);

mReactRootView = new ReactRootView(this);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import com.facebook.react.bridge.NativeModuleCallExceptionHandler;
import com.facebook.react.bridge.WritableNativeMap;
import com.facebook.react.bridge.queue.ReactQueueConfigurationSpec;
import com.facebook.react.common.TestIdUtil;
import com.facebook.react.cxxbridge.CatalystInstanceImpl;
import com.facebook.react.cxxbridge.JSBundleLoader;
import com.facebook.react.cxxbridge.JSCJavaScriptExecutor;
Expand Down Expand Up @@ -179,37 +180,6 @@ public static <T extends View> T getViewAtPath(ViewGroup rootView, int... path)
* propagated into view content description.
*/
public static View getViewWithReactTestId(View rootView, String testId) {
return findChild(rootView, hasTagValue(testId));
}

public static String getTestId(View view) {
return view.getTag() instanceof String ? (String) view.getTag() : null;
}

private static View findChild(View root, Predicate<View> predicate) {
if (predicate.apply(root)) {
return root;
}
if (root instanceof ViewGroup) {
ViewGroup viewGroup = (ViewGroup) root;
for (int i = 0; i < viewGroup.getChildCount(); i++) {
View child = viewGroup.getChildAt(i);
View result = findChild(child, predicate);
if (result != null) {
return result;
}
}
}
return null;
}

private static Predicate<View> hasTagValue(final String tagValue) {
return new Predicate<View>() {
@Override
public boolean apply(View view) {
Object tag = view.getTag();
return tag != null && tag.equals(tagValue);
}
};
return rootView.findViewById(TestIdUtil.getTestId(testId));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
import com.facebook.react.bridge.UiThreadUtil;
import com.facebook.react.modules.systeminfo.AndroidInfoModule;
import com.facebook.react.uimanager.PixelUtil;
import com.facebook.react.uimanager.UIImplementation;
import com.facebook.react.uimanager.UIImplementationProvider;
import com.facebook.react.uimanager.UIManagerModule;
import com.facebook.react.uimanager.ViewManager;
Expand All @@ -34,6 +33,8 @@
import com.facebook.react.testing.ReactIntegrationTestCase;
import com.facebook.react.testing.ReactTestHelper;

import static com.facebook.react.common.ViewMethodsUtil.reactTagFor;

/**
* Test case for basic {@link UIManagerModule} functionality.
*/
Expand Down Expand Up @@ -102,7 +103,7 @@ public void run() {

public void testFlexUIRendered() {
FrameLayout rootView = createRootView();
jsModule.renderFlexTestApplication(rootView.getId());
jsModule.renderFlexTestApplication(reactTagFor(rootView));
waitForBridgeAndUIIdle();

assertEquals(1, rootView.getChildCount());
Expand All @@ -126,7 +127,7 @@ public void testFlexUIRendered() {
// Find what could be different and make the test independent of env
// public void testFlexWithTextViews() {
// FrameLayout rootView = createRootView();
// jsModule.renderFlexWithTextApplication(rootView.getId());
// jsModule.renderFlexWithTextApplication(reactTagFor(rootView));
// waitForBridgeAndUIIdle();
//
// assertEquals(1, rootView.getChildCount());
Expand Down Expand Up @@ -164,7 +165,7 @@ public void testFlexUIRendered() {

public void testAbsolutePositionUIRendered() {
FrameLayout rootView = createRootView();
jsModule.renderAbsolutePositionTestApplication(rootView.getId());
jsModule.renderAbsolutePositionTestApplication(reactTagFor(rootView));
waitForBridgeAndUIIdle();

assertEquals(1, rootView.getChildCount());
Expand All @@ -178,7 +179,7 @@ public void testAbsolutePositionUIRendered() {

public void testUpdatePositionInList() {
FrameLayout rootView = createRootView();
jsModule.renderUpdatePositionInListTestApplication(rootView.getId());
jsModule.renderUpdatePositionInListTestApplication(reactTagFor(rootView));
waitForBridgeAndUIIdle();

ViewGroup containerView = getViewByTestId(rootView, "container");
Expand Down Expand Up @@ -207,7 +208,7 @@ public void testUpdatePositionInList() {

public void testAbsolutePositionBottomRightUIRendered() {
FrameLayout rootView = createRootView();
jsModule.renderAbsolutePositionBottomRightTestApplication(rootView.getId());
jsModule.renderAbsolutePositionBottomRightTestApplication(reactTagFor(rootView));
waitForBridgeAndUIIdle();

assertEquals(1, rootView.getChildCount());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@
import com.facebook.react.views.textinput.ReactTextChangedEvent;
import com.facebook.react.views.textinput.ReactTextInputEvent;

import static com.facebook.react.common.ViewMethodsUtil.reactTagFor;

/**
* Test to verify that TextInput renders correctly
*/
Expand Down Expand Up @@ -114,15 +116,15 @@ public void testMetionsInputColors() throws Throwable {

eventDispatcher.dispatchEvent(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why change these?

Copy link
Contributor Author

@jsdevel jsdevel Sep 23, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Simulates the behavior of reactEditText.getId() I.E. it returns View.NO_ID if no react tag has been set. Being that getTag returns Object this also removes the need to cast to int everywhere.

new ReactTextChangedEvent(
reactEditText.getId(),
reactTagFor(reactEditText),
newText.toString(),
(int) PixelUtil.toDIPFromPixel(contentWidth),
(int) PixelUtil.toDIPFromPixel(contentHeight),
reactEditText.incrementAndGetEventCounter()));

eventDispatcher.dispatchEvent(
new ReactTextInputEvent(
reactEditText.getId(),
reactTagFor(reactEditText),
newText.toString(),
"",
start,
Expand All @@ -146,15 +148,15 @@ public void testMetionsInputColors() throws Throwable {

eventDispatcher.dispatchEvent(
new ReactTextChangedEvent(
reactEditText.getId(),
reactTagFor(reactEditText),
newText.toString(),
(int) PixelUtil.toDIPFromPixel(contentWidth),
(int) PixelUtil.toDIPFromPixel(contentHeight),
reactEditText.incrementAndGetEventCounter()));

eventDispatcher.dispatchEvent(
new ReactTextInputEvent(
reactEditText.getId(),
reactTagFor(reactEditText),
moreText,
"",
start,
Expand All @@ -178,15 +180,15 @@ public void testMetionsInputColors() throws Throwable {

eventDispatcher.dispatchEvent(
new ReactTextChangedEvent(
reactEditText.getId(),
reactTagFor(reactEditText),
newText.toString(),
(int) PixelUtil.toDIPFromPixel(contentWidth),
(int) PixelUtil.toDIPFromPixel(contentHeight),
reactEditText.incrementAndGetEventCounter()));

eventDispatcher.dispatchEvent(
new ReactTextInputEvent(
reactEditText.getId(),
reactTagFor(reactEditText),
moreText,
"",
start,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@
import static com.facebook.react.bridge.ReactMarkerConstants.PROCESS_PACKAGES_START;
import static com.facebook.react.bridge.ReactMarkerConstants.SETUP_REACT_CONTEXT_END;
import static com.facebook.react.bridge.ReactMarkerConstants.SETUP_REACT_CONTEXT_START;
import static com.facebook.react.common.ViewMethodsUtil.reactTagFor;
import static com.facebook.systrace.Systrace.TRACE_TAG_REACT_JAVA_BRIDGE;

/**
Expand Down Expand Up @@ -797,7 +798,7 @@ private void attachMeasuredRootViewToInstance(

// Reset view content as it's going to be populated by the application content from JS
rootView.removeAllViews();
rootView.setId(View.NO_ID);
rootView.setTag(View.NO_ID);

UIManagerModule uiManagerModule = catalystInstance.getNativeModule(UIManagerModule.class);
int rootTag = uiManagerModule.addMeasuredRootView(rootView);
Expand All @@ -818,7 +819,7 @@ private void detachViewFromInstance(
CatalystInstance catalystInstance) {
UiThreadUtil.assertOnUiThread();
catalystInstance.getJSModule(AppRegistry.class)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a fan of adding complexity here.

Copy link
Contributor Author

@jsdevel jsdevel Sep 23, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.unmountApplicationComponentAtRootTag(rootView.getId());
.unmountApplicationComponentAtRootTag(reactTagFor(rootView));
}

private void tearDownReactContext(ReactContext reactContext) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
package com.facebook.react.common;

import android.view.View;

import com.facebook.react.common.annotations.VisibleForTesting;

import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.atomic.AtomicInteger;

public class TestIdUtil {
private static final ConcurrentHashMap<String, Integer> mTestIds = new ConcurrentHashMap<>();
// Integer values in R.class are typically large. To avoid colliding with R.class we
// use smaller values for ids when no resource id exists.
private static final int mStartingInternalId = 1;
private static final AtomicInteger mInternalId = new AtomicInteger(mStartingInternalId);

/**
* Looks for defined resource IDs in R.class by the name of testId and if a matching resource ID is
* found it is passed to the view's setId method. If the given testId cannot be found in R.class,
* an increment value is assigned instead.
*/
public static <T extends View> void setTestId(T view, String testId) {
int mappedTestId;
if (!mTestIds.containsKey(testId)) {
mappedTestId = view.getResources().getIdentifier(testId, "id", view.getContext().getPackageName());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do the resource lookup? If you remove this branch and just use the map, there's no need to define ids in the xml file, correct?

Copy link
Contributor Author

@jsdevel jsdevel Oct 25, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ids should be in the xml file so R.class is generated correctly; otherwise, uiautomatorviewer and appium can't associate it to the testID.

final boolean idNotFoundInResources = mappedTestId <= 0;
if (idNotFoundInResources) {
mappedTestId = mInternalId.getAndIncrement();
}
mTestIds.put(testId, mappedTestId);
} else {
mappedTestId = mTestIds.get(testId);
}

if (mappedTestId != 0 && view.getId() != mappedTestId) {
view.setId(mappedTestId);
}
}

/**
* Used for e2e tests that do not yet have testIDs stored in ids.xml. It is strongly
* advised that you reference ids that have been generated in R.class to avoid collisions and
* to properly support UIAutomatorViewer.
*/
@VisibleForTesting
public static int getTestId(String testId) {
return mTestIds.containsKey(testId) ? mTestIds.get(testId) : View.NO_ID;
}

@VisibleForTesting
public static void resetStateInTest() {
mTestIds.clear();
mInternalId.set(mStartingInternalId);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package com.facebook.react.common;

import android.view.View;

public class ViewMethodsUtil {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: ViewUtil. The fact that a class contains methods is implied.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Android has a ViewUtil class. Would it be confusing if we define one as well?


/**
* Returns the react tag for the view. If no react tag has been set then {@link View#NO_ID} is
* returned.
*/
public static int reactTagFor(View view) {
return view == null || view.getTag() == null ?
View.NO_ID :
(int) view.getTag();
}
}
1 change: 1 addition & 0 deletions ReactAndroid/src/main/java/com/facebook/react/touch/BUCK
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ android_library(
deps = [
react_native_dep('third-party/java/infer-annotations:infer-annotations'),
react_native_dep('third-party/java/jsr-305:jsr-305'),
react_native_target('java/com/facebook/react/common:common'),
],
visibility = [
'PUBLIC'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
import android.view.ViewGroup;
import android.view.ViewParent;

import static com.facebook.react.common.ViewMethodsUtil.reactTagFor;

/**
* This class coordinates JSResponder commands for {@link UIManagerModule}. It should be set as
* OnInterceptTouchEventListener for all newly created native views that implements
Expand Down Expand Up @@ -70,7 +72,7 @@ public boolean onInterceptTouchEvent(ViewGroup v, MotionEvent event) {
// Therefore since "UP" event is the last event in a gesture, we should just let it reach the
// original target that is a child view of {@param v}.
// http://developer.android.com/reference/android/view/ViewGroup.html#onInterceptTouchEvent(android.view.MotionEvent)
return v.getId() == currentJSResponder;
return reactTagFor(v) == currentJSResponder;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm thinking getReactTag might actually be more obvious. Somehow when I see reactTagFor I'm assuming it's going to do a lookup in a map. On the other hand "get" implies a simple (cheap) getter.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yea, getReactTag is also used in a few places as class methods, so I'm not sure renaming it would always be more readable. I'm open to changing it.

}
return false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@
import android.graphics.Color;
import android.os.Build;
import android.view.View;
import android.view.ViewGroup;

import com.facebook.react.bridge.ReadableArray;
import com.facebook.react.common.TestIdUtil;
import com.facebook.react.uimanager.annotations.ReactProp;

/**
Expand Down Expand Up @@ -84,7 +84,7 @@ public void setRenderToHardwareTexture(T view, boolean useHWTexture) {

@ReactProp(name = PROP_TEST_ID)
public void setTestId(T view, String testId) {
view.setTag(testId);
TestIdUtil.setTestId(view, testId);
}

@ReactProp(name = PROP_ACCESSIBILITY_LABEL)
Expand Down
Loading