-
-
Notifications
You must be signed in to change notification settings - Fork 8.7k
[dotnet] Start ChromeDriver asynchronously #17112
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.
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
base: trunk
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -157,6 +157,61 @@ public ChromeDriver(ChromeDriverService service, ChromeOptions options, TimeSpan | |||||||||||||||||||||||
| this.AddCustomChromeCommands(); | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| /// <summary> | ||||||||||||||||||||||||
| /// Initializes a new instance of the <see cref="ChromeDriver"/> class using the specified command executor and options. | ||||||||||||||||||||||||
| /// </summary> | ||||||||||||||||||||||||
| /// <param name="commandExecutor">The <see cref="ICommandExecutor"/> to use for executing commands.</param> | ||||||||||||||||||||||||
| /// <param name="options">The <see cref="ChromeOptions"/> to use for this driver.</param> | ||||||||||||||||||||||||
| /// <param name="autoStartSession">Whether to automatically start the session.</param> | ||||||||||||||||||||||||
| protected ChromeDriver(ICommandExecutor commandExecutor, ChromeOptions options, bool autoStartSession) | ||||||||||||||||||||||||
| : base(commandExecutor, options, autoStartSession) | ||||||||||||||||||||||||
| { | ||||||||||||||||||||||||
| if (autoStartSession) | ||||||||||||||||||||||||
| { | ||||||||||||||||||||||||
| this.AddCustomChromeCommands(); | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| /// <summary> | ||||||||||||||||||||||||
| /// Asynchronously creates and starts a new instance of the <see cref="ChromeDriver"/> class with default options. | ||||||||||||||||||||||||
| /// </summary> | ||||||||||||||||||||||||
| /// <returns>A task that represents the asynchronous operation. The task result contains the initialized <see cref="ChromeDriver"/>.</returns> | ||||||||||||||||||||||||
| public static Task<ChromeDriver> StartAsync() | ||||||||||||||||||||||||
| { | ||||||||||||||||||||||||
| return StartAsync(new ChromeOptions()); | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| /// <summary> | ||||||||||||||||||||||||
| /// Asynchronously creates and starts a new instance of the <see cref="ChromeDriver"/> class using the specified options. | ||||||||||||||||||||||||
| /// </summary> | ||||||||||||||||||||||||
| /// <param name="options">The <see cref="ChromeOptions"/> to be used with the Chrome driver.</param> | ||||||||||||||||||||||||
| /// <returns>A task that represents the asynchronous operation. The task result contains the initialized <see cref="ChromeDriver"/>.</returns> | ||||||||||||||||||||||||
| /// <exception cref="ArgumentNullException">If <paramref name="options"/> is <see langword="null"/>.</exception> | ||||||||||||||||||||||||
| public static async Task<ChromeDriver> StartAsync(ChromeOptions options) | ||||||||||||||||||||||||
| { | ||||||||||||||||||||||||
| if (options is null) | ||||||||||||||||||||||||
| { | ||||||||||||||||||||||||
| throw new ArgumentNullException(nameof(options), "Chrome options must not be null"); | ||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
I would remove the custom exception message. The default one is fine. |
||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| ChromeDriverService service = ChromeDriverService.CreateDefaultService(); | ||||||||||||||||||||||||
| ICommandExecutor executor = await GenerateDriverServiceCommandExecutorAsync(service, options, DefaultCommandTimeout).ConfigureAwait(false); | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
| ICommandExecutor executor = await GenerateDriverServiceCommandExecutorAsync(service, options, DefaultCommandTimeout).ConfigureAwait(false); | |
| ICommandExecutor executor; | |
| try | |
| { | |
| executor = await GenerateDriverServiceCommandExecutorAsync(service, options, DefaultCommandTimeout).ConfigureAwait(false); | |
| } | |
| catch | |
| { | |
| service.Dispose(); | |
| throw; | |
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new
ChromeDriver.StartAsync(...)entry point is user-facing behavior, but there are no tests validating it (successful startup path and failure/disposal behavior). Please add coverage in the existing .NET test suite (e.g., underdotnet/test/chromeordotnet/test/common) to ensureStartAsyncactually creates a usable session and that resources are cleaned up on exceptions.