Add android adb command
#782
Conversation
|
|
||
| try | ||
| { | ||
| var result = runner.RunAdbCommand(string.Join(" ", PassThroughArguments), Arguments.Timeout); |
There was a problem hiding this comment.
Does it handle white space in the arguments correctly? For example if I call xharness adb -- "some arg with spaces" it should pass it as a single argument.
There was a problem hiding this comment.
Good point. I blindly trusted the existing Android code but now I verified it and we were not doing this so I added quotation from Apple side (we unfortunately have different process managers 😥 for some reason)
There was a problem hiding this comment.
I think the most correct way to handle this would be to keep the arguments as an array and use ProcessStartInfo.ArgumentList instead of Arguments. It's more errorproof than making sure that everything is quoted correctly.
There was a problem hiding this comment.
Oooh, I was too fast and didn't even think for a second arguments is a string 😅
I see the failures in tests now:
Executing command: 'adb.exe "devices -l"'
There was a problem hiding this comment.
I think the most correct way would be to switch to a single implementation in this repo which would be the one coming from Xamarin that already does all this
There was a problem hiding this comment.
@lpatalas I switched AdbRunner to accept an array of arguments instead. It uncovered many places where the code wasn't escaping potential problems in paths and other places.
src/Microsoft.DotNet.XHarness.Android/Execution/AdbProcessManager.cs
Outdated
Show resolved
Hide resolved
In dotnet/xharness#782 we introduced a the android adb command and removed android state --adb so we now need to use that to call ADB
Resolves #726