-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Idle wait before ready #1150
Idle wait before ready #1150
Conversation
import android.support.test.espresso.Espresso | ||
import com.wix.detox.espresso.UiAutomatorHelper | ||
|
||
class ActionsFacade { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What did we do wrong to have a Facade
as a class name? 😣
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lol, I don't care much about names. It's something that simplifies and aggregates other things we have spread around in other places (mainly helpers), instead of providing its own direct implementation (which is a facade basically 🤔). In our case, it also makes actions unit-testable. wdyt? 🤷♂️
@@ -102,6 +101,14 @@ public void onAction(final String type, final String params, final long messageI | |||
handler.post(new Runnable() { | |||
@Override | |||
public void run() { | |||
|
|||
final ExternalAction externalAction = externalActions.get(type); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does External mean?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actions that are externally invoked (by the test runner).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you mean device
API?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm not necessarily, i think
} | ||
|
||
@Override | ||
public void onClosed() { | ||
stop(); | ||
} | ||
|
||
private void initReactNativeIfNeeded() { | ||
if (ReactNativeSupport.isReactNativeApp()) { | ||
ReactNativeCompat.waitForReactNativeLoad(reactNativeHostHolder); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to think of a way to support native apps with no RN as dependency. Currently native apps crash in classNotFound when running a detox test .
But this is probably a different issue
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's different but sounds important nonetheless. Do we have an actual issue? I don't understand why this would happen, though, because we only check for the ReactApplication
class' existence
ee51895
to
3d6d3ec
Compare
Description:
A fix for issue #1149. Putting the testable refactoring aside, the suggested solution is to add an
Espresso.onIdle()
before sendingready
messages over the web socket.