Skip to content

fix(env): escape and test generated Fish paths - #2492

Open
fengmk2 wants to merge 4 commits into
mainfrom
agent/fish-shell-snapshots
Open

fix(env): escape and test generated Fish paths#2492
fengmk2 wants to merge 4 commits into
mainfrom
agent/fish-shell-snapshots

Conversation

@fengmk2

@fengmk2 fengmk2 commented Aug 18, 2026

Copy link
Copy Markdown
Member

Tests did not run env.fish in Fish. They missed errors in path quoting, PATH updates, and wrapper exit status.

Fish returns a nonzero status when set -e targets a variable that does not exist. The old wrapper stopped before file-based version selection.

CI installs Fish and runs the generated file in Fish. The renderer escapes Fish paths and removes duplicate Vite+ bin entries. The wrapper makes unset safe when the version variable does not exist. It returns a failed vp env use status without applying output.

The snapshot scheme follows #2191. The snapshots cover setup and version selection. They also cover command failure.

@netlify

netlify Bot commented Aug 18, 2026

Copy link
Copy Markdown

Deploy Preview for viteplus-preview canceled.

Name Link
🔨 Latest commit a3bd5de
🔍 Latest deploy log https://app.netlify.com/projects/viteplus-preview/deploys/6a85bdfc0c5e0c0009c3b54d

@github-actions

github-actions Bot commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Native binary sizes (a3bd5de)

Final release artifacts built by the canonical build-upstream and build-windows-cli actions.

Artifact Format Base PR Change
vp (Linux x64) Binary 10.70 MiB 10.70 MiB 0 B (0.00%)
vp (Linux x64) gzip -9 4.63 MiB 4.63 MiB +107 B (+0.00%)
NAPI (Linux x64) Binary 32.03 MiB 32.03 MiB 0 B (0.00%)
NAPI (Linux x64) gzip -9 12.61 MiB 12.61 MiB 0 B (0.00%)
vp (macOS ARM64) Binary 8.00 MiB 8.00 MiB 0 B (0.00%)
vp (macOS ARM64) gzip -9 4.04 MiB 4.04 MiB -52 B (-0.00%)
NAPI (macOS ARM64) Binary 39.68 MiB 39.68 MiB 0 B (0.00%)
NAPI (macOS ARM64) gzip -9 16.92 MiB 16.92 MiB 0 B (0.00%)
vp (Windows x64) Binary 8.59 MiB 8.59 MiB 0 B (0.00%)
vp (Windows x64) gzip -9 3.75 MiB 3.75 MiB +131 B (+0.00%)
NAPI (Windows x64) Binary 26.91 MiB 26.91 MiB 0 B (0.00%)
NAPI (Windows x64) gzip -9 10.69 MiB 10.69 MiB +27 B (+0.00%)
Trampoline (Windows x64) Binary 216.50 KiB 216.50 KiB 0 B (0.00%)
Trampoline (Windows x64) gzip -9 104.34 KiB 104.34 KiB -2 B (-0.00%)
Installer (Windows x64) Binary 4.48 MiB 4.48 MiB 0 B (0.00%)
Installer (Windows x64) gzip -9 2.10 MiB 2.10 MiB -1 B (-0.00%)

@fengmk2

This comment was marked as resolved.

chatgpt-codex-connector[bot]

This comment was marked as resolved.

@fengmk2 fengmk2 self-assigned this Aug 19, 2026
@fengmk2
fengmk2 marked this pull request as ready for review August 19, 2026 02:03
@fengmk2
fengmk2 requested review from naokihaba and nekomoyi August 19, 2026 02:04
@fengmk2 fengmk2 added test: e2e Auto run e2e tests test: install-e2e run vite install e2e test test: create-e2e Run `vp create` e2e tests test: sfw labels Aug 19, 2026
@fengmk2
fengmk2 marked this pull request as draft August 19, 2026 02:41
@fengmk2
fengmk2 marked this pull request as ready for review August 19, 2026 03:26
@fengmk2
fengmk2 requested a review from wan9chi August 19, 2026 03:27

@naokihaba naokihaba left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I will check the rest as soon as I finish my main work. 🙏

@nekomoyi nekomoyi left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Should we add a test where vp env use fails, verifying that the nonzero status is propagated and VP_NODE_VERSION remains unchanged?

@fengmk2

fengmk2 commented Aug 19, 2026

Copy link
Copy Markdown
Member Author

@nekomoyi Added in a0972e7. The Fish snapshot now invokes vp env use --invalid-option, asserts that the wrapper returns a nonzero status, and verifies that VP_NODE_VERSION remains 22.18.0. The deterministic failure diagnostic is also recorded in the snapshot. The focused snapshot, Fish unit tests, cargo fmt, and vp check all pass.

@fengmk2
fengmk2 force-pushed the agent/fish-shell-snapshots branch from a0972e7 to a3bd5de Compare August 19, 2026 14:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

test: create-e2e Run `vp create` e2e tests test: e2e Auto run e2e tests test: install-e2e run vite install e2e test test: sfw

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants