Skip to content

Conversation

@cgoldberg
Copy link
Member

@cgoldberg cgoldberg commented Jun 10, 2025

User description

💥 What does this PR do?

This PR removes publication of mypy type coverage during CI runs (introduced in #9523). The coverage stats were previously published to codecov.io (https://app.codecov.io/gh/SeleniumHQ/selenium/), but they haven't been updated in over 5 months. AFAIK nobody is looking at them.

The mypy CI job remains intact, but it will never fail or block CI. Eventually we should gate merges if coverage dips below 100%, but we aren't there yet.

mypy type coverage is being tracked in #15697

🔄 Types of changes

  • Build infrastructure

PR Type

Other


Description

• Remove mypy type coverage publication to codecov.io from CI
• Update job names to be more descriptive
• Remove lxml dependency from mypy environment
• Keep mypy checks running but non-blocking


Changes walkthrough 📝

Relevant files
Configuration changes
ci-python.yml
Remove codecov integration from CI workflow                           

.github/workflows/ci-python.yml

• Updated job step names for clarity ("Generate docs", "Run type
checking")
• Removed codecov publication commands and bash script
execution
• Simplified mypy job to only run type checking without
coverage reporting

+3/-4     
Dependencies
tox.ini
Remove lxml dependency from mypy environment                         

py/tox.ini

• Removed lxml dependency from mypy test environment
• Cleaned up
dependencies list for type checking

+0/-1     

Need help?
  • Type /help how to ... in the comments thread for any questions about Qodo Merge usage.
  • Check out the documentation for more information.
  • @selenium-ci selenium-ci added C-py Python Bindings B-build Includes scripting, bazel and CI integrations labels Jun 10, 2025
    @qodo-merge-pro
    Copy link
    Contributor

    PR Reviewer Guide 🔍

    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

    Silent Failures

    The mypy job now uses || true which will always succeed even if type checking fails. This could mask legitimate type checking issues and defeats the purpose of running mypy in CI.

      tox -c py/tox.ini || true
    env:

    @qodo-merge-pro
    Copy link
    Contributor

    qodo-merge-pro bot commented Jun 10, 2025

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Remove error suppression from type checking

    The || true command prevents CI from failing when type checking finds issues,
    which defeats the purpose of running type checks in CI. Remove || true to ensure
    type checking failures are properly reported and cause the CI to fail.

    .github/workflows/ci-python.yml [51-55]

     - name: Run type checking
       run: |
    -    tox -c py/tox.ini || true
    +    tox -c py/tox.ini
       env:
         TOXENV: mypy
    • Apply / Chat
    Suggestion importance[1-10]: 9

    __

    Why: The suggestion correctly identifies that || true suppresses errors from the type checker, which undermines the purpose of having a type-checking step in the CI pipeline. Removing it ensures that type errors will cause the build to fail, enforcing code quality.

    High
    • Update

    @cgoldberg cgoldberg merged commit c65425f into SeleniumHQ:trunk Jun 11, 2025
    16 checks passed
    @cgoldberg cgoldberg deleted the py-remove-codecov-publishing branch June 11, 2025 14:52
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Labels

    B-build Includes scripting, bazel and CI integrations C-py Python Bindings Review effort 2/5

    Projects

    None yet

    Development

    Successfully merging this pull request may close these issues.

    2 participants