Skip to content

test: add windows & macos on tests runner #2710

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

lucasgomide
Copy link
Contributor

This will help us ensure the framework works well on Windows, Ubuntu, and macOS

@mplachta
Copy link
Contributor

Disclaimer: This review was made by a crew of AI Agents.

Summary of Key Findings

This PR improves the CI workflow by expanding test coverage from a single Ubuntu environment to include Windows and macOS runners, using a matrix strategy in GitHub Actions. The modification is concise and correctly applies GitHub Actions’ matrix syntax to run tests on multiple platforms and Python versions in parallel. This expansion will help detect OS-specific issues earlier, improving overall code robustness and cross-platform compatibility.

Specific Code Improvements

  1. Display OS Environment for Clarity:
    Adding a step to echo the runner OS helps developers and maintainers quickly identify which platform each job ran on during CI runs. For example:

    - name: Show runner OS
      run: echo "Running on $RUNNER_OS"
  2. Consider fail-fast: false in Strategy:
    To allow all OS+Python combinations to run even if some fail, reduce flaky failure impact and provide full test coverage:

    strategy:
      fail-fast: false
      matrix:
        runner: [ubuntu-latest, windows-latest, macos-latest]
        python-version: ['3.10', '3.11', '3.12']
  3. Commenting for Future Maintainability:
    Adding comments explaining the purpose of matrix runners avoids confusion for future maintainers:

    # Run tests across Ubuntu, Windows, and macOS to ensure platform compatibility
  4. Watch for OS-Specific Step Adjustments:
    Upon encountering failures, consider conditional steps or shell usage adjustments especially with Windows (PowerShell or CMD) versus Unix-like shells. For instance:

    - name: Install dependencies
      run: |
        python -m pip install --upgrade pip
        pip install -r requirements.txt
      shell: bash

    or OS conditionals via if expressions may be needed for commands that vary.

Historical Context and Learnings from Related PRs

  • Progressive enhancement of CI workflows often starts with Ubuntu-only runners, then expands to Windows and macOS using the matrix strategy, as done here.
  • Past similar PRs show the importance of balancing build time vs cross-platform coverage, often adopting fail-fast: false to optimize developer feedback.
  • OS-specific installation or runtime adjustments typically follow multi-platform runs due to environment differences surfaced by these changes.
  • Clear documentation inside workflows is recommended in related PRs to ease team understanding and maintenance effort.

Implications for Related Files

  • Potential impact on requirements.txt or dependency installation if any packages have OS-specific installation issues.
  • Test code might need review for platform-dependent assumptions (e.g., file paths, shell commands).
  • The workflow changes do not touch source code but surface platform-specific failings sooner, prompting timely fixes.

Specific Improvement Suggestions with Example

jobs:
  tests:
    runs-on: $
    timeout-minutes: 15
    strategy:
      fail-fast: false  # Run all matrix jobs even if one fails
      matrix:
        runner: [ubuntu-latest, windows-latest, macos-latest]
        python-version: ['3.10', '3.11', '3.12']
    steps:
      - name: Show runner OS
        run: echo "Running on $RUNNER_OS"
      - name: Checkout code
        uses: actions/checkout@v3
      - name: Setup Python
        uses: actions/setup-python@v5
        with:
          python-version: $
      - name: Install dependencies
        run: |
          python -m pip install --upgrade pip
          pip install -r requirements.txt
      - name: Run tests
        run: pytest

Final Recommendation

This PR is a well-implemented and low-risk improvement that increases CI robustness across major platforms. It follows standard GitHub Actions patterns and should be merged. Future work should monitor for any OS-specific failures, add runner environment outputs, and document workflow rationale. Additional OS-specific shell handling or conditional steps might be required as needed after initial test runs.

Thank you for advancing the project’s testing infrastructure to be truly cross-platform!


If you want direct references on matrix strategy usage, see the official docs: https://docs.github.com/en/actions/using-jobs/using-a-matrix-for-your-jobs

This will help us ensure the framework works well on Windows, Ubuntu, and macOS
@lucasgomide lucasgomide force-pushed the lg-improve-test-runner branch from 4def95b to 14feeaa Compare April 28, 2025 22:11
The version 1.167.2 is not compatible with Windows
@lucasgomide lucasgomide force-pushed the lg-improve-test-runner branch from 14feeaa to 63dfea9 Compare April 28, 2025 22:23
@lucasgomide lucasgomide marked this pull request as draft April 28, 2025 22:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants