Recovering

[UGURAKSAHIN]

User description

Thanks for contributing to Selenium!
A PR well described will help maintainers to quickly review and merge it

Before submitting your PR, please check our contributing guidelines.
Avoid large PRs, help reviewers by making them as simple and short as possible.

Motivation and Context

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist

• I have read the contributing document.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have added tests to cover my changes.
• All new and existing tests passed.

---

PR Type

Enhancement, Tests, Configuration changes

---

Description

• Improved graceful shutdown handling in ExecutorServices.

• Refactored test methods in ChromeDriverFunctionalTest and LocationContextTest.

• Increased heap size configurations in .idea files.

• Minor updates to .NET test setup file.

---

Changes walkthrough 📝

	Relevant files
Enhancement	ExecutorServices.java Enhanced graceful shutdown handling in ExecutorServices    java/src/org/openqa/selenium/concurrent/ExecutorServices.java Added DEFAULT_SHUTDOWN_TIMEOUT constant. Introduced awaitTermination and forceShutdown methods. Improved shutdown handling with better logging and separation of concerns. +18/-9
Tests	ChromeDriverFunctionalTest.java Refactored ChromeDriver functional test methods                    java/test/org/openqa/selenium/chrome/ChromeDriverFunctionalTest.java Refactored test to use helper methods for initialization and assertions. Improved readability and modularity of test code. +9/-2      LocationContextTest.java Refactored LocationContext test methods                                    java/test/org/openqa/selenium/html5/LocationContextTest.java Introduced helper method setAndRetrieveLocation for location tests. Simplified test code for latitude, longitude, and altitude. +7/-7
Miscellaneous	AssemblyTeardown.cs Minor updates to .NET AssemblyTeardown                                      dotnet/test/remote/AssemblyTeardown.cs Added System namespace import. Minor updates to test setup file. +1/-0
Configuration changes	androidDexCompiler.xml Updated Android Dex Compiler heap size                                      .idea/androidDexCompiler.xml Increased MAX_HEAP_SIZE from 4096 to 8192. +1/-1      compiler.xml Updated compiler heap size configurations                                .idea/compiler.xml Increased BUILD_PROCESS_HEAP_SIZE and MAXIMUM_HEAP_SIZE to 2048. Adjusted project compiler settings for better performance. +3/-3

---

Need help?

Type /help how to ... in the comments thread for any questions about Qodo Merge usage.

Check out the documentation for more information.

[CLAassistant]

All committers have signed the CLA.

[qodo-merge-pro]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Syntax Error The method contains a syntax error where Thread.currentThread().interrupt; is missing parentheses. It should be Thread.currentThread().interrupt();. Also, there's a missing closing brace and incorrect method name forceShutDown vs forceShutdown. Thread.currentThread().interrupt; LOG.log(WARNING, String.format("Failed to shutdown %s", name), e); return false; private static void forceShutDown(String name, ExecutorService, service){ LOG.warning(String.format("Failed to shutdown %s", name), e); service.shutdownNow(); } Missing Method The helper method setAndRetrieveLocation is referenced in multiple tests but its implementation is missing from the provided code. This method needs to be defined to make the tests work properly. Location location = setAndRetrieveLocation(locationContext, 40.714353, -74.005973, 0.056747); Variable Reference The test uses localDriver in assertions but references driver in the helper method parameter. This inconsistency could cause issues when the helper method is called. private void assertDefaultChromeCapabilities(ChromeDriver driver){ Capabilities capabilities = driver.getCapabilities(); assertThat(localDriver.manage().timeouts().getImplicitWaitTimeout()).isEqualTo(Duration.ZERO); assertThat(capabilities.getCapability("browserName")).isEqualTo("chrome");

[qodo-merge-pro]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Fix method call syntax The Thread.currentThread().interrupt is missing parentheses - it should be Thread.currentThread().interrupt() to properly call the method. Also, the exception variable is named ex but referenced as e in the logging statement. java/src/org/openqa/selenium/concurrent/ExecutorServices.java [42-45] }catch(InterruptedException ex){ - Thread.currentThread().interrupt; - LOG.log(WARNING, String.format("Failed to shutdown %s", name), e); + Thread.currentThread().interrupt(); + LOG.log(WARNING, String.format("Failed to shutdown %s", name), ex); return false; } [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 10 __ Why: This suggestion fixes two critical syntax errors that would cause compilation failures: the missing parentheses in the interrupt() method call and the mismatched exception variable name. These are serious issues that would prevent the code from working.	High
	Fix method parameter syntax The method declaration has a syntax error with an extra comma in the parameter list. Also, the variable e is undefined in this context. Remove the comma between ExecutorService and service, and remove the undefined variable e from the logging statement. java/src/org/openqa/selenium/concurrent/ExecutorServices.java [47-51] -private static void forceShutDown(String name, ExecutorService, service){ - LOG.warning(String.format("Failed to shutdown %s", name), e); +private static void forceShutdown(String name, ExecutorService service){ + LOG.warning(String.format("Failed to shutdown %s", name)); service.shutdownNow(); - } Apply this suggestion Suggestion importance[1-10]: 10 __ Why: The suggestion addresses two critical syntax errors: an invalid comma in the method parameter list and an undefined variable 'e' in the logging statement. Both issues would cause compilation failures, making this a high-impact fix.	High
	Fix inconsistent method name The method is called forceShutdown in the shutdownGracefully method but is defined as forceShutDown (with a capital D). Make the method names consistent to avoid a compilation error. java/src/org/openqa/selenium/concurrent/ExecutorServices.java [33-35] if(!awaitTermination(name, service, DEFAULT_SHUTDOWN_TIMEOUT)){ - forceShutdown(name, service); + forceShutDown(name, service); } Apply this suggestion Suggestion importance[1-10]: 9 __ Why: The method name inconsistency between the call ('forceShutdown') and the definition ('forceShutDown') would cause a compilation error. This is a critical issue that would prevent the code from running properly.	High
More

[pujagani]

We appreciate your contribution. But can you please help add the issue title and description so we understand the incoming changes. I see changes done in two languages but those seem unrelated. Please create separate PRs for separate purposes. Thank you!

[diemol]

We never got feedback from the comment above.