Skip to content

refactor(e2e): scope vscode settings HOME with SetEnvForSpec - #1673

Merged
skevetter merged 1 commit into
mainfrom
refactor/e2e-ide-home-restore
Oct 23, 2026
Merged

skevetter merged 1 commit into
mainfrom
refactor/e2e-ide-home-restore

Conversation

@skevetter

@skevetter skevetter commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • In e2e/tests/ide/vscode_settings.go, use framework.SetEnvForSpec for spec-scoped HOME restoration.
  • Register temp directory removal with ginkgo.DeferCleanup immediately after MkdirTemp.
  • Specs, labels, setup, and assertions are unchanged.

Why

The previous save and restore used os.Getenv and os.Setenv. If HOME was unset before a spec, the restore left it set to an empty string for later specs. SetEnvForSpec preserves the original unset, set-empty, or set-value state.

Validation

  • mise run cli:format: only the target file changed.
  • mise run cli:lint:ci: config verify passed; full-repo run hit its 10m timeout. golangci-lint run --timeout=30m --new-from-patch on ./e2e/...: 0 issues.
  • go vet ./e2e/...: pass; gofmt -l e2e/tests/ide: clean; go test ./e2e/framework/...: pass.
  • Ginkgo dry run selected 7 VS Code Settings specs. Linux container run passed all 7 focused specs.
  • All 93 PR CI jobs passed on dfb346c374322d40204d86848988a893a4fbea08, including Test ide on ubuntu-latest.
  • Local darwin run of the same 7 specs fails on both origin/main and this branch because the existing product code uses the macOS settings path while the spec expects .config. This predates this change.

CodeRabbit CLI

coderabbit review --agent --committed --base-commit f8bc546c completed on the current head with 0 findings.

Size

One Ginkgo spec file changed: 2 insertions and 11 deletions (13 raw spec lines; about 6.5 at half weight). Non-test source lines changed: 0. Framework lines changed: 0.

Review

PR is ready for review. Awaiting GitHub CodeRabbit and Greptile.

Summary by CodeRabbit

  • Tests
    • Improved isolation and cleanup in the VS Code settings test setup, helping ensure temporary environment changes are restored after tests complete.

BeforeEach saved os.Getenv("HOME") and AfterEach restored it with os.Setenv, so an initially unset HOME was leaked back as an empty value. Use framework.SetEnvForSpec, which restores unset and set-empty states exactly, and register temp dir removal with ginkgo.DeferCleanup right after MkdirTemp.
@skevetter skevetter added the e2e-refactor e2e refactor label Oct 23, 2026
@netlify

netlify Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

✅ Deploy Preview for devsydev canceled.

Name Link
🔨 Latest commit dfb346c
🔍 Latest deploy log https://app.netlify.com/projects/devsydev/deploys/6adb06c2e157ca9b994274c7

@coderabbitai

coderabbitai Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: bdc36594-b025-4830-b5c1-6ed6be68e0fa
📥 Commits

Reviewing files that changed from the base of the PR and between f8bc546 and dfb346c.

📒 Files selected for processing: 1
  • e2e/tests/ide/vscode_settings.go

Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.

📝 Walkthrough

The VS Code settings test now defers temporary-home cleanup and sets HOME through framework.SetEnvForSpec. It removes the manual HOME save-and-restore logic and the AfterEach cleanup block.

Changes: VS Code settings test setup

Layer / File(s) Summary
Temporary home and environment setup
e2e/tests/ide/vscode_settings.go
The test defers removal of the temporary home directory and sets HOME through framework.SetEnvForSpec. It removes manual HOME restoration and the AfterEach cleanup block.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~5 minutes

Change: Bug fix

Suggested labels: size/xs

Merge Risk: ⚪ Minimal · up to dfb34

The scoped environment restoration and deferred temporary-directory cleanup are ready to merge.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Description check Warning The description explains the change, motivation, validation results, and known macOS limitation. However, it does not follow the repository template and omits the required Type of change, AI assistanc… Update the description to include the template sections. Select the applicable change type, disclose AI assistance and review status, complete the checklist, state the issue reference or write "none", and include any relevant reviewer notes…
Passed Passed checks: 4 passed
Check name Status Explanation
Title check Passed The title is concise, uses Conventional Commits format, and clearly describes the main change: scoping the VS Code settings HOME environment with SetEnvForSpec.
Docstring Coverage Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1 files.

Full details: Description check

Explanation

The description explains the change, motivation, validation results, and known macOS limitation. However, it does not follow the repository template and omits the required Type of change, AI assistance, Checklist, and Notes sections, along with the issue reference field.

Resolution

Update the description to include the template sections. Select the applicable change type, disclose AI assistance and review status, complete the checklist, state the issue reference or write "none", and include any relevant reviewer notes. Preserve the existing summary, rationale, and testing details under the matching sections.

✨ Finishing Touches
✨ Simplify code
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@netlify

netlify Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

✅ Deploy Preview for images-devsy-sh canceled.

Name Link
🔨 Latest commit dfb346c
🔍 Latest deploy log https://app.netlify.com/projects/images-devsy-sh/deploys/6adb06c2e6544f7827565789

@skevetter
skevetter marked this pull request as ready for review October 23, 2026 09:01
@mergify

mergify Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

This pull request does not currently match the merge queue conditions, so it cannot be queued from here. The box comes back if it matches again.

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Oct 23, 2026 •

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@skevetter
skevetter merged commit f1555f8 into main Oct 23, 2026
114 checks passed
@skevetter
skevetter deleted the refactor/e2e-ide-home-restore branch October 23, 2026 09:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant