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: Increase RN-timers idling resources look-ahead to 1.5sec #1037

Merged
merged 3 commits into from
Nov 14, 2018
Merged
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
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,6 @@ public void run() {
* @param reactNativeHostHolder the object that has a getReactNativeHost() method
*/
static void waitForReactNativeLoad(@NonNull Context reactNativeHostHolder) {

if (!isReactNativeApp()) {
return;
}
Expand Down Expand Up @@ -184,7 +183,6 @@ private static void setupEspressoIdlingResources(@NonNull ReactContext reactCont

setupReactNativeQueueInterrogators(reactContext);


rnBridgeIdlingResource = new ReactBridgeIdlingResource(reactContext);
rnTimerIdlingResource = new ReactNativeTimersIdlingResource(reactContext);
rnUIModuleIdlingResource = new ReactNativeUIModuleIdlingResource(reactContext);
Expand Down Expand Up @@ -303,4 +301,14 @@ private static void removeNetworkIdlingResource() {
networkIR = null;
}
}

public static void pauseRNTimersIdlingResource() {
if (rnTimerIdlingResource != null) {
rnTimerIdlingResource.pause();
}
}

public static void resumeRNTimersIdlingResource() {
rnTimerIdlingResource.resume();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
package com.wix.detox.espresso;

import android.support.test.espresso.UiController;
import android.support.test.espresso.ViewAction;
import android.support.test.espresso.action.ViewActions;
import android.view.View;

import com.wix.detox.ReactNativeSupport;

import org.hamcrest.Matcher;

/**
* An alternative to {@link ViewActions} - providing alternative implementations, where needed.
*/
public class DetoxViewActions {
private static class BusyViewActionWrapper implements ViewAction {
private final ViewAction espressoViewAction;

BusyViewActionWrapper(ViewAction clickAction) {
this.espressoViewAction = clickAction;
}

@Override
public Matcher<View> getConstraints() {
return espressoViewAction.getConstraints();
}

@Override
public String getDescription() {
return espressoViewAction.getDescription();
}

@Override
public void perform(UiController uiController, View view) {
ReactNativeSupport.pauseRNTimersIdlingResource();
espressoViewAction.perform(uiController, view);
ReactNativeSupport.resumeRNTimersIdlingResource();
uiController.loopMainThreadUntilIdle();
}
}

public static ViewAction click() {
final ViewAction clickAction = ViewActions.click();
return new BusyViewActionWrapper(clickAction);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,15 @@ public class ReactNativeTimersIdlingResource implements IdlingResource, Choreogr
private final static String METHOD_GET_NATIVE_MODULE = "getNativeModule";
private final static String METHOD_HAS_NATIVE_MODULE = "hasNativeModule";
private final static String FIELD_TIMERS = "mTimers";
private final static String FIELD_TARGET_TIME = "mTargetTime";
private final static String TIMER_FIELD_TARGET_TIME = "mTargetTime";
private final static String TIMER_FIELD_INTERVAL = "mInterval";
private final static String TIMER_FIELD_REPETITIVE = "mRepeat";
private final static String FIELD_CATALYST_INSTANCE = "mCatalystInstance";
private final static String LOCK_TIMER = "mTimerGuard";

private AtomicBoolean stopped = new AtomicBoolean(false);
private AtomicBoolean paused = new AtomicBoolean(false);

private static final long LOOK_AHEAD_MS = 15;
private static final long LOOK_AHEAD_MS = 1500;

private ResourceCallback callback = null;
private Object reactContext = null;
Expand All @@ -57,12 +59,10 @@ public String getName() {

@Override
public boolean isIdleNow() {
if (stopped.get()) {
if (callback != null) {
callback.onTransitionToIdle();
}
if (paused.get()) {
return true;
}

Class<?> timingClass;
try {
timingClass = Class.forName(CLASS_TIMING);
Expand Down Expand Up @@ -93,28 +93,21 @@ public boolean isIdleNow() {
Object timingModule = Reflect.on(reactContext).call(METHOD_GET_NATIVE_MODULE, timingClass).get();
Object timerLock = Reflect.on(timingModule).field(LOCK_TIMER).get();
synchronized (timerLock) {
PriorityQueue<?> timers = Reflect.on(timingModule).field(FIELD_TIMERS).get();
if (timers.isEmpty()) {
final PriorityQueue<?> timers = Reflect.on(timingModule).field(FIELD_TIMERS).get();
final Object nextTimer = timers.peek();
if (nextTimer == null) {
if (callback != null) {
callback.onTransitionToIdle();
}
return true;
}

// Log.i(LOG_TAG, "Num of Timers : " + timers.size());

long targetTime = Reflect.on(timers.peek()).field(FIELD_TARGET_TIME).get();
long currentTimeMS = System.nanoTime() / 1000000;

// Log.i(LOG_TAG, "targetTime " + targetTime + " currentTime " + currentTimeMS);
// Log.i(LOG_TAG, "Num of Timers : " + timers.size());

if (targetTime - currentTimeMS > LOOK_AHEAD_MS || targetTime < currentTimeMS) {
// Timer is too far in the future. Mark it as OK for now.
// This is similar to what Espresso does internally.
if (isTimerOutsideBusyWindow(nextTimer)) {
if (callback != null) {
callback.onTransitionToIdle();
}
// Log.i(LOG_TAG, "JS Timer is idle: true");
return true;
}
}
Expand Down Expand Up @@ -144,7 +137,46 @@ public void doFrame(long frameTimeNanos) {
isIdleNow();
}

public void stop() {
stopped.set(true);
public void pause() {
paused.set(true);
if (callback != null) {
callback.onTransitionToIdle();
}
}
public void resume() {
paused.set(false);
}

private boolean isTimerOutsideBusyWindow(Object nextTimer) {
final long currentTimeMS = System.nanoTime() / 1000000L;
final Reflect nextTimerReflected = Reflect.on(nextTimer);
final long targetTimeMS = nextTimerReflected.field(TIMER_FIELD_TARGET_TIME).get();
final int intervalMS = nextTimerReflected.field(TIMER_FIELD_INTERVAL).get();
final boolean isRepetitive = nextTimerReflected.field(TIMER_FIELD_REPETITIVE).get();

// Log.i(LOG_TAG, "Next timer has duration of: " + intervalMS
Copy link
Member

Choose a reason for hiding this comment

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

Not sure those comments are necessary, unless there's a hack that needs an explanation.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Assuming you're referring to the comments and not the logs -
they're necessary because we didn't unit test, so there's nothing here to explain the idea behind the implementation. This will change soon, of course.

// + "; due time is: " + targetTimeMS + ", current is: " + currentTimeMS
// + "; is " + (isRepetitive ? "repeating" : "a one-shot"));

// Before making any concrete checks, be sure to ignore repeating timers or we'd loop forever.
// TODO: Should we iterate to the first, non-repeating timer?
if (isRepetitive) {
return true;
}

// Core condition is for the timer interval (duration) to be set beyond our window.
// Note: we check the interval in an 'absolute' way rather than comparing to the 'current time'
// since it always takes a while till we get dispatched (compared to when the timer was created),
// and that could make a significant difference in timers set close to our window (up to ~ LOOK_AHEAD_MS+200ms).
if (intervalMS > LOOK_AHEAD_MS) {
return true;
}

// Edge case: timer has expired during this probing process and is yet to have left the queue.
if (targetTimeMS <= currentTimeMS) {
return true;
}

return false;
}
}
5 changes: 2 additions & 3 deletions detox/src/android/espressoapi/ViewActions.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
*/



class ViewActions {
static clearGlobalAssertions() {
return {
Expand Down Expand Up @@ -33,7 +32,7 @@ class ViewActions {
return {
target: {
type: "Class",
value: "android.support.test.espresso.action.ViewActions"
value: "com.wix.detox.espresso.DetoxViewActions"
},
method: "click",
args: []
Expand Down Expand Up @@ -238,4 +237,4 @@ class ViewActions {

}

module.exports = ViewActions;
module.exports = ViewActions;
2 changes: 1 addition & 1 deletion detox/test/e2e/03.actions.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ describe('Actions', () => {
await expect(element(by.text('Long Press Working!!!'))).toBeVisible();
});

it(':ios: should long press with duration on an element', async () => {
it('should long press with duration on an element', async () => {
await element(by.text('Long Press Me 1.5s')).longPress(1500);
await expect(element(by.text('Long Press With Duration Working!!!'))).toBeVisible();
});
Expand Down
2 changes: 1 addition & 1 deletion detox/test/e2e/09.stress-timeouts.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ describe('StressTimeouts', () => {
await element(by.text('Timeouts')).tap();
});

it(':ios: should handle a short timeout', async () => {
it('should handle a short timeout', async () => {
await element(by.id('TimeoutShort')).tap();
await expect(element(by.text('Short Timeout Working!!!'))).toBeVisible();
});
Expand Down
8 changes: 4 additions & 4 deletions detox/test/e2e/12.animations.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,11 @@ describe('Animations', () => {
await _startTest(driver, {delay: 1600});
await expect(element(by.id('UniqueId_AnimationsScreen_afterAnimationText'))).toNotExist();
});
it(`:ios: should wait during delays shorter than 1.5s (driver: ${driver})`, async () => {

it(`should wait during delays shorter than 1.5s (driver: ${driver})`, async () => {
await _startTest(driver, {delay: 500});
await expect(element(by.id('UniqueId_AnimationsScreen_afterAnimationText'))).toExist();
});
});

});
});