fix: make test suite portable across platforms#56
fix: make test suite portable across platforms#56dkundel-openai merged 3 commits intoopenai:mainfrom
Conversation
Replace hardcoded macOS path with fileURLToPath for cross-platform compatibility. Add .cmd wrapper creation and platform-aware PATH separator in test fixtures so the fake codex binary is discoverable on Windows. Add shell and windowsHide options to the test helper run() function to match production behavior. Test results on Windows improve from 12/64 pass to 59/64 pass. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7dad7809c4
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
My PR #45 already fixed the hardcoded path issue, and this PR builds nicely on that to add full Windows support. Glad we’re working together to make the test suite cross-platform! |
Thanks for the fix in #45! |
…path breakage When `process.execPath` resolves to a path with spaces (e.g., `C:\Program Files\nodejs\node.exe`), `shell: true` causes cmd.exe to split the path at the space. Guard with `path.isAbsolute()` so only bare command names (which need `.cmd` shim resolution) use the shell. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
3 participants
Summary
/Users/dkundel/code/codex-pluginwith dynamicimport.meta.url-based resolution in both test entry files..cmdshim creation, platform-awarePATHseparators, andshell: truetospawnSyncto support the test harness on Windows.Problem
The test suite currently has two portability issues:
commands.test.mjsandruntime.test.mjsdefineROOTas"/Users/dkundel/code/codex-plugin". This path is specific to the original author's machine, causing 52 out of 64 tests to fail on any other checkout.helpers.mjscallsspawnSyncwithoutshell: true(preventing Windows from resolving.cmdshims),fake-codex-fixture.mjshardcodes:as thePATHseparator (Windows requires;), and the fake codex binary lacks a.cmdwrapper.Changes
tests/commands.test.mjs: Replaces the hardcodedROOTwithpath.resolve(path.dirname(fileURLToPath(import.meta.url)), "..").tests/runtime.test.mjs: Applies the sameROOTpath resolution.tests/fake-codex-fixture.mjs: Generates acodex.cmdbatch wrapper on Windows after writing the Unix script, and updatesbuildEnvto use a platform-awarePATHseparator.tests/helpers.mjs: Addsshell: process.platform === "win32"andwindowsHide: truetospawnSyncinrun().Test results
Related
ROOTpath). This PR expands on their work by including the necessary Windows spawn,PATH, and shim fixes required to run the tests end-to-end on Windows.