[dotnet] add asynchronous methods to Navigation class

[titusfortner]

User description

Description

• This is a stripped down version of [dotnet] allow asynchronous command execution #13952 to focus on the bare minimum needed to get to the next step and compare BiDi implementations;
• Execute() on CommandExecutor classes is already async, but has been made blocking; this PR now creates ExecuteAsync() to bubble up the await to WebDriver
• Execute() methods remain blocking but now call ExecuteAsync()
• Async methods implemented for all Navigation methods with existing sync methods calling the Async methods in a blocking fashion to maintain backwards compatibility
• Note that this adds ConfigureAwait(false) inside all async lambdas executed in Task.Run() that include GetAwaiter().GetResult() at the end

Motivation and Context

This is an example of a partial implementation of #14067
We explicitly want to maintain both Sync and Async implementations for the time being. (possibly until Selenium 6)

Questions

1. Is this the public facing API we want to expose to users? (I'm pretty sure the answer is yes, but I want to get an affirmative before starting next steps).

Follow on

1. Add Async methods on all other classes
2. Create helper classes to improve the maintainability of the implementation class as desired (I'll let the .NET experts figure this one out)
3. AI thinks we shouldn't wrap things in Task.Run(), but I'll leave this for .NET experts to update as desired

Consider removing the unnecessary use of Task.Run() for wrapping synchronous calls in Back(), Forward(), GoToUrl(), and Refresh() methods. Instead, directly await the asynchronous methods. This change will reduce overhead and improve performance

5. Same thing with Exception Handling
6. Same thing with the CancellationToken recommendation

attn: @YevgeniyShunevych

---

PR Type

Enhancement, Tests

---

Description

• Added asynchronous methods for navigation actions (Back, Forward, GoToUrl, Refresh) in EventFiringWebDriver, INavigation, and Navigator.
• Introduced ExecuteAsync method to ICommandExecutor, DriverServiceCommandExecutor, and HttpCommandExecutor.
• Synchronous methods now call their asynchronous counterparts to maintain backward compatibility.
• Added ConfigureAwait(false) to all async calls to prevent deadlocks.
• Added and updated tests to cover the new asynchronous methods.

---

Changes walkthrough 📝

	Relevant files
Enhancement	7 files EventFiringWebDriver.cs Add asynchronous navigation methods to EventFiringWebDriver dotnet/src/support/Events/EventFiringWebDriver.cs Added asynchronous methods for navigation actions (Back, Forward, GoToUrl, Refresh). Synchronous methods now call their asynchronous counterparts. Introduced ConfigureAwait(false) for all async calls. +62/-20  ICommandExecutor.cs Add asynchronous command execution to ICommandExecutor interface dotnet/src/webdriver/ICommandExecutor.cs Added ExecuteAsync method to ICommandExecutor interface. +9/-0      INavigation.cs Add asynchronous navigation methods to INavigation interface dotnet/src/webdriver/INavigation.cs Added asynchronous methods for navigation actions (Back, Forward, GoToUrl, Refresh). +33/-0    Navigator.cs Implement asynchronous navigation methods in Navigator class dotnet/src/webdriver/Navigator.cs Implemented asynchronous methods for navigation actions (Back, Forward, GoToUrl, Refresh). Synchronous methods now call their asynchronous counterparts. +59/-12  DriverServiceCommandExecutor.cs Add asynchronous command execution to DriverServiceCommandExecutor dotnet/src/webdriver/Remote/DriverServiceCommandExecutor.cs Added ExecuteAsync method to DriverServiceCommandExecutor. Synchronous Execute method now calls the asynchronous version. +12/-1    HttpCommandExecutor.cs Add asynchronous command execution to HttpCommandExecutor dotnet/src/webdriver/Remote/HttpCommandExecutor.cs Added ExecuteAsync method to HttpCommandExecutor. Synchronous Execute method now calls the asynchronous version. +11/-1    WebDriver.cs Add asynchronous internal command execution to WebDriver  dotnet/src/webdriver/WebDriver.cs Added InternalExecuteAsync method for internal command execution. Synchronous methods now call their asynchronous counterparts. +22/-3
Tests	2 files NavigationTest.cs Add tests for asynchronous navigation methods                        dotnet/test/common/NavigationTest.cs Added tests for asynchronous navigation methods. +70/-1    EventFiringWebDriverTest.cs Update tests for EventFiringWebDriver to use async methods dotnet/test/support/Events/EventFiringWebDriverTest.cs Updated tests to verify asynchronous navigation methods. +7/-4

---

💡 PR-Agent usage:
Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

[codiumai-pr-agent-pro]

PR Review 🔍

(Review updated until commit f50fd80)

⏱️ Estimated effort to review [1-5]	4, because the PR involves significant changes across multiple files, including the introduction of asynchronous methods alongside existing synchronous methods. Reviewing these changes requires a deep understanding of asynchronous programming in C# and the existing architecture of the Selenium WebDriver. Additionally, the changes impact core navigation functionalities which are critical, thus requiring thorough testing and validation.
🧪 Relevant tests	Yes
⚡ Possible issues	Possible Bug: The use of Task.Run() to wrap synchronous code execution in asynchronous method calls might lead to unnecessary overhead and could potentially affect the performance. This pattern is seen across multiple methods like Back(), Forward(), GoToUrl(), and Refresh() in various classes.
	Thread Safety Concern: The current implementation may not be fully thread-safe. The use of asynchronous programming requires careful consideration of thread safety, especially in a library that might be used in a multi-threaded context.
🔒 Security concerns	No

Code feedback:

---

relevant file	dotnet/src/support/Events/EventFiringWebDriver.cs
suggestion	Consider removing the unnecessary use of Task.Run() for wrapping synchronous calls in Back(), Forward(), GoToUrl(), and Refresh() methods. Instead, directly await the asynchronous methods. This change will reduce overhead and improve performance. [important]
relevant line	Task.Run(async () => await this.BackAsync().ConfigureAwait(false)).GetAwaiter().GetResult();

---

relevant file	dotnet/src/webdriver/Remote/HttpCommandExecutor.cs
suggestion	Ensure proper exception handling in ExecuteAsync() method. Currently, the method catches HttpRequestException but does not rethrow it, which could lead to swallowing exceptions that should be visible to the caller. Consider adding a throw; statement in the catch block to rethrow the caught exception. [important]
relevant line	responseInfo = await this.MakeHttpRequest(requestInfo).ConfigureAwait(false);

---

relevant file	dotnet/src/webdriver/WebDriver.cs
suggestion	Avoid using Task.Run() to call ExecuteAsync() within InternalExecuteAsync(). Since ExecuteAsync() is already asynchronous, you can directly await it without wrapping in Task.Run(), which is more efficient and clearer. [important]
relevant line	return Task.Run(() => this.ExecuteAsync(driverCommandToExecute, parameters)).GetAwaiter().GetResult();

---

relevant file	dotnet/src/webdriver/Navigator.cs
suggestion	Refactor GoToUrlAsync(Uri url) to avoid calling ToString() on the URI, which is unnecessary and can be error-prone. Instead, use the Uri object directly in the asynchronous call to improve reliability and performance. [medium]
relevant line	await this.GoToUrlAsync(url.ToString()).ConfigureAwait(false);

---

[codiumai-pr-agent-pro]

PR Code Suggestions ✨

Category	Suggestion	Score
Performance	Use async and await directly in synchronous methods to avoid unnecessary thread pool usage Instead of using Task.Run to call the asynchronous methods from the synchronous ones, consider using async and await directly in the synchronous methods. This will avoid unnecessary thread pool usage and potential deadlocks. dotnet/src/support/Events/EventFiringWebDriver.cs [848-851] -public void Back() +public async void Back() { - Task.Run(this.BackAsync).GetAwaiter().GetResult(); + await this.BackAsync().ConfigureAwait(false); } Suggestion importance[1-10]: 8 Why: Using async and await directly instead of Task.Run can help prevent unnecessary thread pool usage and potential deadlocks, which is a significant improvement in asynchronous programming practices.	8
Best practice	Add a CancellationToken parameter to the asynchronous method for better control over task execution To ensure that the asynchronous method ExecuteAsync is properly implemented, consider adding a CancellationToken parameter to allow the operation to be cancelled if needed. This is a common practice in asynchronous programming to provide more control over the task execution. dotnet/src/webdriver/ICommandExecutor.cs [50] -Task<Response> ExecuteAsync(Command commandToExecute); +Task<Response> ExecuteAsync(Command commandToExecute, CancellationToken cancellationToken = default); Suggestion importance[1-10]: 8 Why: Adding a CancellationToken parameter to asynchronous methods is a best practice in C# to allow more control over task execution and to handle cancellations effectively. This suggestion is relevant and improves the API's usability and robustness.	8
	Add exception handling in asynchronous methods to properly catch and handle exceptions Add exception handling in the asynchronous methods to ensure that any exceptions thrown during the asynchronous operations are properly caught and handled. dotnet/src/webdriver/Navigator.cs [53-56] public async Task BackAsync() { - await this.driver.InternalExecuteAsync(DriverCommand.GoBack, null).ConfigureAwait(false); + try + { + await this.driver.InternalExecuteAsync(DriverCommand.GoBack, null).ConfigureAwait(false); + } + catch (Exception ex) + { + // Handle exception + throw; + } } Suggestion importance[1-10]: 7 Why: Adding exception handling in asynchronous methods is a best practice that ensures exceptions are properly managed, enhancing the robustness of the code.	7
Maintainability	Remove redundant synchronous wrapper methods to simplify the code and improve performance Remove the redundant InternalExecute method that wraps the asynchronous method with Task.Run, as it adds unnecessary complexity and potential performance overhead. dotnet/src/webdriver/WebDriver.cs [558-561] -internal Response InternalExecute(string driverCommandToExecute, Dictionary<string, object> parameters) -{ - return Task.Run(() => this.InternalExecuteAsync(driverCommandToExecute, parameters)).GetAwaiter().GetResult(); -} +// Removed redundant InternalExecute method Suggestion importance[1-10]: 6 Why: Removing redundant synchronous wrappers around asynchronous methods simplifies the codebase and can improve performance by reducing unnecessary complexity.	6
Test coverage	Add verification for the RefreshAsync method in the test to ensure asynchronous refresh functionality is tested Add verification for the RefreshAsync method in the test to ensure that the refresh functionality is also tested asynchronously. dotnet/test/support/Events/EventFiringWebDriverTest.cs [66-69] mockNavigation.Verify(x => x.ForwardAsync(), Times.Once); +mockNavigation.Verify(x => x.RefreshAsync(), Times.Once); string normalizedActualLog = log.ToString().Replace("\r\n", "\n").Replace("\r", "\n"); Assert.AreEqual(normalizedExpectedLog, normalizedActualLog); Suggestion importance[1-10]: 5 Why: Adding verification for the RefreshAsync method in unit tests ensures that all asynchronous functionality is properly tested, which is crucial for maintaining the reliability of the asynchronous operations in the system.	5

[titusfortner]

/help

[codiumai-pr-agent-pro]

PR Agent Walkthrough 🤖

Welcome to the PR Agent, an AI-powered tool for automated pull request analysis, feedback, suggestions and more.

Here is a list of tools you can use to interact with the PR Agent:

Tool	Description	Trigger Interactively 💎
DESCRIBE	Generates PR description - title, type, summary, code walkthrough and labels	Run
REVIEW	Adjustable feedback about the PR, possible issues, security concerns, review effort and more	Run
IMPROVE	Code suggestions for improving the PR	Run
UPDATE CHANGELOG	Automatically updates the changelog	Run
ADD DOCS 💎	Generates documentation to methods/functions/classes that changed in the PR	Run
TEST 💎	Generates unit tests for a specific component, based on the PR code change	Run
IMPROVE COMPONENT 💎	Code suggestions for a specific component that changed in the PR	Run
ANALYZE 💎	Identifies code components that changed in the PR, and enables to interactively generate tests, docs, and code suggestions for each component	Run
ASK	Answering free-text questions about the PR	[*]
GENERATE CUSTOM LABELS 💎	Generates custom labels for the PR, based on specific guidelines defined by the user	[*]
CI FEEDBACK 💎	Generates feedback and analysis for a failed CI job	[*]
CUSTOM PROMPT 💎	Generates custom suggestions for improving the PR code, derived only from a specific guidelines prompt defined by the user	[*]
SIMILAR ISSUE	Automatically retrieves and presents similar issues	[*]

(1) Note that each tool be triggered automatically when a new PR is opened, or called manually by commenting on a PR.

(2) Tools marked with [*] require additional parameters to be passed. For example, to invoke the /ask tool, you need to comment on a PR: /ask "<question content>". See the relevant documentation for each tool for more details.

[codiumai-pr-agent-pro]

PR Description updated to latest commit (f50fd80)

• Copy walkthrough table to "Files Changed" Tab

[codiumai-pr-agent-pro]

Persistent review updated to latest commit f50fd80

[codiumai-pr-agent-pro]

PR Code Suggestions ✨

Category	Suggestion	Score
Possible issue	Ensure the test method runs asynchronously by marking it as async and returning a Task The ShouldAcceptInvalidUrlsUsingUrisAsync test method should be marked as async and return a Task to ensure it runs asynchronously. dotnet/test/common/NavigationTest.cs [120-124] [Test] -public void ShouldAcceptInvalidUrlsUsingUrisAsync() +public async Task ShouldAcceptInvalidUrlsUsingUrisAsync() { INavigation navigation = driver.Navigate(); Assert.That(async () => await navigation.GoToUrlAsync((Uri)null), Throws.InstanceOf<ArgumentNullException>()); } Suggestion importance[1-10]: 10 Why: The suggestion correctly identifies a critical issue where an asynchronous test method is not properly defined as async and does not return a Task. This is crucial for the correct execution of asynchronous tests.	10
	Await asynchronous calls to ensure proper execution order and error handling The ShouldGoToUrlUsingUriAsync test method should await the asynchronous calls to GoToUrlAsync to ensure proper execution order and error handling. dotnet/test/common/NavigationTest.cs [139-146] [Test] -public void ShouldGoToUrlUsingUriAsync() +public async Task ShouldGoToUrlUsingUriAsync() { var navigation = driver.Navigate(); - navigation.GoToUrlAsync(new Uri(macbethPage)); + await navigation.GoToUrlAsync(new Uri(macbethPage)); Assert.AreEqual(driver.Title, macbethTitle); - navigation.GoToUrl(new Uri(simpleTestPage)); + await navigation.GoToUrlAsync(new Uri(simpleTestPage)); Assert.AreEqual(simpleTestTitle, driver.Title); } Suggestion importance[1-10]: 10 Why: The suggestion is accurate and addresses a significant issue where asynchronous methods are called without await, potentially leading to race conditions and incorrect test behavior.	10
	Validate the URL format in GoToUrlAsync method to ensure it is a valid URL before proceeding In the GoToUrlAsync method, after checking for null, consider validating the URL format to ensure it is a valid URL before proceeding with the navigation. dotnet/src/webdriver/Navigator.cs [91-95] if (url == null) { throw new ArgumentNullException(nameof(url), "URL cannot be null."); } +if (!Uri.IsWellFormedUriString(url, UriKind.Absolute)) +{ + throw new ArgumentException("The URL provided is not valid.", nameof(url)); +} Suggestion importance[1-10]: 5 Why: The suggestion to validate the URL format is a good practice to ensure the URL is correct before processing. However, the impact of this change is moderate as it primarily adds a validation step which might not be critical depending on the broader context of the application usage.	5
Maintainability	Extract log normalization logic into a separate method for better readability and maintainability To improve readability and maintainability, consider extracting the log normalization logic into a separate method. dotnet/test/support/Events/EventFiringWebDriverTest.cs [61-69] -string normalizedExpectedLog = expectedLog.Replace("\r\n", "\n").Replace("\r", "\n"); +string NormalizeLog(string log) => log.Replace("\r\n", "\n").Replace("\r", "\n"); ... -string normalizedActualLog = log.ToString().Replace("\r\n", "\n").Replace("\r", "\n"); +string normalizedExpectedLog = NormalizeLog(expectedLog); +string normalizedActualLog = NormalizeLog(log.ToString()); Assert.AreEqual(normalizedExpectedLog, normalizedActualLog); Suggestion importance[1-10]: 8 Why: This suggestion improves code maintainability and readability by extracting repeated logic into a separate method, which is a good practice, especially when dealing with string manipulations that are used multiple times.	8
	Remove unnecessary Task.CompletedTask return statement in the asynchronous test method In the ShouldNotHaveProblemNavigatingWithNoPagesBrowsedAsync method, the Task.CompletedTask return statement is unnecessary and can be removed. dotnet/test/common/NavigationTest.cs [94-101] [Test] [NeedsFreshDriver(IsCreatedBeforeTest = true)] -public Task ShouldNotHaveProblemNavigatingWithNoPagesBrowsedAsync() +public async Task ShouldNotHaveProblemNavigatingWithNoPagesBrowsedAsync() { var navigation = driver.Navigate(); Assert.DoesNotThrowAsync(async () => await navigation.BackAsync()); Assert.DoesNotThrowAsync(async () => await navigation.ForwardAsync()); - return Task.CompletedTask; } Suggestion importance[1-10]: 7 Why: The suggestion correctly identifies an unnecessary return statement in an asynchronous test method, improving the code's clarity and maintainability. However, it's a relatively minor improvement compared to the other issues.	7
Best practice	Use async and await directly in synchronous methods instead of Task.Run for better error handling and to avoid potential deadlocks Instead of using Task.Run to call the asynchronous methods from the synchronous ones, consider using async and await directly in the synchronous methods. This will provide better error handling and avoid potential deadlocks. dotnet/src/support/Events/EventFiringWebDriver.cs [850] -Task.Run(async () => await this.BackAsync().ConfigureAwait(false)).GetAwaiter().GetResult(); +public async void Back() +{ + await this.BackAsync().ConfigureAwait(false); +} Suggestion importance[1-10]: 7 Why: The suggestion to replace Task.Run with direct async and await usage in synchronous methods is valid and improves readability and error handling. However, the suggested code change to make Back() an async void method is not recommended due to potential issues with error handling and control flow.	7
Enhancement	Add logging inside the catch blocks to capture exceptions for better debugging Add logging inside the catch blocks to capture exceptions. This will help in debugging and understanding the issues that occur during navigation. dotnet/src/support/Events/EventFiringWebDriver.cs [866] catch (Exception ex) +{ + // Log the exception + Console.WriteLine($"Exception occurred: {ex.Message}"); + throw; +} Suggestion importance[1-10]: 6 Why: Adding logging within catch blocks is a good practice for better error diagnostics and understanding operational issues. The suggestion is contextually correct and targets the new exception handling code introduced in the PR.	6

[nvborisenko]

Note that this adds ConfigureAwait(false) inside all async lambdas executed in Task.Run() that include GetAwaiter().GetResult() at the end.

I think it is unnecessary in this context. I also would suggest to introduce kind of AsyncHelper who will be responsible to "resolve" sync-over-async problem. Later on we will be able to find all places across the repo too understand what is still "bad" in the code.

[YevgeniyShunevych]

Agree with @nvborisenko. I would also keep sync-over-async implementation encapsulated in a class like this:

internal static class AsyncHelper
{
    internal static TResult RunSync<TResult>(Func<Task<TResult>> function) =>
        Task.Run(function).GetAwaiter().GetResult();

    internal static void RunSync(Func<Task> function) =>
        Task.Run(function).GetAwaiter().GetResult();
}

[titusfortner]

Abstracting out a helper class is out of scope for this PR. You can argue the best way to do that later; this is what the internet suggested I do (well, mostly) based on this particular execution.

This is the user-facing API we want?

I want to get something basic merged in before we start evaluating/comparing BiDi implementations

[nvborisenko]

If you land it now, than nobody will return to it back later. My internet is different https://devblogs.microsoft.com/dotnet/configureawait-faq/#can-i-use-task-run-to-avoid-using-configureawaitfalse:

[titusfortner]

Nice, hadn't seen that, just the general advice that in a library every await should include a ConfigureAwait(false)

[titusfortner]

than nobody will return to it back later

That's fine too. That's a good indication that it doesn't actually matter.

Really I just want to make sure that a different implementation won't require a different user-facing API, and that any changes you might want to make will work with this API.

[codiumai-pr-agent-pro]

CI Failure Feedback 🧐

(Checks updated until commit e037c9f)

Action: .NET / Browser Tests / Browser Tests

Failed stage: Run Bazel [❌]

Failed test name: ElementFindingTest-firefox

Failure summary: The action failed due to the following reasons: The protoc.exe command failed to execute successfully, causing the generation of proto_library and Descriptor Set proto_library to fail. Specific errors include: - Generating proto_library @@io_bazel_rules_closure//java/io/bazel/rules/closure:build_info_proto [for tool] failed: (Exit -1073741819): protoc.exe failed - Generating Descriptor Set proto_library @@protobuf~//:timestamp_proto [for tool] failed: (Exit -1073741819): protoc.exe failed As a result, the build did not complete successfully, leading to the failure of the ElementFindingTest-firefox test to build.

Relevant error logs: 1: ##[group]Operating System 2: Microsoft Windows Server 2022 ... 679: �[32m[1,149 / 2,113]�[0m Building Python zip: //common/devtools:pdl_to_json [for tool]; 4s local ... (4 actions, 3 running) 680: �[32m[1,154 / 2,113]�[0m Building Python zip: //common/devtools:pdl_to_json [for tool]; 5s local ... (4 actions running) 681: �[32m[1,158 / 2,113]�[0m Building Python zip: //common/devtools:pdl_to_json [for tool]; 6s local ... (4 actions running) 682: �[32m[1,159 / 2,113]�[0m Building Python zip: //common/devtools:pdl_to_json [for tool]; 9s local ... (4 actions, 3 running) 683: �[32mINFO: �[0mFrom Compiling generator [for tool]: 684: warning CS1701: Assuming assembly reference 'System.Runtime, Version=4.2.2.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a' used by 'Microsoft.Extensions.DependencyInjection' matches identity 'System.Runtime, Version=6.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a' of 'System.Runtime', you may need to supply runtime policy 685: �[32m[1,163 / 2,113]�[0m Compiling src/google/protobuf/compiler/cpp/message_field.cc [for tool]; 2s local ... (4 actions, 3 running) 686: �[32mINFO: �[0mFrom Building java/src/org/openqa/selenium/remote/libapi-class.jar (66 source files): 687: java\src\org\openqa\selenium\remote\ErrorHandler.java:46: warning: [removal] ErrorCodes in org.openqa.selenium.remote has been deprecated and marked for removal 688: private final ErrorCodes errorCodes; 689: ^ 690: java\src\org\openqa\selenium\remote\ErrorHandler.java:60: warning: [removal] ErrorCodes in org.openqa.selenium.remote has been deprecated and marked for removal 691: this.errorCodes = new ErrorCodes(); 692: ^ 693: java\src\org\openqa\selenium\remote\ErrorHandler.java:68: warning: [removal] ErrorCodes in org.openqa.selenium.remote has been deprecated and marked for removal 694: public ErrorHandler(ErrorCodes codes, boolean includeServerErrors) { 695: ^ 696: java\src\org\openqa\selenium\remote\Response.java:97: warning: [removal] ErrorCodes in org.openqa.selenium.remote has been deprecated and marked for removal 697: ErrorCodes errorCodes = new ErrorCodes(); 698: ^ 699: java\src\org\openqa\selenium\remote\Response.java:97: warning: [removal] ErrorCodes in org.openqa.selenium.remote has been deprecated and marked for removal 700: ErrorCodes errorCodes = new ErrorCodes(); 701: ^ 702: java\src\org\openqa\selenium\remote\ProtocolHandshake.java:181: warning: [removal] ErrorCodes in org.openqa.selenium.remote has been deprecated and marked for removal 703: response.setStatus(ErrorCodes.SUCCESS); 704: ^ 705: java\src\org\openqa\selenium\remote\ProtocolHandshake.java:182: warning: [removal] ErrorCodes in org.openqa.selenium.remote has been deprecated and marked for removal 706: response.setState(ErrorCodes.SUCCESS_STRING); 707: ^ 708: java\src\org\openqa\selenium\remote\W3CHandshakeResponse.java:53: warning: [removal] ErrorCodes in org.openqa.selenium.remote has been deprecated and marked for removal 709: new ErrorCodes().toStatus((String) rawError, Optional.of(tuple.getStatusCode()))); 710: ^ 711: java\src\org\openqa\selenium\remote\W3CHandshakeResponse.java:56: warning: [removal] ErrorCodes in org.openqa.selenium.remote has been deprecated and marked for removal 712: new ErrorCodes().getExceptionType((String) rawError); 713: ^ 714: java\src\org\openqa\selenium\remote\codec\AbstractHttpResponseCodec.java:44: warning: [removal] ErrorCodes in org.openqa.selenium.remote has been deprecated and marked for removal 715: private final ErrorCodes errorCodes = new ErrorCodes(); 716: ^ 717: java\src\org\openqa\selenium\remote\codec\AbstractHttpResponseCodec.java:44: warning: [removal] ErrorCodes in org.openqa.selenium.remote has been deprecated and marked for removal 718: private final ErrorCodes errorCodes = new ErrorCodes(); 719: ^ 720: java\src\org\openqa\selenium\remote\codec\AbstractHttpResponseCodec.java:55: warning: [removal] ErrorCodes in org.openqa.selenium.remote has been deprecated and marked for removal 721: int status = response.getStatus() == ErrorCodes.SUCCESS ? HTTP_OK : HTTP_INTERNAL_ERROR; 722: ^ 723: java\src\org\openqa\selenium\remote\codec\AbstractHttpResponseCodec.java:101: warning: [removal] ErrorCodes in org.openqa.selenium.remote has been deprecated and marked for removal 724: response.setStatus(ErrorCodes.UNKNOWN_COMMAND); 725: ^ 726: java\src\org\openqa\selenium\remote\codec\AbstractHttpResponseCodec.java:103: warning: [removal] ErrorCodes in org.openqa.selenium.remote has been deprecated and marked for removal 727: response.setStatus(ErrorCodes.UNHANDLED_ERROR); 728: ^ 729: java\src\org\openqa\selenium\remote\codec\AbstractHttpResponseCodec.java:124: warning: [removal] ErrorCodes in org.openqa.selenium.remote has been deprecated and marked for removal 730: response.setStatus(ErrorCodes.SUCCESS); 731: ^ 732: java\src\org\openqa\selenium\remote\codec\AbstractHttpResponseCodec.java:125: warning: [removal] ErrorCodes in org.openqa.selenium.remote has been deprecated and marked for removal 733: response.setState(errorCodes.toState(ErrorCodes.SUCCESS)); 734: ^ 735: java\src\org\openqa\selenium\remote\codec\AbstractHttpResponseCodec.java:131: warning: [removal] ErrorCodes in org.openqa.selenium.remote has been deprecated and marked for removal 736: response.setState(errorCodes.toState(ErrorCodes.SUCCESS)); 737: ^ 738: java\src\org\openqa\selenium\remote\codec\w3c\W3CHttpResponseCodec.java:70: warning: [removal] ErrorCodes in org.openqa.selenium.remote has been deprecated and marked for removal 739: private final ErrorCodes errorCodes = new ErrorCodes(); 740: ^ 741: java\src\org\openqa\selenium\remote\codec\w3c\W3CHttpResponseCodec.java:70: warning: [removal] ErrorCodes in org.openqa.selenium.remote has been deprecated and marked for removal 742: private final ErrorCodes errorCodes = new ErrorCodes(); 743: ^ 744: java\src\org\openqa\selenium\remote\codec\w3c\W3CHttpResponseCodec.java:93: warning: [removal] ErrorCodes in org.openqa.selenium.remote has been deprecated and marked for removal 745: response.setStatus(ErrorCodes.UNKNOWN_COMMAND); 746: ^ 747: java\src\org\openqa\selenium\remote\codec\w3c\W3CHttpResponseCodec.java:98: warning: [removal] ErrorCodes in org.openqa.selenium.remote has been deprecated and marked for removal 748: response.setStatus(ErrorCodes.UNHANDLED_ERROR); 749: ^ 750: java\src\org\openqa\selenium\remote\codec\w3c\W3CHttpResponseCodec.java:145: warning: [removal] ErrorCodes in org.openqa.selenium.remote has been deprecated and marked for removal 751: response.setStatus(ErrorCodes.SUCCESS); ... 838: �[32m[1,302 / 2,113]�[0m Generating v124 DevTools Protocol bindings for .NET; 6s local ... (4 actions running) 839: �[32m[1,304 / 2,113]�[0m Generating v124 DevTools Protocol bindings for .NET; 7s local ... (4 actions running) 840: �[32m[1,305 / 2,113]�[0m Generating v124 DevTools Protocol bindings for .NET; 10s local ... (4 actions, 3 running) 841: �[32m[1,307 / 2,113]�[0m Generating v124 DevTools Protocol bindings for .NET; 12s local ... (4 actions running) 842: �[32m[1,308 / 2,113]�[0m Generating v124 DevTools Protocol bindings for .NET; 13s local ... (4 actions, 3 running) 843: �[32m[1,311 / 2,113]�[0m Generating v124 DevTools Protocol bindings for .NET; 14s local ... (3 actions, 2 running) 844: �[32m[1,311 / 2,113]�[0m Generating v124 DevTools Protocol bindings for .NET; 15s local ... (3 actions running) 845: �[32m[1,313 / 2,113]�[0m Calculating 712 JavaScript deps to javascript/atoms/deps.js; 1s local ... (4 actions running) 846: �[31m�[1mERROR: �[0mD:/_bazel/external/io_bazel_rules_closure/java/io/bazel/rules/closure/BUILD:64:14: Generating proto_library @@io_bazel_rules_closure//java/io/bazel/rules/closure:build_info_proto [for tool] failed: (Exit -1073741819): protoc.exe failed: error executing GenProto command (from target @@io_bazel_rules_closure//java/io/bazel/rules/closure:build_info_proto) 847: cd /d D:/_bazel/execroot/_main 848: SET PATH=C:\Program Files\Git\bin;C:\Program Files\Git\usr\bin;C:\Windows;C:\Windows\System32;C:\Windows\System32\WindowsPowerShell\v1.0 849: bazel-out\x64_windows-opt-exec-ST-4eedf4a3b688\bin\external\protobuf~\protoc.exe --java_out=bazel-out/x64_windows-opt-exec-ST-4eedf4a3b688/bin/external/io_bazel_rules_closure/java/io/bazel/rules/closure/build_info_proto-speed-src.jar -Iexternal/io_bazel_rules_closure -I. external/io_bazel_rules_closure/java/io/bazel/rules/closure/build_info.proto 850: # Configuration: c4c5b213685058484698870969db21c43c1827cd924f94ca91a1e5ef606ef2ca 851: # Execution platform: @@local_config_platform//:host 852: �[31m�[1mERROR: �[0mD:/_bazel/external/protobuf~/BUILD.bazel:316:14: Generating Descriptor Set proto_library @@protobuf~//:timestamp_proto [for tool] failed: (Exit -1073741819): protoc.exe failed: error executing GenProtoDescriptorSet command (from target @@protobuf~//:timestamp_proto) 853: cd /d D:/_bazel/execroot/_main 854: SET PATH=C:\Program Files\Git\bin;C:\Program Files\Git\usr\bin;C:\Windows;C:\Windows\System32;C:\Windows\System32\WindowsPowerShell\v1.0 855: bazel-out\x64_windows-opt-exec-ST-4eedf4a3b688\bin\external\protobuf~\protoc.exe --direct_dependencies google/protobuf/timestamp.proto --direct_dependencies_violation_msg=%s is imported, but @@protobuf~//:timestamp_proto doesn't directly depend on a proto_library that 'srcs' it. --descriptor_set_out=bazel-out/x64_windows-opt-exec-ST-4eedf4a3b688/bin/external/protobuf~/timestamp_proto-descriptor-set.proto.bin -Ibazel-out/x64_windows-opt-exec-ST-4eedf4a3b688/bin/external/protobuf~/_virtual_imports/timestamp_proto -I. bazel-out/x64_windows-opt-exec-ST-4eedf4a3b688/bin/external/protobuf~/_virtual_imports/timestamp_proto/google/protobuf/timestamp.proto 856: # Configuration: c4c5b213685058484698870969db21c43c1827cd924f94ca91a1e5ef606ef2ca 857: # Execution platform: @@local_config_platform//:host 858: �[32mINFO: �[0mElapsed time: 308.251s, Critical Path: 56.97s 859: �[32mINFO: �[0m1318 processes: 827 internal, 448 local, 43 worker. 860: �[31m�[1mERROR: �[0mBuild did NOT complete successfully 861: //dotnet/test/common:ElementFindingTest-chrome �[0m�[31m�[1mNO STATUS�[0m 862: //dotnet/test/common:ElementFindingTest-firefox �[0m�[31m�[1mFAILED TO BUILD�[0m 863: Executed 0 out of 2 tests: �[0m�[31m�[1m1 fails to build�[0m and �[0m�[35m1 was skipped�[0m. 864: �[0m 865: ##[error]Process completed with exit code 1. 866: Post job cleanup. 867: ##[group]Save cache for external-rules_dotnet~~rules_dotnet_nuget_packages_extension~nuget.netstandard.library.v2.0.3 868: [command]"C:\Program Files\Git\usr\bin\tar.exe" --posix -cf cache.tzst --exclude cache.tzst -P -C D:/a/selenium/selenium --files-from manifest.txt --force-local --use-compress-program "zstd -T0" 869: Failed to save: Unable to reserve cache with key setup-bazel-2-win32-external-rules_dotnet~~rules_dotnet_nuget_packages_extension~nuget.netstandard.library.v2.0.3-04e76b0f7a32645b4adb2fb87265df951412574d58a3904263ab6e131831a4cd, another job may be creating this cache. More details: Cache already exists. Scope: refs/pull/14051/merge, Key: setup-bazel-2-win32-external-rules_dotnet~~rules_dotnet_nuget_packages_extension~nuget.netstandard.library.v2.0.3-04e76b0f7a32645b4adb2fb87265df951412574d58a3904263ab6e131831a4cd, Version: 64e7856ef7807061948c4d0bd31e6d574b51e201a14b538096271148e3c7707b

---

✨ CI feedback usage guide:

---

The CI feedback tool (/checks) automatically triggers when a PR has a failed check.
The tool analyzes the failed checks and provides several feedbacks:

• Failed stage
• Failed test name
• Failure summary
• Relevant error logs

In addition to being automatically triggered, the tool can also be invoked manually by commenting on a PR:

/checks "https://github.com/{repo_name}/actions/runs/{run_number}/job/{job_number}"

where {repo_name} is the name of the repository, {run_number} is the run number of the failed check, and {job_number} is the job number of the failed check.

Configuration options

• enable_auto_checks_feedback - if set to true, the tool will automatically provide feedback when a check is failed. Default is true.
• excluded_checks_list - a list of checks to exclude from the feedback, for example: ["check1", "check2"]. Default is an empty list.
• enable_help_text - if set to true, the tool will provide a help message with the feedback. Default is true.
• persistent_comment - if set to true, the tool will overwrite a previous checks comment with the new feedback. Default is true.
• final_update_message - if persistent_comment is true and updating a previous checks message, the tool will also create a new message: "Persistent checks updated to latest commit". Default is true.

See more information about the checks tool in the docs.

[titusfortner]

updated to use async delegate. Windows tests are failing due to whatever is going on with protoc.exe; Navigation tests are passing locally and on RBE.

[titusfortner]

Anyone should feel free to create helpers/abstractions for this and/or other code already in the library if it makes sense to do so.