-
-
Notifications
You must be signed in to change notification settings - Fork 197
Fix debug android command #2713
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
Conversation
When executing `tns debug android` for slow emulators/devices we have logic to wait several seconds until debugger is attached. However we've missed to `await` the `sleep` method and in fact we are not waiting at all. Add the missing await, which fixes the `tns debug android` command.
A while ago, we had a logic to find out which node_modules should be processed. However it is not used anymore (removed in #2152 ). So remove the code - we do not need it anymore.
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.
Looking good
@@ -174,7 +174,7 @@ class AndroidDebugService extends DebugServiceBase implements IPlatformDebugServ | |||
debugerStarted = forwardsResult.indexOf(waitText) === -1; | |||
|
|||
if (!debugerStarted) { | |||
sleep(500); | |||
await sleep(500); |
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.
Great! :)
Small typo: debugerStarted
=> debuggerStarted
What do you think of introducing a function wait.Until(function, condition, sleepIntervalMs)
which implements a loop which sleeps for several iterations or until a condition is met? This way, we can introduce unit tests for it, reuse it and don't worry about such bugs anymore? I've seen this implemented and it was working good.
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.
Here's the snippet I've been referring to (from another project and in C#):
public class PollingService : IPollingService
{
public bool WaitUntil(Func<bool> successCriteria, int millisecondsInterval, int iterations)
{
var succeeded = false;
for (int i = 0; i < iterations; i++)
{
if (successCriteria())
{
succeeded = true;
break;
}
else if (i < iterations - 1)
{
Thread.Sleep(millisecondsInterval);
}
}
return succeeded;
}
}
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.
I like the idea and I suggest to implement it in another PR (where we will add tests for it). We can use it on several places in CLI (at least two :) )
Thanks for the suggestion.
Fix Android debugging
When executing
tns debug android
for slow emulators/devices we have logic to wait several seconds until debugger is attached.However we've missed to
await
thesleep
method and in fact we are not waiting at all. Add the missing await, which fixes thetns debug android
command.Remove unused code
A while ago, we had a logic to find out which node_modules should be processed. However it is not used anymore (removed in #2152 ).
So remove the code - we do not need it anymore.