Skip to content
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

Android: Support RN50-51 changes #652

Merged
merged 5 commits into from
Apr 4, 2018
Merged
Show file tree
Hide file tree
Changes from 4 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
6 changes: 3 additions & 3 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ matrix:
os: osx
osx_image: xcode9.2
env:
- REACT_NATIVE_VERSION=0.49.3
- REACT_NATIVE_VERSION=0.51.1
install:
- ./scripts/install.ios.sh
script:
Expand All @@ -25,7 +25,7 @@ matrix:
os: linux
jdk: oraclejdk8
env:
- REACT_NATIVE_VERSION=0.49.3
- REACT_NATIVE_VERSION=0.51.1
android:
components:
- build-tools-27.0.2
Expand All @@ -44,7 +44,7 @@ matrix:
os: osx
osx_image: xcode9
env:
- REACT_NATIVE_VERSION=0.49.3
- REACT_NATIVE_VERSION=0.51.1
install:
- ./scripts/install.ios.sh
script:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ class DetoxManager implements WebSocketClient.ActionHandler {
void start() {
if (detoxServerUrl != null && detoxSessionId != null) {
if (ReactNativeSupport.isReactNativeApp()) {
ReactNativeSupport.waitForReactNativeLoad(reactNativeHostHolder);
ReactNativeCompat.waitForReactNativeLoad(reactNativeHostHolder);
}

wsClient = new WebSocketClient(this);
Expand Down Expand Up @@ -124,7 +124,6 @@ public void run() {
}
break;
case "isReady":
// It's always ready, because reload, waitForRn are both synchronous.
wsClient.sendAction("ready", Collections.emptyMap(), messageId);
break;
case "cleanup":
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
package com.wix.detox;

import org.joor.Reflect;

import java.util.Map;

public class ReactNativeCompat {
static Map<String, Object> VERSION;

static {
try {
Class reactNativeVersion = Class.forName("com.facebook.react.modules.systeminfo.ReactNativeVersion");
Copy link
Contributor

@simonracz simonracz Apr 4, 2018

Choose a reason for hiding this comment

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

This class is present only in RN 50 and above.

Later, the getMinor call could fail on these older versions. As we don't actually use this API on older versions, I think we could just set it to 49.5.0 in the catch as a default version.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch, thanks

VERSION = Reflect.on(reactNativeVersion).field("VERSION").get();
} catch (ClassNotFoundException e) {
}

}

public static int getMinor() {
return (Integer) VERSION.get("minor");
}

public static void waitForReactNativeLoad(Object reactNativeHostHolder) {
if (getMinor() >= 50) {
ReactNativeSupport.waitForReactNativeLoad(reactNativeHostHolder);
try {
//TODO- Temp hack to make Detox usable for RN>=50 till we find a better sync solution.
Thread.sleep(1000);
} catch (InterruptedException e) {
e.printStackTrace();
}
} else {
ReactNativeSupport.waitForReactNativeLoad(reactNativeHostHolder);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ public void run() {
}
});

waitForReactNativeLoad(reactNativeHostHolder);
ReactNativeCompat.waitForReactNativeLoad(reactNativeHostHolder);
}

// Ideally we would not store this at all.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,11 @@
import android.util.Log;
import android.view.Choreographer;

import com.wix.detox.ReactNativeCompat;

import org.joor.Reflect;
import org.joor.ReflectException;

import java.sql.Ref;

/**
* Created by simonracz on 25/08/2017.
*/
Expand All @@ -18,13 +18,14 @@
* <p>
* Espresso IdlingResource for React Native's Animated Module.
* </p>
*
* <p>
* <p>
* Hooks up to React Native internals to monitor the state of the animations.
* </p>
*
* <p>
* This Idling Resource is inherently tied to the UI Module IR. It must be registered after
* the UI Module IR. This order is not enforced now.
*
* @see <a href="https://github.com/facebook/react-native/blob/259eac8c30b536abddab7925f4c51f0bf7ced58d/ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedModule.java#L143">AnimatedModule</a>
*/
public class AnimatedModuleIdlingResource implements IdlingResource, Choreographer.FrameCallback {
Expand Down Expand Up @@ -82,91 +83,102 @@ public boolean isIdleNow() {
return false;
}

if (!(boolean)Reflect.on(reactContext).call(METHOD_HAS_NATIVE_MODULE, animModuleClass).get()) {
if (!(boolean) Reflect.on(reactContext).call(METHOD_HAS_NATIVE_MODULE, animModuleClass).get()) {
Log.e(LOG_TAG, "Can't find Animated Module.");
if (callback != null) {
callback.onTransitionToIdle();
}
return true;
}

Object animModule = Reflect.on(reactContext)
.call(METHOD_GET_NATIVE_MODULE, animModuleClass)
.get();
Object operationsLock = Reflect.on(animModule)
.field(LOCK_OPERATIONS)
.get();
boolean operationsAreEmpty;
boolean animationsConsideredIdle;
synchronized (operationsLock) {
Object operations = Reflect.on(animModule)
.field(FIELD_OPERATIONS).get();
if (operations == null) {
operationsAreEmpty = true;
} else {
operationsAreEmpty = Reflect.on(operations).call(METHOD_IS_EMPTY).get();
if (ReactNativeCompat.getMinor() >= 51) {
if(isIdleRN51(animModuleClass)) {
return true;
}
}
Object nodesManager = Reflect.on(animModule)
.field(FIELD_NODES_MANAGER)
.get();

// We do this in this complicated way
// to not consider looped animations
// as a busy state.
int updatedNodesSize = Reflect.on(nodesManager)
.field(FIELD_UPDATED_NODES)
.call(METHOD_SIZE)
.get();
if (updatedNodesSize > 0) {
animationsConsideredIdle = false;
} else {
Object activeAnims = Reflect.on(nodesManager)
.field(FIELD_ACTIVE_ANIMATIONS)
.get();
int activeAnimsSize = Reflect.on(activeAnims)
.call(METHOD_SIZE)
.get();
if (activeAnimsSize == 0) {
animationsConsideredIdle = true;
} else {
animationsConsideredIdle = true;
for (int i = 0; i < activeAnimsSize; ++i) {
int iterations = Reflect.on(activeAnims)
.call(METHOD_VALUE_AT, i)
.field(FIELD_ITERATIONS)
.get();
// -1 means it is looped
if (iterations != -1) {
animationsConsideredIdle = false;
break;
}
}
if (isIdleRNOld(animModuleClass)) {
return true;
}
}

if (operationsAreEmpty && animationsConsideredIdle) {
if (callback != null) {
callback.onTransitionToIdle();
}
// Log.i(LOG_TAG, "AnimatedModule is idle.");
return true;
}

Log.i(LOG_TAG, "AnimatedModule is busy.");
Choreographer.getInstance().postFrameCallback(this);
return false;
} catch (ReflectException e) {
// Log.e(LOG_TAG, "Couldn't set up RN AnimatedModule listener, old RN version?");
// Log.e(LOG_TAG, "Can't set up RN AnimatedModule listener", e.getCause());
Log.e(LOG_TAG, "Couldn't set up RN AnimatedModule listener, old RN version?");
Log.e(LOG_TAG, "Can't set up RN AnimatedModule listener", e.getCause());
}

if (callback != null) {
callback.onTransitionToIdle();
}
Log.i(LOG_TAG, "AnimatedModule is idle.");
return true;
}

private boolean isIdleRN51(Object animModuleClass) {
Object animModule = Reflect.on(reactContext).call(METHOD_GET_NATIVE_MODULE, animModuleClass).get();
Object nodesManager = Reflect.on(animModule).call("getNodesManager").get();
boolean hasActiveAnimations = Reflect.on(nodesManager).call("hasActiveAnimations").get();
if (!hasActiveAnimations) {
if (callback != null) {
callback.onTransitionToIdle();
}
Log.i(LOG_TAG, "AnimatedModule is idle, no operations");
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we shouldn't log out idle states. Only busy states.

Originally, I also logged both states, but after the initial debugging it become overwhelming.

Copy link
Member Author

Choose a reason for hiding this comment

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

Agreed

Copy link
Contributor

@simonracz simonracz Apr 4, 2018

Choose a reason for hiding this comment

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

I looked at this diff.

I am not even sure that we need an animatedIdlingResource at all on later RN versions. It seems, they delegate everything to the UIModule. I don't fully understand the new flow in that code though.

The check here certainly doesn't do any harm.

Copy link
Member Author

Choose a reason for hiding this comment

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

Well, it was a trial and error kinda fix :)
But I think that there's a small gap between the time when the UIBlock is posted and when it starts executing.

return true;
}
return false;
}

private boolean isIdleRNOld(Object animModuleClass) {
Object animModule = Reflect.on(reactContext).call(METHOD_GET_NATIVE_MODULE, animModuleClass).get();
Object operationsLock = Reflect.on(animModule).field(LOCK_OPERATIONS).get();
boolean operationsAreEmpty;
boolean animationsConsideredIdle;
synchronized (operationsLock) {
Object operations = Reflect.on(animModule).field(FIELD_OPERATIONS).get();
if (operations == null) {
operationsAreEmpty = true;
} else {
operationsAreEmpty = Reflect.on(operations).call(METHOD_IS_EMPTY).get();
}
}
Object nodesManager = Reflect.on(animModule).field(FIELD_NODES_MANAGER).get();

// We do this in this complicated way
// to not consider looped animations
// as a busy state.
int updatedNodesSize = Reflect.on(nodesManager).field(FIELD_UPDATED_NODES).call(METHOD_SIZE).get();
if (updatedNodesSize > 0) {
animationsConsideredIdle = false;
} else {
Object activeAnims = Reflect.on(nodesManager).field(FIELD_ACTIVE_ANIMATIONS).get();
int activeAnimsSize = Reflect.on(activeAnims).call(METHOD_SIZE).get();
if (activeAnimsSize == 0) {
animationsConsideredIdle = true;
} else {
animationsConsideredIdle = true;
for (int i = 0; i < activeAnimsSize; ++i) {
int iterations = Reflect.on(activeAnims).call(METHOD_VALUE_AT, i).field(FIELD_ITERATIONS).get();
// -1 means it is looped
if (iterations != -1) {
animationsConsideredIdle = false;
break;
}
}
}
}

if (operationsAreEmpty && animationsConsideredIdle) {
if (callback != null) {
callback.onTransitionToIdle();
}
Log.i(LOG_TAG, "AnimatedModule is idle.");
return true;
}
return false;
}

@Override
public void registerIdleTransitionCallback(ResourceCallback callback) {
this.callback = callback;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ public void onOpen(WebSocket webSocket, Response response) {

@Override
public void onFailure(WebSocket webSocket, Throwable t, Response response) {
Log.e(LOG_TAG, "Detox Error: ", t);
// Log.e(LOG_TAG, "Detox Error: ", t);

//OKHttp won't recover from failure if it got ConnectException,
// this is a workaround to make the websocket client try reconnecting when failed.
Expand Down
2 changes: 0 additions & 2 deletions detox/src/android/expect.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ function setInvocationManager(im) {
invocationManager = im;
}

const ViewAssertions = 'android.support.test.espresso.assertion.ViewAssertions';
const DetoxMatcher = 'com.wix.detox.espresso.DetoxMatcher';
const DetoxAssertion = 'com.wix.detox.espresso.DetoxAssertion';
const EspressoDetox = 'com.wix.detox.espresso.EspressoDetox';
Expand Down Expand Up @@ -107,7 +106,6 @@ class SwipeAction extends Action {

class Interaction {
async execute() {
//if (!this._call) throw new Error(`Interaction.execute cannot find a valid _call, got ${typeof this._call}`);
await invocationManager.execute(this._call);
}
}
Expand Down
1 change: 1 addition & 0 deletions detox/src/client/Client.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
const AsyncWebSocket = require('./AsyncWebSocket');
const actions = require('./actions/actions');
const argparse = require('../utils/argparse');
const retry = require('../utils/retry');

class Client {
constructor(config) {
Expand Down
6 changes: 3 additions & 3 deletions detox/test/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@
"build:android": "detox build --configuration android.emu.release"
},
"dependencies": {
"react": "16.0.0-beta.5",
"react-native": "0.49.3"
"react": "16.0.0",
"react-native": "0.51.x"
},
"devDependencies": {
"detox": "^7.0.0",
Expand Down Expand Up @@ -69,4 +69,4 @@
}
}
}
}
}