Skip to content

fix: add Windows test coverage - #362

Open
abhinavkr26104 wants to merge 1 commit into
browserbase:mainfrom
abhinavkr26104:fix/355-windows-ci
Open

fix: add Windows test coverage#362
abhinavkr26104 wants to merge 1 commit into
browserbase:mainfrom
abhinavkr26104:fix/355-windows-ci

Conversation

@abhinavkr26104

@abhinavkr26104 abhinavkr26104 commented Aug 15, 2026

Copy link
Copy Markdown

Summary

  • run the supported Python test matrix on both Ubuntu and Windows
  • force the byte fixture to LF in Git checkouts so its base64 assertion is platform-stable
  • make proxy environment setup robust to Windows case-insensitive variable names
  • skip tracemalloc leak heuristics on Windows, where runtime and worker allocations make them unstable

Testing

  • uv run --frozen --isolated --all-extras --python 3.9 pytest (502 passed, 297 skipped)
  • uv run --frozen --isolated --all-extras --python 3.14 pytest (502 passed, 297 skipped)
  • uv run --frozen ruff check .
  • uv run --frozen pyright -p .
  • uv run --frozen mypy --platform linux .

Fixes #355


Summary by cubic

Add Windows to the CI test matrix and make tests cross-platform stable. CI previously ran only on Ubuntu; now it runs on Ubuntu and Windows, with fixes for CRLF-sensitive fixtures, Windows proxy env var behavior, and flaky tracemalloc leak checks. Fixes #355.

  • Review notes:
    • CI runs on both ubuntu-latest and windows-latest; bootstrap and test steps run with bash.
    • .gitattributes forces LF for tests/sample_file.txt to keep base64 assertions stable across OSes.
    • Proxy environment test handles case-insensitive names on Windows by clearing both cases and setting HTTPS_PROXY last.
    • Tracemalloc-based leak assertions are skipped on Windows and Python >=3.10 to avoid flaky failures; no runtime behavior changes.

Written for commit b3ea05e. Summary will update on new commits.

Review in cubic

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

1 issue found across 3 files

Confidence score: 2/5

  • In .github/workflows/ci.yml, the runs-on logic in stainless-sdks/* repos appears to force the Ubuntu depot runner for all matrix entries, so the Windows job may never execute on Windows and Windows-specific breakages can slip through CI—restrict the depot override to only the Ubuntu matrix case so windows uses a Windows runner.
Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name=".github/workflows/ci.yml">

<violation number="1" location=".github/workflows/ci.yml:85">
P1: In `stainless-sdks/*` repositories, this `runs-on` expression always picks the Ubuntu depot runner, so the Windows matrix job never gets a Windows runner. Gate the depot override to the Ubuntu matrix entry so `windows-latest` still runs on Windows.</violation>
</file>
Architecture diagram
sequenceDiagram
    participant GH as GitHub Actions
    participant CI as CI Workflow (test job)
    participant Runner as CI Runner (Ubuntu/Windows)
    participant Bash as Bash Shell
    participant Bootstrap as Bootstrap Script
    participant Test as Test Script
    participant Git as Git Checkout (LF forced)
    participant PyTest as Pytest (Python 3.9/3.14)
    participant Env as Environment Variables
    participant Tracemalloc as Tracemalloc Leak Check
    participant ProxyTest as Proxy Env Test
    participant Client as DefaultHttpxClient

    Note over GH,Runner: CI Test Matrix (Ubuntu + Windows)

    GH->>CI: Trigger test job
    CI->>Runner: Select runner (matrix.os)
    Runner->>Git: Checkout repo
    Git->>Git: CHANGED: Force LF for tests/sample_file.txt (.gitattributes)

    Runner->>Bash: Run bootstrap
    Bash->>Bootstrap: Execute ./scripts/bootstrap
    Bootstrap-->>Bash: Setup complete

    Runner->>Bash: Run tests
    Bash->>Test: Execute ./scripts/test
    Test->>PyTest: Run test suite (3.9 and 3.14)

    alt Windows OS
        Note over PyTest,Tracemalloc: Platform-specific behavior
        PyTest->>Tracemalloc: Skip leak assertions (win32)
        PyTest->>ProxyTest: Run proxy env test (case-insensitive vars)
    else Ubuntu OS
        PyTest->>Tracemalloc: Skip only if Python >= 3.10
        PyTest->>ProxyTest: Run proxy env test
    end

    ProxyTest->>Env: Clear HTTP_PROXY, ALL_PROXY, lowercase variants
    ProxyTest->>Env: Set HTTPS_PROXY (last, after clearing)
    Env-->>ProxyTest: Case-insensitive handling on Windows
    ProxyTest->>Client: Instantiate client with proxy env
    Client-->>ProxyTest: Client configured

    PyTest-->>Test: Test results
    Test-->>Bash: Exit code
    Bash-->>Runner: Pass/Fail
    Runner-->>CI: Report status
    CI-->>GH: Job status
Loading

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread .github/workflows/ci.yml
fail-fast: false
matrix:
os: [ubuntu-latest, windows-latest]
runs-on: ${{ startsWith(github.repository, 'stainless-sdks/') && 'depot-ubuntu-24.04' || matrix.os }}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1: In stainless-sdks/* repositories, this runs-on expression always picks the Ubuntu depot runner, so the Windows matrix job never gets a Windows runner. Gate the depot override to the Ubuntu matrix entry so windows-latest still runs on Windows.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/ci.yml, line 85:

<comment>In `stainless-sdks/*` repositories, this `runs-on` expression always picks the Ubuntu depot runner, so the Windows matrix job never gets a Windows runner. Gate the depot override to the Ubuntu matrix entry so `windows-latest` still runs on Windows.</comment>

<file context>
@@ -77,8 +77,12 @@ jobs:
+      fail-fast: false
+      matrix:
+        os: [ubuntu-latest, windows-latest]
+    runs-on: ${{ startsWith(github.repository, 'stainless-sdks/') && 'depot-ubuntu-24.04' || matrix.os }}
     if: github.event_name == 'push' || github.event.pull_request.head.repo.fork
     steps:
</file context>
Suggested change
runs-on: ${{ startsWith(github.repository, 'stainless-sdks/') && 'depot-ubuntu-24.04' || matrix.os }}
runs-on: ${{ startsWith(github.repository, 'stainless-sdks/') && matrix.os == 'ubuntu-latest' && 'depot-ubuntu-24.04' || matrix.os }}

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.

Make byte fixtures checkout-stable and run tests on Windows CI

1 participant