Skip to content

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

Merged
merged 3 commits into from
Apr 13, 2017
Merged

Conversation

rosen-vladimirov
Copy link
Contributor

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 the sleep method and in fact we are not waiting at all. Add the missing await, which fixes the tns 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.

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.
Copy link
Contributor

@Mitko-Kerezov Mitko-Kerezov left a 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);
Copy link
Contributor

@yyosifov yyosifov Apr 13, 2017

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.

Copy link
Contributor

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;
		}
	}

Copy link
Contributor Author

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.

@rosen-vladimirov rosen-vladimirov merged commit c203cc9 into master Apr 13, 2017
@rosen-vladimirov rosen-vladimirov deleted the vladimirov/fix-android-debugger branch April 13, 2017 08:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants