Skip to content

Add Windows backend test preflight and runner#7666

Closed
tianmind-studio wants to merge 8 commits into
BasedHardware:mainfrom
tianmind-studio:codex/windows-backend-preflight
Closed

Add Windows backend test preflight and runner#7666
tianmind-studio wants to merge 8 commits into
BasedHardware:mainfrom
tianmind-studio:codex/windows-backend-preflight

Conversation

@tianmind-studio

@tianmind-studio tianmind-studio commented Jun 6, 2026

Copy link
Copy Markdown
Contributor

Summary

  • add a PowerShell backend preflight for Windows contributors
  • add a Windows test.ps1 runner that mirrors the unit-test list from test.sh
  • document the Windows backend workflow and explicit -Python override

Review follow-up

  • Python version mismatches are warnings, not hard failures, so Python 3.12/3.13 users are not blocked by the preflight itself
  • test.sh validation now extracts tests/* tokens instead of depending on a narrow trailing-flag regex

Verification

  • powershell -NoProfile -ExecutionPolicy Bypass -File .\backend\test-preflight.ps1 -Python 'E:\维修前备份-2026-05-23\omi-dev-tools-venv\Scripts\python.exe'
    • 10 passed, 10 warnings, 0 failed
    • warnings were environment/tooling warnings: Python 3.10.6 vs expected 3.11, optional deepgram/openpipe imports, optional integration env vars, and redis-cli not installed

@greptile-apps

greptile-apps Bot commented Jun 6, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds a PowerShell equivalent of the existing test-preflight.sh for Windows contributors, along with documentation updates in AGENTS.md and README.md. The script checks Python, pytest, formatter availability, key backend imports, optional env vars, Redis CLI connectivity, and test.sh file references.

  • test-preflight.ps1: New script mirroring the bash preflight; uses $ErrorActionPreference = \"Stop\", a helper Invoke-Python wrapper that temporarily relaxes error handling for native commands, and structured pass/warn/fail counters.
  • Documentation: Both AGENTS.md and README.md are updated to document the Windows invocation pattern and the -Python flag for specifying an explicit Python 3.11 path.

Confidence Score: 3/5

Safe to merge for documentation and tooling; the PowerShell script is read-only but will block Windows contributors running Python 3.12 or 3.13 with a hard exit-1 that the bash counterpart does not impose.

The preflight script only affects local developer workflow, not CI or production. The Python version gate diverges from the bash counterpart, producing a hard exit-1 for Python 3.12/3.13 users on Windows where the bash user would see no failure at all.

backend/test-preflight.ps1 — specifically the Python version check block around line 107 and the test.sh flag-stripping regex around line 219.

Important Files Changed

Filename Overview
backend/test-preflight.ps1 New Windows preflight script mirroring the bash version; the Python version check uses Write-Bad (hard failure) instead of Write-Warn for non-3.11 Python, diverging from the bash script's permissive behavior and causing exit code 1 for Python 3.12/3.13 users.
backend/AGENTS.md Adds one-line note for Windows contributors pointing to the new PowerShell preflight command; straightforward and accurate.
backend/README.md Adds a 'Running Backend Tests on Windows' section documenting the PowerShell preflight and the -Python flag for explicit version overrides; content matches the script behaviour.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A([Start test-preflight.ps1]) --> B{-Python param provided?}
    B -- Yes --> C[Use provided path]
    B -- No --> D{python in PATH?}
    D -- Yes --> E[Use python]
    D -- No --> F{py in PATH?}
    F -- Yes --> G[Use py -3]
    F -- No --> H[Write-Bad: python not found]
    C & E & G --> I{python --version matches 3.11.x?}
    I -- Yes --> J[Write-Ok]
    I -- No --> K[Write-Bad: expects 3.11]
    J & K --> L{pytest installed?}
    L -- Yes --> M[Write-Ok]
    L -- No --> N[Write-Bad]
    M & N --> O{black available?}
    O -- found --> P[Write-Ok]
    O -- missing --> Q[Write-Warn]
    P & Q --> R[Check backend package imports]
    R --> S[Check env vars]
    S --> T{redis-cli in PATH?}
    T -- Yes --> U{redis-cli ping succeeds?}
    U -- Yes --> V[Write-Ok]
    U -- No --> W[Write-Warn]
    T -- No --> X[Write-Warn]
    V & W & X --> Y[Count unit test files in tests/unit/]
    Y --> Z[Parse test.sh references and verify paths]
    Z --> AA{FailCount > 0?}
    AA -- Yes --> AB([exit 1])
    AA -- No --> AC{WarnCount > 0?}
    AC -- Yes --> AD([exit 0 with warnings])
    AC -- No --> AE([exit 0 all clear])
Loading

Reviews (1): Last reviewed commit: "Avoid Redis password argv in Windows pre..." | Re-trigger Greptile

Comment on lines +101 to +105
if ($pythonVersion -match "Python 3\.11\.") {
Write-Ok $pythonVersion
} else {
Write-Bad "$pythonVersion detected; backend expects Python 3.11"
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 Version mismatch exits with hard failure, blocking non-3.11 Windows contributors

The bash counterpart (test-preflight.sh) reports whatever Python version is present as "ok" without any version gate. This script uses Write-Bad for any Python that isn't 3.11.x, which causes exit code 1 and the message "Fix failures above before running test.sh" for a developer running Python 3.12 or 3.13 — even if their environment is otherwise fully functional. Using Write-Warn here would match the bash behavior (advisory rather than blocking) and avoid unnecessarily gating Windows contributors on an exact minor version.

Suggested change
if ($pythonVersion -match "Python 3\.11\.") {
Write-Ok $pythonVersion
} else {
Write-Bad "$pythonVersion detected; backend expects Python 3.11"
}
if ($pythonVersion -match "Python 3\.11\.") {
Write-Ok $pythonVersion
} else {
Write-Warn "$pythonVersion detected; backend expects Python 3.11"
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Addressed in 3d942f839: the Python version mismatch now uses Write-Warn instead of Write-Bad, so non-3.11 Windows users are not blocked by the preflight. Re-ran on this Windows machine with Python 3.10.6: 10 passed, 10 warnings, 0 failed, exit code 0.

Comment on lines +219 to +224
} else {
Write-Bad "No unit test files found in tests/unit/"
}

$missingTests = @()
if (Test-Path "test.sh") {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 test.sh flag-stripping regex may silently drop test references if flags change

The pattern \s+-v\s*$ strips only a trailing -v. Lines in test.sh that carry additional pytest flags (e.g., --tb=short, -x) won't be stripped correctly, leaving the flag text as part of the resolved path, which will then fail Test-Path and be reported as a missing file. The bash counterpart uses sed 's/ -v//' (not anchored) which has the same narrow assumption. Both are fine today, but worth noting if test.sh flags ever change.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Addressed in 3d942f839: the test.sh validation now tokenizes each pytest line and only keeps tokens matching tests/*, so future pytest flags are ignored instead of being resolved as paths. Re-ran the Windows preflight: 10 passed, 10 warnings, 0 failed, exit code 0.

@tianmind-studio tianmind-studio changed the title Add Windows backend test preflight Add Windows backend test preflight and runner Jun 6, 2026
@tianmind-studio tianmind-studio force-pushed the codex/windows-backend-preflight branch from 5f845fc to 69ea4d7 Compare June 8, 2026 13:21

Copy link
Copy Markdown
Contributor Author

I rebased and force-pushed this branch onto the current main, then reran the Windows preflight with the explicit local Python path. It still reports 10 passed, 0 failed; the remaining items are environment warnings for Python 3.10 vs 3.11, optional imports/env vars, and missing redis-cli.

@tianmind-studio tianmind-studio force-pushed the codex/windows-backend-preflight branch from 69ea4d7 to d22cb2a Compare June 10, 2026 04:02

Copy link
Copy Markdown
Contributor Author

Closing this in favor of #7799 for the Windows backend preflight path.

After rechecking both branches, #7799 is the cleaner canonical PR for the preflight helper: it is rebased onto the current main, only changes backend/test-preflight.ps1 and backend/README.md, and already includes the Windows execution-policy and Push-Location/Pop-Location review fixes.

This older PR also carries the broader test.ps1 runner and backend/AGENTS.md docs. That runner idea is still useful, but it should be split into a separate, focused follow-up after the preflight lands so maintainers do not have to review the runner and the preflight together.

@github-actions

Copy link
Copy Markdown
Contributor

Hey @tianmind-studio 👋

Thank you so much for taking the time to contribute to Omi! We truly appreciate you putting in the effort to submit this pull request.

After careful review, we've decided not to merge this particular PR. Please don't take this personally — we genuinely try to merge as many contributions as possible, but sometimes we have to make tough calls based on:

  • Project standards — Ensuring consistency across the codebase
  • User needs — Making sure changes align with what our users need
  • Code best practices — Maintaining code quality and maintainability
  • Project direction — Keeping aligned with our roadmap and vision

Your contribution is still valuable to us, and we'd love to see you contribute again in the future! If you'd like feedback on how to improve this PR or want to discuss alternative approaches, please don't hesitate to reach out.

Thank you for being part of the Omi community! 💜

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.

1 participant