Skip to content

Tests: skip ios-team-id on Windows#32602

Closed
arjunaskykok wants to merge 5 commits intoopenclaw:mainfrom
arjunaskykok:fix/ios-team-id-test-windows-skip
Closed

Tests: skip ios-team-id on Windows#32602
arjunaskykok wants to merge 5 commits intoopenclaw:mainfrom
arjunaskykok:fix/ios-team-id-test-windows-skip

Conversation

@arjunaskykok
Copy link
Copy Markdown

@arjunaskykok arjunaskykok commented Mar 3, 2026

Summary

Describe the problem and fix in 2–5 bullets:

  • Problem: test/scripts/ios-team-id.test.ts executes a macOS-only bash script and fails on Windows.
  • Why it matters: Windows test runs fail even though the script is not supported there.
  • What changed: Skip the script-executing tests on win32.
  • What did NOT change (scope boundary): The parsing-only tests still run on all platforms; the script itself is unchanged.

Change Type (select all)

  • Bug fix
  • Feature
  • Refactor
  • Docs
  • Security hardening
  • Chore/infra

Scope (select all touched areas)

  • Gateway / orchestration
  • Skills / tool execution
  • Auth / tokens
  • Memory / storage
  • Integrations
  • API / contracts
  • UI / DX
  • CI/CD / infra

Linked Issue/PR

  • Closes #
  • Related #

User-visible / Behavior Changes

None.

Security Impact (required)

  • New permissions/capabilities? (No)
  • Secrets/tokens handling changed? (No)
  • New/changed network calls? (No)
  • Command/tool execution surface changed? (No)
  • Data access scope changed? (No)
  • If any Yes, explain risk + mitigation:

Repro + Verification

Environment

  • OS: Windows 11 (PowerShell)
  • Runtime/container: Node (pnpm)
  • Model/provider: N/A
  • Integration/channel (if any): N/A
  • Relevant config (redacted): N/A

Steps

  1. Run pnpm test test/scripts/ios-team-id.test.ts on Windows.
  2. Observe the macOS-only script tests fail.

Expected

  • Script-executing tests are skipped on Windows; non-script tests still run.

Actual

  • Before: tests failed on Windows.
  • After: tests are skipped on Windows (not re-run locally due to pnpm EPERM when invoking node.exe --version).

Evidence

Attach at least one:

  • Failing test/log before + passing after
  • Trace/log snippets
  • Screenshot/recording
  • Perf numbers (if relevant)

Human Verification (required)

What you personally verified (not just CI), and how:

  • Verified scenarios: Not run locally; pnpm failed with EPERM when invoking node.exe --version.
  • Edge cases checked: None.
  • What you did not verify: The updated tests on Windows/macOS.

Compatibility / Migration

  • Backward compatible? (Yes)
  • Config/env changes? (No)
  • Migration needed? (No)
  • If yes, exact upgrade steps:

Failure Recovery (if this breaks)

  • How to disable/revert this change quickly: Revert the commit.
  • Files/config to restore: test/scripts/ios-team-id.test.ts.
  • Known bad symptoms reviewers should watch for: None expected.

Risks and Mitigations

None.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Mar 3, 2026

Greptile Summary

This PR skips the two macOS-specific shell-script execution tests in test/scripts/ios-team-id.test.ts on Windows by introducing an IS_WINDOWS constant and using Vitest's it.runIf(!IS_WINDOWS) API. The pure-TypeScript parsing/picking tests continue to run on all platforms unaffected.

  • Two script-executing tests are now guarded with it.runIf(!IS_WINDOWS) so they are skipped on win32 but still run on macOS/Linux.
  • The new IS_WINDOWS constant is correct and consistent with the existing inline process.platform === "win32" checks already present in BASH_BIN / BASH_ARGS.
  • The beforeAll / afterAll hooks still execute on Windows (writing bash-shebang fixtures and calling chmodSync), but since none of the guarded tests run on Windows this is harmless.
  • Minor: the callback body of the second it.runIf block is indented at the same level as its arguments rather than one level deeper, creating a cosmetic inconsistency with the first block.

Confidence Score: 5/5

  • This PR is safe to merge — it only skips two macOS-only tests on Windows without touching any production code.
  • The change is minimal and isolated to a single test file. The Vitest it.runIf API is used correctly, the IS_WINDOWS guard is accurate, and the non-script parsing tests remain cross-platform. The only finding is a cosmetic indentation inconsistency with no runtime impact.
  • No files require special attention.

Last reviewed commit: 5b7e90d

greptile-apps[bot]
greptile-apps bot reviewed Mar 3, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

1 file reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

@arjunaskykok arjunaskykok changed the title Tests: skip ios-team-id on Windows Tests: skip ios-team-id on Windows and Linux Mar 3, 2026
chatgpt-codex-connector[bot]
chatgpt-codex-connector bot reviewed Mar 9, 2026
View reviewed changes
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 440ed1e963

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

@arjunaskykok arjunaskykok changed the title Tests: skip ios-team-id on Windows and Linux Tests: skip ios-team-id on Windows Mar 9, 2026
@BradGroux BradGroux mentioned this pull request Mar 17, 2026
Draft
@vincentkoc vincentkoc added the r: no-ci-pr Auto-response for CI/test-failure PRs label Mar 20, 2026
@openclaw-barnacle
Copy link
Copy Markdown

openclaw-barnacle bot commented Mar 20, 2026

Please don't make PRs for test failures on main.

The team is aware of those and will handle them directly on the codebase, not only fixing the tests but also investigating what the root cause is. Having to sift through test-fix-PRs (including some that have been out of date for weeks...) on top of that doesn't help. There are already way too many PRs for humans to manage; please don't make the flood worse.

Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@chatgpt-codex-connector chatgpt-codex-connector[bot] chatgpt-codex-connector[bot] left review comments

+1 more reviewer

@greptile-apps greptile-apps[bot] greptile-apps[bot] left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

r: no-ci-pr Auto-response for CI/test-failure PRs size: XS

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@arjunaskykok @vincentkoc