tox formatting for python files in `format.sh`

[navin772]

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.

Description

It would be contributor friendly if we can have python formatting in ./scripts/format.sh. Currently due to lack of formatting support from rules_python, bazel targets were not configurable (Added it as a TODO)

Types of changes

• [x] 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

• [x] 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.
• [x] All new and existing tests passed.

---

PR Type

enhancement, configuration changes

---

Description

• Introduced Python formatting in the format.sh script using tox.
• Configured tox to apply isort, black, flake8, and docformatter for Python files.
• Added a TODO note for using Bazel targets when rules_python supports formatting.

---

Changes walkthrough 📝

	Relevant files
Enhancement	format.sh Add Python formatting using tox in format.sh script            scripts/format.sh Added a new section for Python formatting. Installed tox for Python linting and formatting. Configured tox to use isort, black, flake8, and docformatter. Added a TODO note for future Bazel target support. +7/-0

---

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

[qodo-code-review[bot]]

PR Reviewer Guide 🔍

(Review updated until commit https://github.com/SeleniumHQ/selenium/commit/7d084534055bb8a46243abdee5bf75ee6fb464b5)

Here are some key observations to aid the review process:

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

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Dependency Management The PR installs tox using pip, which may not be the ideal way to manage dependencies in a project using Bazel. Environment Variable Setting TOXENV as an environment variable might affect other parts of the build process or CI/CD pipeline.

[qodo-code-review[bot]]

PR Code Suggestions ✨

Latest suggestions up to 7d08453 Explore these optional code suggestions:

Category	Suggestion	Score
Best practice	Implement a more robust method for installing and running tox in an isolated environment Consider using a virtual environment or checking if tox is already installed before running 'pip install tox'. This can prevent potential conflicts with system-wide packages and ensure a clean, isolated environment for running tox. scripts/format.sh [37-39] -pip install tox +if ! command -v tox &> /dev/null; then + python -m venv .venv + source .venv/bin/activate + pip install tox +fi export TOXENV=linting tox -c py/tox.ini [ ] Apply this suggestion Suggestion importance[1-10]: 9 Why: This suggestion significantly improves the robustness of the script by ensuring that tox is installed in a virtual environment, preventing potential conflicts with system-wide packages. It also checks if tox is already installed, which is a best practice for managing dependencies.	9
Enhancement	Add error handling for the tox command to improve script robustness Add error handling to check if the tox command was successful. This can help identify and report any issues that occur during the Python formatting process. scripts/format.sh [38-39] export TOXENV=linting -tox -c py/tox.ini +if ! tox -c py/tox.ini; then + echo "Error: Python formatting failed" >&2 + exit 1 +fi [ ] Apply this suggestion Suggestion importance[1-10]: 8 Why: Adding error handling for the tox command enhances the script's robustness by providing feedback if the command fails. This is a valuable improvement for debugging and maintaining the script.	8

💡 Need additional feedback ? start a PR chat

---

Previous suggestions

Suggestions up to commit d1215ea

Category	Suggestion	Score
Error handling	Add error handling for the formatting command Add error handling to check if the tox command was successful. This will help catch and report any issues during the formatting process. scripts/format.sh [38-39] TOXENV=linting -tox -c py/tox.ini +if ! tox -c py/tox.ini; then + echo "Error: Python formatting failed" >&2 + exit 1 +fi Suggestion importance[1-10]: 9 Why: Adding error handling for the tox command is crucial for catching and reporting issues during the formatting process, improving the robustness and reliability of the script.	9
Best practice	Use a virtual environment for managing Python dependencies Consider using a virtual environment or a requirements file to manage dependencies instead of directly installing tox with pip. This approach is more reproducible and avoids potential conflicts with system-wide packages. scripts/format.sh [37] -pip install tox +python -m venv venv +source venv/bin/activate +pip install -r requirements.txt # Include tox in requirements.txt Suggestion importance[1-10]: 8 Why: Using a virtual environment or a requirements file is a best practice for managing dependencies, ensuring reproducibility, and avoiding conflicts with system-wide packages. This suggestion significantly improves the maintainability and reliability of the script.	8
Optimization	Check if the required tool is already installed before installation Consider adding a check to verify if tox is already installed before attempting to install it. This can save time and avoid unnecessary installations. scripts/format.sh [37] -pip install tox +if ! command -v tox &> /dev/null; then + pip install tox +fi Suggestion importance[1-10]: 7 Why: Checking if tox is already installed before attempting installation is a useful optimization that can save time and resources, although it is not as critical as the other suggestions.	7

[AutomatedTester]

This seems to not work on the CI. Can you look into it?

[navin772]

I will take a look, from the CI logs it seems like selenium py package failed to build.

For now, I will convert this to draft.

[qodo-code-review[bot]]

Persistent review updated to latest commit https://github.com/SeleniumHQ/selenium/commit/7d084534055bb8a46243abdee5bf75ee6fb464b5

[navin772]

@AutomatedTester I have updated the PR, can you please trigger the CI?

[navin772]

The RBE test failures are unrelated to the changes.