[Java] Migrate the String.Format to String.Format(Locale.US) to handle different langauges for Edge, Chrome and Firefox

[MustafaAgamy]

User description

Description

• This is a contributions to fix:
  [🐛 Bug]: Getting SessionNotCreatedException when the Date of the Windows is in Arabic language format #13930

*Changes Done:

• Migrate the String.Format to Formatter.Format in :
• ChromeDriverSerivce.java
• EdgeDriverService.java
• RemoteWebDriver.java

Motivation and Context

*Issue explaination:

• Having System Date & Language in Arabic caused with driver setup. Using String.Format for ports and some other lines caused them to be localized in the system language and in arabic case it caused the numbers and some other character to be localized in arabic, e.g. the port. And that caused an issues when appending the port to the URL in order to setup the browser

*Fix explaination:

• Now it's utilizing the Formatter.Format and forcing the localization to be in (English.US) ny passing the corresponding Locale in the Formatter Constructor

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

Bug fix

---

Description

• Updated ChromeDriverService, EdgeDriverService, and RemoteWebDriver to use Formatter.format with Locale.US to ensure consistent formatting across different system locales.
• This change addresses issues where system locale settings (like Arabic) affected the formatting of numbers and text in logs and port settings, potentially causing errors in browser setup.
• The use of Locale.US ensures that numbers and text are formatted in a standard way, avoiding issues related to localization.

---

Changes walkthrough 📝

Relevant files

Bug fix

ChromeDriverService.java Use Formatter with Locale.US in ChromeDriverService            java/src/org/openqa/selenium/chrome/ChromeDriverService.java Replaced String.format with Formatter.format using Locale.US for consistent number formatting. Ensured log file paths and log levels are formatted without localization issues. +6/-4      EdgeDriverService.java Standardize EdgeDriverService Formatting with Locale.US    java/src/org/openqa/selenium/edge/EdgeDriverService.java Switched from String.format to Formatter.format with Locale.US to avoid localization in log and port settings. Adjusted logging and port settings to use non-localized number formats. +6/-4      RemoteWebDriver.java Implement Formatter in RemoteWebDriver for Uniform Output java/src/org/openqa/selenium/remote/RemoteWebDriver.java Updated string formatting to use Formatter with Locale.US for consistent command and log messages. Modified session handling and logging to prevent localization issues. +8/-6

---

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

[CLAassistant]

All committers have signed the CLA.

[codiumai-pr-agent-pro]

PR Description updated to latest commit (1c526d0)

• Copy walkthrough table to "Files Changed" Tab

[codiumai-pr-agent-pro]

PR Review 🔍

⏱️ Estimated effort to review [1-5]	2, because the changes are straightforward and localized to string formatting across a few files. The logic is simple, replacing String.format with Formatter to handle locale-specific formatting issues.
🧪 Relevant tests	No
⚡ Possible issues	Resource Leak: Each new Formatter instance should be properly closed to avoid resource leaks. Consider using try-with-resources or ensuring each formatter is closed after use.
	Consistency Issue: In some places, Locale.US is used, and in one place, Locale.ENGLISH is used. It's important to ensure consistency across the application unless intentionally varied.
🔒 Security concerns	No

[codiumai-pr-agent-pro]

PR Code Suggestions ✨

Category	Suggestion	Score
Resource management	Ensure resource management by properly closing Formatter objects Ensure that the Formatter objects are properly closed to prevent resource leaks. Use try-with-resources to manage the Formatter lifecycle. java/src/org/openqa/selenium/chrome/ChromeDriverService.java [287] -args.add(String.valueOf(new Formatter(Locale.US).format("--port=%d", getPort()))); +try (Formatter formatter = new Formatter(Locale.US)) { + args.add(String.valueOf(formatter.format("--port=%d", getPort()))); +} Suggestion importance[1-10]: 9 Why: The suggestion correctly identifies a resource management issue with the Formatter class, which is crucial to prevent potential resource leaks in Java.	9
Performance	Optimize resource usage by using a single Formatter instance Use a single Formatter instance within the method to optimize resource usage and improve performance. java/src/org/openqa/selenium/edge/EdgeDriverService.java [281] -args.add(String.valueOf(new Formatter(Locale.US).format("--port=%d", getPort()))); +try (Formatter formatter = new Formatter(Locale.US)) { + args.add(String.valueOf(formatter.format("--port=%d", getPort()))); + if (getLogFile() != null) { + args.add(String.valueOf(formatter.format("--log-path=%s", getLogFile().getAbsolutePath()))); + } + if (logLevel != null) { + args.add(String.valueOf(formatter.format("--log-level=%s", logLevel.toString().toUpperCase()))); + } + if (allowedListIps != null) { + args.add(String.valueOf(formatter.format("--allowed-ips=%s", allowedListIps))); + } +} Suggestion importance[1-10]: 8 Why: This suggestion is valid and would improve performance by reducing the number of Formatter instances created, which is a good practice in resource management.	8
Code simplification	Simplify code and reduce object creation by using direct String conversion Replace String.valueOf with direct String conversion for cleaner code and to avoid unnecessary object creation. java/src/org/openqa/selenium/remote/RemoteWebDriver.java [731] -copyToLog.setValue(String.valueOf(new Formatter(Locale.US).format("*%s response suppressed*", commandName))); +copyToLog.setValue(new Formatter(Locale.US).format("*%s response suppressed*", commandName).toString()); Suggestion importance[1-10]: 7 Why: The suggestion improves code readability and efficiency by eliminating an unnecessary method call, which is a good practice for cleaner and more efficient code.	7
Robustness	Enhance robustness by handling exceptions in toUpperCase() method Consider handling potential exceptions that might be thrown by the toUpperCase() method in a locale-sensitive manner. java/src/org/openqa/selenium/remote/RemoteWebDriver.java [298] -args.add(String.valueOf(new Formatter(Locale.US).format("--log-level=%s", logLevel.toString().toUpperCase()))); +try { + args.add(String.valueOf(new Formatter(Locale.US).format("--log-level=%s", logLevel.toString().toUpperCase(Locale.ROOT)))); +} catch (Exception e) { + // Handle exception +} Suggestion importance[1-10]: 6 Why: This suggestion adds robustness by considering locale sensitivity and exception handling, although toUpperCase() typically does not throw exceptions that need to be caught in this context, making the suggestion slightly less relevant.	6

[pujagani]

I think the changes in the PR are not necessary. I looked into this further to realize the findings below:

  public static String format(String format, Object... args) {
        return new Formatter().format(format, args).toString();
    } 

This creates a Formatter instance:

  public Formatter() {
        this(Locale.getDefault(Locale.Category.FORMAT), new StringBuilder());
    }

So setting the default using "setDefault(Locale)" should work out of the box without any changes.

[pujagani]

Before we close this PR, can you try on your end to double-check if it works as expected using "setDefault"?

[MustafaAgamy]

@pujagani
I just did this to minimize the scope of the affects

But setting the setting the Default to Locale.US should work

You can call it In the same create Args methods once

I will change the fixes to that and give it a try?

[MustafaAgamy]

@pujagani

Done can you please check and let me know ? (it worked local)
Also maybe another pipeline run to make sure everything is working as expected?

[MustafaAgamy]

@pujagani I revisited the formatting issue

Should be good now