Skip to content
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

Jump to bottom

refactor: Use netstandard2.0 as target framework to support wider frameworks #845

Merged
merged 1 commit into from
Oct 18, 2024
Merged

refactor: Use netstandard2.0 as target framework to support wider frameworks #845

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
37 changes: 9 additions & 28 deletions src/Appium.Net/Appium.Net.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFrameworks>net48;net6.0</TargetFrameworks>
<PackageLicenseFile>LICENSE.txt</PackageLicenseFile>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why was this removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed and added netstandard2.0

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you mean licence.txt, then it was duplicated.

<TargetFrameworks>netstandard2.0</TargetFrameworks>
<RootNamespace>OpenQA.Selenium</RootNamespace>
<LangVersion>latest</LangVersion>
<Company>Appium Commiters</Company>
<Product>Dotnet-Client</Product>
<PackageRequireLicenseAcceptance>false</PackageRequireLicenseAcceptance>
<GeneratePackageOnBuild>true</GeneratePackageOnBuild>
<Copyright>Copyright © 2023</Copyright>
<PackageProjectUrl>https://github.com/appium/dotnet-client</PackageProjectUrl>
<PackageProjectUrl>https://appium.io</PackageProjectUrl>
<RepositoryUrl>https://github.com/appium/dotnet-client</RepositoryUrl>
<RepositoryType>git</RepositoryType>
<IncludeSymbols>true</IncludeSymbols>
Expand All @@ -19,40 +19,25 @@
<PackageIcon>appium-icon.png</PackageIcon>
<Description>Selenium Webdriver extension for Appium.</Description>
<PackageTags>Appium Webdriver device automation</PackageTags>
</PropertyGroup>
<PropertyGroup>
<PackageLicenseFile>LICENSE.txt</PackageLicenseFile>
</PropertyGroup>
<PropertyGroup>
<PackageReadmeFile>README.md</PackageReadmeFile>
</PropertyGroup>
<PropertyGroup>
<GenerateDocumentationFile>true</GenerateDocumentationFile>
</PropertyGroup>
<PropertyGroup>
<LangVersion>latest</LangVersion>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Shouldn't be we keep it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed here, added upper.

</PropertyGroup>

<PropertyGroup Condition="'$(Configuration)|$(TargetFramework)|$(Platform)'=='Debug|netstandard2.0|AnyCPU'">
<NoWarn>1701;1702;1591</NoWarn>
</PropertyGroup>

<PropertyGroup Condition="'$(Configuration)|$(TargetFramework)|$(Platform)'=='Release|netstandard2.0|AnyCPU'">
<NoWarn>1701;1702;1591</NoWarn>
</PropertyGroup>
<PropertyGroup Condition="'$(Configuration)|$(TargetFramework)|$(Platform)'=='Debug|net48|AnyCPU'">
<NoWarn>1701;1702;1591</NoWarn>
</PropertyGroup>
<PropertyGroup Condition="'$(Configuration)|$(TargetFramework)|$(Platform)'=='Release|net48|AnyCPU'">
<NoWarn>1701;1702;1591</NoWarn>
</PropertyGroup>

<ItemGroup>
<None Include="LICENSE.txt" Pack="true" PackagePath="" />
</ItemGroup>
<ItemGroup>
<None Include="appium-icon.png" Pack="true" PackagePath="" />
<None Include="..\..\README.md" Pack="true" PackagePath="" />
</ItemGroup>
<ItemGroup>
<None Include="..\..\README.md" Pack="true" PackagePath="" />
</ItemGroup>

<ItemGroup>
<PackageReference Include="Microsoft.SourceLink.GitHub" Version="8.0.0">
<PrivateAssets>all</PrivateAssets>
Expand All @@ -62,8 +47,4 @@
<PackageReference Include="Selenium.WebDriver" Version="4.24.0" />
<PackageReference Include="System.Drawing.Common" Version="8.0.8" />
</ItemGroup>
<ItemGroup>
<Folder Include="Properties\" />
<Folder Include="Appium\Mac\" />
</ItemGroup>
</Project>
</Project>
50 changes: 5 additions & 45 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 @@ -20,9 +20,7 @@
using System.IO;
using System.Linq;
using System.Net;
#if NET
using System.Net.Http;
#endif
using System.Runtime.CompilerServices;
using System.Threading.Tasks;

Expand All @@ -39,9 +37,7 @@ public class AppiumLocalService : ICommandServer
private readonly int Port;
private readonly TimeSpan InitializationTimeout;
private readonly IDictionary<string, string> EnvironmentForProcess;
#if !NET48
private readonly HttpClient SharedHttpClient;
#endif
private Process Service;
private List<string> ArgsList;

Expand All @@ -65,24 +61,14 @@ internal AppiumLocalService(
Port = port;
InitializationTimeout = initializationTimeout;
EnvironmentForProcess = environmentForProcess;
#if !NET48
SharedHttpClient = CreateHttpClientInstance;
#endif
SharedHttpClient = CreateHttpClientInstance();
}

#if !NET48
private HttpClient CreateHttpClientInstance
Comment on lines -73 to -74
Copy link
Collaborator

Choose a reason for hiding this comment

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

@nvborisenko have you run some tests against .NET framework 4.7+?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you know any known issues?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Only .net framework 4.7 is under interest? netstandard2.0 is supported by .net framework 4.6.1

Copy link
Collaborator

Choose a reason for hiding this comment

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

You said you were trying to run with. Net 4.7, so this is why I asked.
:)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you know any known issues?

Should be the same issues I had for 4.8 😁

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 am not from Selenium team, I am contributing as personal.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As for Selenium:

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

And you will be surprised if you look at dependencies of Playwright.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok. Let's go ahead with this change.
Fingers crossed :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Selenium is already felt it.

private static HttpClient CreateHttpClientInstance()
{
get
{
SocketsHttpHandler handler = new SocketsHttpHandler
{
PooledConnectionLifetime = TimeSpan.FromMinutes(2)
};
return new HttpClient(handler);
}
return new HttpClient() { Timeout = TimeSpan.FromMinutes(2) };
}
#endif

/// <summary>
/// The base URL for the managed appium server.
/// </summary>
Expand Down Expand Up @@ -175,9 +161,7 @@ private void DestroyProcess()
finally
{
Service?.Close();
#if !NET48
SharedHttpClient.Dispose();
#endif
}
}

Expand Down Expand Up @@ -296,20 +280,12 @@ private async Task<bool> PingAsync(TimeSpan span)
{
try
{
#if NET48
HttpWebResponse response = await GetHttpResponseAsync(status).ConfigureAwait(false);
if (response.StatusCode == HttpStatusCode.OK)
{
return true;
}
#elif NET
HttpResponseMessage response = await GetHttpResponseAsync(status).ConfigureAwait(false);
using HttpResponseMessage response = await SharedHttpClient.GetAsync(status).ConfigureAwait(false);

if (response.IsSuccessStatusCode)
{
return true;
}
#endif
}
catch
{
Expand All @@ -318,21 +294,5 @@ private async Task<bool> PingAsync(TimeSpan span)
}
return pinged;
}
#if NET48
private async Task<HttpWebResponse> GetHttpResponseAsync(Uri status)
{
return await Task.Run(() =>
{
HttpWebRequest request = (HttpWebRequest)WebRequest.Create(status);
return (HttpWebResponse)request.GetResponse();
}).ConfigureAwait(false);
}
#else
private async Task<HttpResponseMessage> GetHttpResponseAsync(Uri status)
{
HttpResponseMessage response = await SharedHttpClient.GetAsync(status).ConfigureAwait(false);
return response;
}
#endif
}
}