Skip to content

test(windows): fix process and checkout fixture portability - #1623

Merged
Teingi merged 1 commit into
oceanbase:masterfrom
knqiufan:codex/fix-windows-test-portability-1501
Sep 16, 2026
Merged

Teingi merged 1 commit into
oceanbase:masterfrom
knqiufan:codex/fix-windows-test-portability-1501

Conversation

@knqiufan

Copy link
Copy Markdown
Contributor

Which issue or RFC does this PR close?

Closes #1501.

Rationale for this change

On Windows with long-path support disabled, six tests from #1501 still fail on master 0ed20c54: the three OpenCode probe cleanup assertions use POSIX os.kill(pid, 0) semantics, and three remote-checkout fixtures exceed the path limit under pytest's default temporary root. These are test portability defects, not evidence of broken product authorization or subprocess cleanup.

What changes are included in this PR?

  • Check the probe server's published PID with a Windows process handle and a bounded wait. A live process still fails the assertion, unexpected API errors propagate, and handles are closed. Keep the POSIX liveness check on POSIX systems. Checking the server PID also accounts for Windows virtualenv launchers whose PID differs from the Python child.
  • Give the three nested checkout tests an isolated, automatically cleaned temporary directory directly under the system temporary root, leaving room for hashes, plugin files, staging directories, and backup UUIDs.
  • Add a regression that verifies the Windows exit assertion rejects a live subprocess without terminating it.
  • Add a Windows CI job covering all issue scenarios plus related package, external-skill, OpenCode, and bootstrap tests. Disable long-path support on the disposable runner and retain pytest's default temporary root so a short global --basetemp cannot mask fixture regressions. POSIX-only skips remain explicit.

Are there any user-facing changes?

No runtime, API, dependency, or persisted-format changes. Windows contributors can run the affected tests with the default pytest temporary root.

How was this change tested?

Windows x64, CPython 3.12.13, LongPathsEnabled=0:

  • Original issue nodes on unmodified master: 6 failed, 4 passed, 2 skipped.
  • The exact new CI test selection below: 49 passed, 3 skipped, without --basetemp. The skips cover POSIX permissions, O_NOFOLLOW, and symlink support.
uv run pytest -q tests/builtin/artifacts/skill/test_package.py tests/builtin/runtime/test_external_skills.py tests/test_cli.py::test_remote_enroll_can_install_automatic_service_in_one_command tests/test_openclaw_cli.py::test_build_openclaw_plugin_runs_pnpm_install_non_interactively tests/test_opencode_cli.py tests/test_service_bootstrap.py tests/test_cli_workbuddy.py::test_setup_workbuddy_remote_checkout_refreshes_the_requested_ref

Additional validation:

  • uv run pytest -q tests/test_opencode_cli.py: 18 passed after the platform guard was added.
  • uv run pytest -q tests/test_integration_manifest.py: 28 passed.
  • uv lock --locked.
  • uv run python scripts/check_workflow_actions.py .github/workflows .github/actions: 76 action references checked.
  • uv run python scripts/generate_integration_manifest_docs.py --check.
  • Ruff lint and formatting checks for the changed Python files.
  • uv run ty check tests/conftest.py tests/test_opencode_cli.py tests/test_cli_workbuddy.py, also with --python-platform linux.
  • uv run ty check integrations/pydantic-ai/src.
  • Full repository type checking passed with --python-platform linux --exclude docs/my --exclude .t1494 --exclude .tmp-windows-path-1494; exclusions are pre-existing untracked local directories.
  • uv run prek run -a: all hooks except ty-check passed. Native Windows whole-repository type checking reports pre-existing errors in local document scripts and unrelated multiprocessing/POSIX annotations; changed-file Windows checks and the Linux-target repository check above passed.

The hosted Windows CI job and Linux runtime matrix require CI execution; they were not run locally.

AI usage statement

OpenAI Codex (GPT-6) assisted with issue review, Windows reproduction, implementation, regression validation, and PR preparation.

@Teingi Teingi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@Teingi
Teingi merged commit 346fef7 into oceanbase:master Sep 16, 2026
21 checks passed
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.

bug(windows): unit tests rely on POSIX semantics and overlong temporary paths

2 participants