Skip to content

Conversation

@Delta456
Copy link
Member

@Delta456 Delta456 commented Nov 17, 2025

User description

🔗 Related Issues

💥 What does this PR do?

Uses consistent quoting.

🔧 Implementation Notes

💡 Additional Considerations

🔄 Types of changes

  • Cleanup (formatting, renaming)

PR Type

Enhancement


Description

  • Convert single-quoted docstrings to double-quoted format

  • Standardize docstring style across entire generate.py module

  • Improve code consistency with Python documentation conventions


Diagram Walkthrough

File Walkthrough

Relevant files
Formatting
generate.py
Convert all docstrings from single to double quotes           

py/generate.py

  • Replaced all single-quoted docstrings (''') with double-quoted
    docstrings (""") across 50+ functions and classes
  • Affected docstrings include module-level functions, class methods,
    properties, and dataclass definitions
  • Changes applied consistently to functions like indent(),
    escape_backticks(), docstring(), and all CDP-related classes (CdpType,
    CdpCommand, CdpEvent, CdpDomain, etc.)
  • No functional changes to code logic or behavior, purely stylistic
    formatting
+68/-68 

@selenium-ci selenium-ci added the C-py Python Bindings label Nov 17, 2025
@qodo-merge-pro
Copy link
Contributor

qodo-merge-pro bot commented Nov 17, 2025

PR Compliance Guide 🔍

(Compliance updated until commit 5eaf602)

Below is a summary of compliance checks for this PR:

Security Compliance
Unvalidated log level

Description: The logging level is controlled via the LOG_LEVEL environment variable without validation,
allowing users to elevate verbosity and potentially leak sensitive info to logs when
running the generator.
generate.py [45-47]

Referred Code
log_level = getattr(logging, os.environ.get("LOG_LEVEL", "warning").upper())
logging.basicConfig(level=log_level)
logger = logging.getLogger("generate")
Ticket Compliance
🟡
🎫 #5678
🔴 Diagnose and fix ChromeDriver "Error: ConnectFailure (Connection refused)" occurring on
subsequent driver instantiations on Ubuntu 16.04 with Chrome 65 and Chromedriver 2.35
(Selenium 3.9.0).
Provide steps or code changes that prevent the connection refusal after the first
ChromeDriver instance.
Ensure reliability across multiple consecutive ChromeDriver instantiations.
🟡
🎫 #1234
🔴 Restore or ensure that click() triggers JavaScript in link hrefs as it did in 2.47.1,
which regressed in 2.48.x on Firefox 42.
Provide a fix or behavior change to WebDriver so that click() executes href JavaScript.
Validate behavior specifically on Firefox per the ticket label.
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status:
No Audit Logs: The new code adds logging configuration and uses logger.info/debug, but there is no
evidence of audit logging for critical security actions in this diff, which may be outside
the scope of this generator script.

Referred Code
log_level = getattr(logging, os.environ.get("LOG_LEVEL", "warning").upper())
logging.basicConfig(level=log_level)
logger = logging.getLogger("generate")

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Missing Validation: Parsing and file operations lack added try/except or validation for schema shape in the
new code changes, though this PR focuses on quoting/style and may rely on existing
behavior.

Referred Code
with open(json_path, encoding="utf-8") as json_file:
    schema = json.load(json_file)
version = schema["version"]
assert (version["major"], version["minor"]) == ("1", "3")
current_version = f"{version['major']}.{version['minor']}"
domains = []
for domain in schema["domains"]:
    domains.append(CdpDomain.from_json(domain))
return domains

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status:
Schema Assumptions: The generator assumes schema keys like "version" and "domains" without
added validation in the new code; while unchanged logic, lack of explicit validation may
pose risks depending on input source.

Referred Code
version = schema["version"]
assert (version["major"], version["minor"]) == ("1", "3")
current_version = f"{version['major']}.{version['minor']}"
domains = []
for domain in schema["domains"]:
    domains.append(CdpDomain.from_json(domain))
return domains

Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

Previous compliance checks

Compliance check up to commit f9a2e17
Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🟡
🎫 #5678
🔴 Diagnose and resolve "Error: ConnectFailure (Connection refused)" when instantiating
ChromeDriver on Ubuntu 16.04.4 with Chrome 65 / ChromeDriver 2.35 using Selenium 3.9.0.
Ensure subsequent ChromeDriver instances do not produce connection refused errors.
Provide guidance or code changes that address repeated instantiation reliability.
🟡
🎫 #1234
🔴 Make WebDriver click() trigger JavaScript in a link's href consistently (regression from
2.47.1 to 2.48.x) in Firefox 42.
Provide test coverage ensuring href javascript: links fire alerts/JS handlers on click.
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status:
No logging added: The PR only updates docstrings and does not introduce or modify any logging for critical
actions, but the changes appear purely stylistic and may not require audit trail updates.

Referred Code
def parse_json_event(json: T_JSON_DICT) -> typing.Any:
    ''' Parse a JSON dictionary into a CDP event. '''
    return _event_parsers[json['method']].from_json(json['params'])
"""


def indent(s, n):
    """ A shortcut for ``textwrap.indent`` that always uses spaces. """
    return tw_indent(s, n * ' ')


BACKTICK_RE = re.compile(r'`([^`]+)`(\w+)?')


def escape_backticks(docstr):
    """
    Escape backticks in a docstring by doubling them up.
    This is a little tricky because RST requires a non-letter character after
    the closing backticks, but some CDPs docs have things like "`AxNodeId`s".
    If we double the backticks in that string, then it won't be valid RST. The
    fix is to insert an apostrophe if an "s" trails the backticks.


 ... (clipped 883 lines)

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
No error handling changes: The PR introduces only docstring style changes without adding or altering exception
handling or edge-case logic, which may be acceptable for a non-functional change.

Referred Code
def parse_json_event(json: T_JSON_DICT) -> typing.Any:
    ''' Parse a JSON dictionary into a CDP event. '''
    return _event_parsers[json['method']].from_json(json['params'])
"""


def indent(s, n):
    """ A shortcut for ``textwrap.indent`` that always uses spaces. """
    return tw_indent(s, n * ' ')


BACKTICK_RE = re.compile(r'`([^`]+)`(\w+)?')


def escape_backticks(docstr):
    """
    Escape backticks in a docstring by doubling them up.
    This is a little tricky because RST requires a non-letter character after
    the closing backticks, but some CDPs docs have things like "`AxNodeId`s".
    If we double the backticks in that string, then it won't be valid RST. The
    fix is to insert an apostrophe if an "s" trails the backticks.


 ... (clipped 883 lines)

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status:
No validation changes: The PR only standardizes docstrings and does not modify input validation or data handling
paths, which is likely fine given no functional changes but cannot be fully assessed from
the diff.

Referred Code
def parse_json_event(json: T_JSON_DICT) -> typing.Any:
    ''' Parse a JSON dictionary into a CDP event. '''
    return _event_parsers[json['method']].from_json(json['params'])
"""


def indent(s, n):
    """ A shortcut for ``textwrap.indent`` that always uses spaces. """
    return tw_indent(s, n * ' ')


BACKTICK_RE = re.compile(r'`([^`]+)`(\w+)?')


def escape_backticks(docstr):
    """
    Escape backticks in a docstring by doubling them up.
    This is a little tricky because RST requires a non-letter character after
    the closing backticks, but some CDPs docs have things like "`AxNodeId`s".
    If we double the backticks in that string, then it won't be valid RST. The
    fix is to insert an apostrophe if an "s" trails the backticks.


 ... (clipped 883 lines)

Learn more about managing compliance generic rules or creating your own custom rules

@qodo-merge-pro
Copy link
Contributor

qodo-merge-pro bot commented Nov 17, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
High-level
PR offers low-value cosmetic changes

The PR consists of purely cosmetic changes (single to double-quoted docstrings)
which, despite promoting style consistency, add noise to the project's history.
Such changes are better managed by automated formatting tools across the entire
codebase.

Examples:

py/generate.py [92-1046]
def indent(s, n):
    """ A shortcut for ``textwrap.indent`` that always uses spaces. """
    return tw_indent(s, n * ' ')


BACKTICK_RE = re.compile(r'`([^`]+)`(\w+)?')


def escape_backticks(docstr):
    """

 ... (clipped 945 lines)

Solution Walkthrough:

Before:

def indent(s, n):
    """ A shortcut for ``textwrap.indent`` that always uses spaces. """
    return tw_indent(s, n * ' ')

def docstring(description):
    """ Generate a docstring from a description. """
    if not description:
        return ''
    # ...

@dataclass
class CdpType:
    """ A top-level CDP type. """
    # ...

After:

def indent(s, n):
    ''' A shortcut for ``textwrap.indent`` that always uses spaces. '''
    return tw_indent(s, n * ' ')

def docstring(description):
    ''' Generate a docstring from a description. '''
    if not description:
        return ''
    # ...

@dataclass
class CdpType:
    ''' A top-level CDP type. '''
    # ...
Suggestion importance[1-10]: 8

__

Why: The suggestion correctly identifies the PR as a large, low-value cosmetic change that adds noise to the git history, and rightly proposes using automated tools for such modifications.

Medium
General
Ensure consistent docstring quote style
Suggestion Impact:The commit updated the docstring generator to use triple double quotes in dedent, changing from "'''\n{}\n'''" to '"""\n{}\n"""'.

code diff:

     description = escape_backticks(description)
     return dedent("'''\n{}\n'''").format(description)
 

Update the docstring function to generate docstrings with double quotes (""")
instead of single quotes (''') for consistency.

py/generate.py [132-138]

 def docstring(description):
     """ Generate a docstring from a description. """
     if not description:
         return ''
 
     description = escape_backticks(description)
-    return dedent("'''\n{}\n'''").format(description)
+    return dedent('"""\n{}\n"""').format(description)

[Suggestion processed]

Suggestion importance[1-10]: 7

__

Why: The suggestion correctly points out that while the PR changes existing docstrings to use double quotes, the docstring generator function itself still produces single-quoted docstrings, which is inconsistent with the PR's intent.

Medium
  • Update

@cgoldberg cgoldberg changed the title [py] use docstrings for function comments [py] Use double quotes in generate.py Nov 17, 2025
Copy link
Member

@cgoldberg cgoldberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use docstrings for function comments

They are already docstrings... this just changes the quotes from single to double. To make it consistent, we might as well convert all the quotes to double like we do in the rest of the Python files. I just pushed a commit to your branch that does that.

@Delta456 Delta456 merged commit af9b20e into SeleniumHQ:trunk Nov 18, 2025
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants