Skip to content

feat: Migrate AppiumLocalService to Appium 2 #529

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.

Sign up for GitHub

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 6 commits into from
Oct 26, 2022
Merged

feat: Migrate AppiumLocalService to Appium 2 #529

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
c3c69c8
Initial migration to appium2.0 [Removal of /wd/hub]
Dor-bl Oct 13, 2022
1c0d317
Removal of redundent using on Integration.Tests.Properties
Dor-bl Oct 13, 2022
4669067
Add test for StartService using basePath flag
Dor-bl Oct 17, 2022
cafba78
Work with ArgsList for cleaner code
Dor-bl Oct 22, 2022
325e640
Skip GenerateArgsList if it's not null
Dor-bl Oct 22, 2022
35b569b
Minor code fixes
Dor-bl Oct 22, 2022
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
8 changes: 4 additions & 4 deletions src/Appium.Net/Appium/Service/AppiumLocalService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,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)}");
Copy link

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

Copy link

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 @@ -184,17 +184,17 @@ private bool Ping(TimeSpan span)
Uri service = ServiceUrl;
if (service.IsLoopback || IP.ToString().Equals(AppiumServiceConstants.DefaultLocalIPAddress))
{
status = new Uri("http://localhost:" + Convert.ToString(Port) + "/wd/hub/status");
status = new Uri("http://localhost:" + Convert.ToString(Port) + "/status");
}
else
{
status = new Uri(service.ToString() + "/status");
status = new Uri(service.ToString() + "status");
}

DateTime endTime = DateTime.Now.Add(this.InitializationTimeout);
while (!pinged & DateTime.Now < endTime)
{
HttpWebRequest request = (HttpWebRequest) HttpWebRequest.Create(status);
HttpWebRequest request = (HttpWebRequest)WebRequest.Create(status);
HttpWebResponse response = null;
try
{
Expand Down
8 changes: 4 additions & 4 deletions test/integration/Properties/Resources.Designer.cs
View file
Open in desktop

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