Skip to content

Conversation

@cgoldberg
Copy link
Member

@cgoldberg cgoldberg commented Oct 1, 2025

User description

💥 What does this PR do?

This PR updates Python's CI workflow:

  • adds a new unit-tests job to run unit tests on Ubuntu/MacOS on the GH runners
  • adds Chrome/Windows, Edge/Windows to the integration test browser/os matrix on the GH runners
  • adds additional test target for Mac/Safari on the GH runners

This also fixes an assertion in a unit test that was broken on Windows.

🔧 Implementation Notes

We were not currently running any Python tests on Windows in CI. We have some Windows platform-specific code in the Python bindings and not all maintainers have a Windows system to test against.

💡 Additional Considerations

This might make the CI runs slightly longer.

🔄 Types of changes

  • Build/Test/CI

PR Type

Tests


Description

  • Add unit test job for Windows and Ubuntu platforms

  • Add Chrome/Windows integration test configuration

  • Expand CI coverage for Windows platform testing


Diagram Walkthrough

flowchart LR
  A["CI Workflow"] --> B["Unit Tests Job"]
  B --> C["Windows Testing"]
  B --> D["Ubuntu Testing"]
  A --> E["Integration Tests"]
  E --> F["Chrome/Windows Added"]
Loading

File Walkthrough

Relevant files
Tests
ci-python.yml
Expand CI with unit tests and Windows support                       

.github/workflows/ci-python.yml

  • Add new unit-tests job with Windows and Ubuntu matrix
  • Add Chrome/Windows configuration to integration tests matrix
  • Configure Bazel test execution for unit tests
+18/-0   

@selenium-ci selenium-ci added the B-build Includes scripting, bazel and CI integrations label Oct 1, 2025
@qodo-merge-pro
Copy link
Contributor

qodo-merge-pro bot commented Oct 1, 2025

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

Matrix Consistency

Ensure the referenced Bazel target '//py:unit' runs correctly on Windows; some tests may need skipping or platform conditionals, and shell/path separators could cause failures.

run: bazel test //py:unit

@qodo-merge-pro
Copy link
Contributor

qodo-merge-pro bot commented Oct 1, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Fix syntax error in expression
Suggestion Impact:The commit removed the extra '}' from the cache-key line, fixing the syntax error exactly as suggested.

code diff:

-      cache-key: python-unit-test-${{ matrix.os }}}
+      cache-key: python-unit-test-${{ matrix.os }}

Remove the extra closing curly brace } from the cache-key value to fix a syntax
error in the GitHub Actions expression.

.github/workflows/ci-python.yml [70]

-cache-key: python-unit-test-${{ matrix.os }}}
+cache-key: python-unit-test-${{ matrix.os }}

[Suggestion processed]

Suggestion importance[1-10]: 9

__

Why: The suggestion correctly identifies a syntax error in the GitHub Actions expression for cache-key which would cause the CI job to fail.

High
  • Update

@cgoldberg cgoldberg changed the title [build] Python CI - add unit test job and windows integration tests to GH runners [py][build] Python CI - add unit test job and windows integration tests to GH runners Oct 1, 2025
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 Review effort 2/5

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants