break: ExecuteAsync() under the hood implementation after bumping to Selenium 4.23

[kelmelzer]

List of changes

This bumps to Selenium 4.23 and fixes the build errors in AppiumDriver and AppiumCommandExecutor to flow through the new ExecuteAsync() methods in Selenium 4.23+. Fixes #798

Types of changes

What types of changes are you proposing/introducing to the .NET client?

• [x] Bugfix (non-breaking change which fixes an issue)
• [ ] New feature (non-breaking change which adds functionality)
• [ ] New feature (non-breaking change which adds value to the project)
• [ ] Breaking change (fix or feature that would cause existing functionality not to work as expected)

[KazuCocoa]

Will this require selenium 3.22+? (Just I don't have much knowledge about deps handling in dotnet lock files, or src/Appium.Net/Appium.Net.csproj handles it)

[Dor-bl]

Will this require selenium 3.22+? (Just I don't have much knowledge about deps handling in dotnet lock files, or src/Appium.Net/Appium.Net.csproj handles it)

@KazuCocoa I assume you meant 4.22+. Indeed we need to make sure it won't break backward compatibility

[KazuCocoa]

Yea, 4.22. If we need to bump the required version, that will also be fine with at least bumping the minor version such as 5.1.0 dotnet driver and changelog etc. A matrix such as https://github.com/appium/python-client?tab=readme-ov-file#compatibility-matrix would help.

[Dor-bl]

Yea, 4.22. If we need to bump the required version, that will also be fine with at least bumping the minor version such as 5.1.0 dotnet driver and changelog etc. A matrix such as https://github.com/appium/python-client?tab=readme-ov-file#compatibility-matrix would help.

At first look, this will be a breaking change since we inherit from WebDriver the ExecuteAsync method which doesn't exist on previous versions. A matrix of compatibility is a good idea to have regardless.

[Dor-bl]

@nvborisenko your feedback will be appreciated as well :)

[Dor-bl]

@kelmelzer , the selenium team has released a patch (4.23.0) can you please check if we still need adjustments from our side?

[kelmelzer]

Will do when I get a chance! I have been putting out some other fires lately. :)

On Fri, Jul 19, 2024, 8:21 AM Dor Blayzer @.***> wrote:

@kelmelzer https://github.com/kelmelzer , the selenium team has released a patch (4.23.0) can you please check if we still need adjustments from our side?

— Reply to this email directly, view it on GitHub https://github.com/appium/dotnet-client/pull/804#issuecomment-2239022600, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACNNGZVWJTMFGM5FAGOXPD3ZNEAGNAVCNFSM6AAAAABKRQYFMGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEMZZGAZDENRQGA . You are receiving this because you were mentioned.Message ID: @.***>

[Dor-bl]

I hope no fires related to crowdstrike 😅

[kelmelzer]

I hope no fires related to crowdstrike 😅

Definitely related 👎

Anyways, Appium 5 still doesn't build with Selenium 4.23 due to ICommandExecutor's ExecuteAsync() not being implemented by AppiumCommandExecutor. Adding this simple passthrough fixes:

    public Task<Response> ExecuteAsync(Command commandToExecute) => Task.Run(() => this.Execute(commandToExecute));

[kelmelzer]

I (should have) updated my pull request with 4.23 and the small change I mentioned previously.

[Dor-bl]

I hope no fires related to crowdstrike 😅

Definitely related 👎

Anyways, Appium 5 still doesn't build with Selenium 4.23 due to ICommandExecutor's ExecuteAsync() not being implemented by AppiumCommandExecutor. Adding this simple passthrough fixes:

    public Task<Response> ExecuteAsync(Command commandToExecute) => Task.Run(() => this.Execute(commandToExecute));

I see the build still fails. Did you checked the changes locally?

[kelmelzer]

I hope no fires related to crowdstrike 😅

Definitely related 👎 Anyways, Appium 5 still doesn't build with Selenium 4.23 due to ICommandExecutor's ExecuteAsync() not being implemented by AppiumCommandExecutor. Adding this simple passthrough fixes:

    public Task<Response> ExecuteAsync(Command commandToExecute) => Task.Run(() => this.Execute(commandToExecute));

I see the build still fails. Did you checked the changes locally?

Let's see what happens with these recent commits...hopefully I got it right.

[Dor-bl]

@kelmelzer Please update PR title

[Dor-bl]

@kelmelzer, Please fix the merge conflicts.

[kelmelzer]

@kelmelzer, Please fix the merge conflicts.

Been out of pocket most of the week. I accepted your change.

[Dor-bl]

@kelmelzer, Please fix the merge conflicts.

Been out of pocket most of the week. I accepted your change.

Sorry for the confusion. I updated drawing.common yesterday. Can you please align with the main branch?

[Dor-bl]

@kelmelzer, Please fix the merge conflicts.

Been out of pocket most of the week. I accepted your change.

Sorry for the confusion. I updated drawing.common yesterday. Can you please align with the main branch?

@kelmelzer, Hope you don't mind but I resolved the conflict myself so we can speed up this PR :)

[jlipps]

Hi @kelmelzer , congrats, the Appium project wants to compensate you for this (and perhaps other) contribution(s) this month! Please reply to this comment mentioning me and sharing your OpenCollective account name, so that we can initiate payment! Or let me know if you decline to receive compensation via OpenCollective. Thank you!

[kelmelzer]

Hi @kelmelzer , congrats, the Appium project wants to compensate you for this (and perhaps other) contribution(s) this month! Please reply to this comment mentioning me and sharing your OpenCollective account name, so that we can initiate payment! Or let me know if you decline to receive compensation via OpenCollective. Thank you!

@jlipps That's wonderful. My account name is kelmelzer there as well.

[nvborisenko]

That's great!