Skip to content
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
84 changes: 70 additions & 14 deletions src/Appium.Net/Appium/Service/AppiumLocalService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
using System.Collections.Generic;
using System.Diagnostics;
using System.IO;
using System.Linq;
using System.Net;
using System.Runtime.CompilerServices;

Expand All @@ -32,6 +33,7 @@ public class AppiumLocalService : ICommandServer
private readonly TimeSpan InitializationTimeout;
private readonly IDictionary<string, string> EnvironmentForProcess;
private Process Service;
private List<string> ArgsList;

/// <summary>
/// Creates an instance of AppiumLocalService without special settings
Expand All @@ -40,11 +42,11 @@ public class AppiumLocalService : ICommandServer
public static AppiumLocalService BuildDefaultService() => new AppiumServiceBuilder().Build();

internal AppiumLocalService(
FileInfo nodeJS,
string arguments,
IPAddress ip,
FileInfo nodeJS,
string arguments,
IPAddress ip,
int port,
TimeSpan initializationTimeout,
TimeSpan initializationTimeout,
IDictionary<string, string> environmentForProcess)
{
NodeJS = nodeJS;
Expand All @@ -58,7 +60,7 @@ internal AppiumLocalService(
/// <summary>
/// The base URL for the managed appium server.
/// </summary>
public Uri ServiceUrl => new Uri($"http://{IP.ToString()}:{Convert.ToString(Port)}/wd/hub");
public Uri ServiceUrl => new Uri($"http://{IP}:{Convert.ToString(Port)}");

Choose a reason for hiding this comment

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

this will make the service fully incompatible with Appium 1. Also, one could provide --base-path/-bp argument to change this path. We should be able to support such approach. At least Python and Java clients do

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@mykola-mokhnach , already discussed the fact it will be incompatible with appium 1 with @akinsolb, this is the reason we will merge to release 5.0 branch.
I will try to look what I can do with the --base-path/-bp argument in order to support it fully.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I didn't know the other bindings supported v1. It would be ideal if we could support both, where users can build their own client

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@akinsolb /@mykola-mokhnach Do you prefer I add the base path support as part of the arguments in the AppiumLocalService or it's better to create a dedicated var for it? just like we have IP and Port

Choose a reason for hiding this comment

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

I don't have any strong pref there. Do it the way which is closer/more straightforward to dotnet client users. In java and python service implementations we fetch base path value from the corresponding CLI argument. / is used by default.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've pushed some changes to this PR so we will support base-path, but I still need to work on the ipv4 status issue.


/// <summary>
/// Event that can be used to capture the output of the service
Expand Down Expand Up @@ -131,7 +133,7 @@ private void DestroyProcess()
{
Service.Kill();
}
catch (Exception ignored)
catch
{
}
finally
Expand Down Expand Up @@ -175,26 +177,80 @@ public bool IsRunning
}
}

private bool Ping(TimeSpan span)
private string GetArgsValue(string argStr)
{
bool pinged = false;
int idx;
idx= ArgsList.IndexOf(argStr);
return ArgsList[idx + 1];
}

Uri status;
private string ParseBasePath()
{
if (ArgsList.Contains("--base-path"))
{
return GetArgsValue("--base-path");
}
else if (ArgsList.Contains("-pa"))
{
return GetArgsValue("-pa");
}
return AppiumServiceConstants.DefaultBasePath;
}

private void GenerateArgsList()
{
ArgsList = Arguments.Split(' ').ToList();
}
private Uri CreateStatusUrl()
{
Uri status;
Uri service = ServiceUrl;

if (ArgsList == null)
{
GenerateArgsList();
}
string basePath = ParseBasePath();
bool defBasePath = basePath.Equals(AppiumServiceConstants.DefaultBasePath);

if (service.IsLoopback || IP.ToString().Equals(AppiumServiceConstants.DefaultLocalIPAddress))
{
status = new Uri("http://localhost:" + Convert.ToString(Port) + "/wd/hub/status");
var tmpStatus = "http://localhost:" + Convert.ToString(Port);
if (defBasePath)
{
status = new Uri(tmpStatus + AppiumServiceConstants.StatusUrl);
}
else
{
status = new Uri(tmpStatus + basePath + AppiumServiceConstants.StatusUrl);
}
}
else
{
status = new Uri(service.ToString() + "/status");
if (defBasePath)
{
status = new Uri(service, AppiumServiceConstants.StatusUrl);
}
else
{
status = new Uri(service, basePath + AppiumServiceConstants.StatusUrl);
}
}
return status;
}

private bool Ping(TimeSpan span)
{
bool pinged = false;

Uri status;

status = CreateStatusUrl();

DateTime endTime = DateTime.Now.Add(this.InitializationTimeout);
DateTime endTime = DateTime.Now.Add(span);
while (!pinged & DateTime.Now < endTime)
{
HttpWebRequest request = (HttpWebRequest) HttpWebRequest.Create(status);
HttpWebRequest request = (HttpWebRequest)WebRequest.Create(status);
HttpWebResponse response = null;
try
{
Expand All @@ -203,7 +259,7 @@ private bool Ping(TimeSpan span)
pinged = true;
}
}
catch (Exception e)
catch
{
pinged = false;
}
Expand Down
4 changes: 3 additions & 1 deletion src/Appium.Net/Appium/Service/AppiumServiceConstants.cs
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,11 @@ public sealed class AppiumServiceConstants
internal readonly static string Bash = "/bin/bash";
internal readonly static string CmdExe = "cmd.exe";
internal readonly static string Node = "node";

internal static readonly string DefaultLocalIPAddress = "127.0.0.1";
internal static readonly int DefaultAppiumPort = 4723;
internal static readonly string StatusUrl = "/status";
internal static readonly string DefaultBasePath = "/";

internal static readonly string AppiumFolder = "appium";
internal static readonly string BinFolder = "bin";
Expand Down
12 changes: 11 additions & 1 deletion src/Appium.Net/Appium/Service/Options/GeneralOptionList.cs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ namespace OpenQA.Selenium.Appium.Service.Options
///<summary>
/// Here is the list of common Appium server arguments.
/// All flags are optional, but some are required in conjunction with certain others.
/// The full list is available here: http://appium.io/slate/en/master/?ruby#appium-server-arguments
/// The full list is available here: https://appium.io/docs/en/writing-running-appium/server-args/
/// </summary>
public sealed class GeneralOptionList : BaseOptionList
{
Expand All @@ -30,6 +30,16 @@ public sealed class GeneralOptionList : BaseOptionList
public static KeyValuePair<string, string> Shell(string value) =>
GetKeyValuePair("--shell", value);

/// <summary>
/// Initial path segment where the Appium/WebDriver API will be hosted. Every endpoint will be behind this segment. <br/>
/// Default: for Appium 2.0: / ; for Appium 1: /wd/hub <br/>
/// Example: --base-path /my/path/prefix
/// </summary>
/// <param name="value"></param>
/// <returns></returns>
public static KeyValuePair<string, string> BasePath(string value) =>
GetKeyValuePair("--base-path", value);

///<summary>
/// callback IP Address (default: same as address) <br/>
/// Sample <br/>
Expand Down
8 changes: 4 additions & 4 deletions test/integration/Properties/Resources.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

18 changes: 18 additions & 0 deletions test/integration/ServerTests/AppiumLocalServerLaunchingTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,24 @@ public void CheckAbilityToStartServiceUsingFlags()
}
}

[Test]
public void CheckAbilityToStartServiceUsingBasePathFlag()
{
AppiumLocalService service = null;
var args = new OptionCollector().AddArguments(GeneralOptionList.BasePath("/wd/hub"));
try
{
service = new AppiumServiceBuilder().WithArguments(args).Build();
service.Start();
Assert.IsTrue(service.IsRunning);
}
finally
{
service?.Dispose();
}
}


[Test]
public void CheckAbilityToStartServiceUsingCapabilities()
{
Expand Down