Skip to content

[py] Don't compare object identity in conftest#16723

Merged
cgoldberg merged 1 commit intoSeleniumHQ:trunkfrom
cgoldberg:py-conftest-boolean
Dec 12, 2025
Merged

[py] Don't compare object identity in conftest#16723
cgoldberg merged 1 commit intoSeleniumHQ:trunkfrom
cgoldberg:py-conftest-boolean

Conversation

@cgoldberg
Copy link
Member

@cgoldberg cgoldberg commented Dec 12, 2025

User description

💥 What does this PR do?

This is a tiny change to use Falsy comparisons instead of comparing object identity for booleans.

🔧 Implementation Notes

This should affect anything.. just changing unpythonic code.

🔄 Types of changes

  • Cleanup

PR Type

Enhancement


Description

  • Replace identity comparison with falsy check for booleans

  • Improve code pythonicity in conftest fixtures

  • Apply consistent boolean comparison pattern across file


Diagram Walkthrough

flowchart LR
  A["marker.kwargs['run'] is False"] -- "refactor to" --> B["not marker.kwargs['run']"]
  B -- "applied in" --> C["driver fixture"]
  B -- "applied in" --> D["clean_driver fixture"]
Loading

File Walkthrough

Relevant files
Cleanup
conftest.py
Replace identity comparison with falsy boolean checks       

py/conftest.py

  • Replace is False identity comparison with not operator in two
    locations
  • Update boolean check in driver fixture at line 337
  • Update boolean check in clean_driver fixture at line 478
  • Improves code style to follow Python conventions for boolean
    evaluation
+2/-2     

@selenium-ci selenium-ci added the C-py Python Bindings label Dec 12, 2025
@qodo-code-review
Copy link
Contributor

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
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: Robust Error Handling and Edge Case Management

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

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: Security-First Input Validation and Data Handling

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

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 boolean checks alter test control flow (skipping) without adding or referencing
any audit logging of these critical actions, but this is a test fixture where audit trails
may not be applicable.

Referred Code
if not marker.kwargs["run"]:
    pytest.skip()
    yield
    return

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

@qodo-code-review
Copy link
Contributor

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Remove unreachable code after pytest.skip()

Remove the unreachable yield and return statements that follow the pytest.skip()
call.

py/conftest.py [336-339]

 if not marker.kwargs["run"]:
     pytest.skip()
-    yield
-    return
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why: The suggestion correctly identifies that pytest.skip() raises an exception, making the subsequent yield and return statements unreachable. Removing this dead code is a valid improvement for code clarity and correctness.

Low
  • More

@cgoldberg cgoldberg merged commit 50418b8 into SeleniumHQ:trunk Dec 12, 2025
25 checks passed
@cgoldberg cgoldberg deleted the py-conftest-boolean branch December 12, 2025 17:25
This was referenced Jan 19, 2026
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.

2 participants