Skip to content

test(env): snapshot list-remote json markers#1970

Closed
jong-kyung wants to merge 1 commit into
voidzero-dev:mainfrom
jong-kyung:test/env-list-remote-snap-markers
Closed

test(env): snapshot list-remote json markers#1970
jong-kyung wants to merge 1 commit into
voidzero-dev:mainfrom
jong-kyung:test/env-list-remote-snap-markers

Conversation

@jong-kyung

Copy link
Copy Markdown
Collaborator

Description

Follow-up to #1907. Preserve the raw vp env list-remote --lts --json output in the global snap test so the JSON marker fields stay covered.

Testing

  • pnpm -F vite-plus snap-test-global command-env-list-remote

@jong-kyung jong-kyung self-assigned this Jun 28, 2026
@netlify

netlify Bot commented Jun 28, 2026

Copy link
Copy Markdown

Deploy Preview for viteplus-preview canceled.

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

@jong-kyung jong-kyung requested a review from fengmk2 June 28, 2026 07:29

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

Copy link
Copy Markdown

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: 746e940403

ℹ️ 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".

"vp env install lts # Install an LTS Node.js version locally",
"vp env default lts # Set it as the global default (stored as the `lts` alias)",
"vp env list-remote --lts --json | node -e \"const {versions}=JSON.parse(require('fs').readFileSync(0,'utf8')); console.log('installed marked:', versions.some(v=>v.installed)); console.log('current marked:', versions.some(v=>v.current)); console.log('default marked:', versions.some(v=>v.default));\" # installed/current/default flags should all resolve, including the `lts` default alias"
"vp env list-remote --lts --json # installed/current/default flags should all resolve, including the `lts` default alias"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Keep list-remote snapshot hermetic

When this snap test runs on a machine whose ~/.vite-plus contains a different set of installed Node versions, or after the remote Node distribution index gains another LTS release, this command will rewrite snap.txt even though the CLI is correct. vp env list-remote fetches the remote version index and annotates every row from local VP_HOME state (installed/current/default), while the snap runner treats the whole stdout as the expected snapshot; the previous pipe asserted only the marker predicates and avoided depending on the full remote/local list.

Useful? React with 👍 / 👎.

@jong-kyung jong-kyung marked this pull request as draft June 28, 2026 07:35
@jong-kyung jong-kyung removed the request for review from fengmk2 June 28, 2026 07:35
@fengmk2

fengmk2 commented Jun 28, 2026

Copy link
Copy Markdown
Member

I tested this before merging, and it caused unstable output. Also, there was too much content. So I'll stick to using scripts for assertion verification.

@fengmk2 fengmk2 closed this Jun 28, 2026
@jong-kyung jong-kyung deleted the test/env-list-remote-snap-markers branch June 28, 2026 07:37
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.

2 participants