Skip to content

test(cmd): clear SOCKGUARD_* before asserting clean pre-validation stderr - #489

Merged
scttbnsn merged 1 commit into
dev/v2.1from
fix/serve-env-injectable
Sep 6, 2026
Merged

test(cmd): clear SOCKGUARD_* before asserting clean pre-validation stderr#489
scttbnsn merged 1 commit into
dev/v2.1from
fix/serve-env-injectable

Conversation

@scttbnsn

@scttbnsn scttbnsn commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

The promotion PR's podman integration legs (rootful and rootless) both failed on one unit test: TestRunServeErrorPaths/validate_before_opening_log_output asserts nothing reaches stderr before validation, and the job exports SOCKGUARD_TEST_PODMAN_SOCKET, which the new unknown-variable warning (#469) correctly reports before validation runs. The Go Test job has no such variable, so dev Verify was green.

The validate-command tests already handle this with clearSockguardEnv; this applies the same call to that subtest. Reproduced locally with the variable exported (fails before, passes after), and the whole internal/cmd package passes both with and without it set.

Changelog

  • 🐛 Fixed TestRunServeErrorPaths/validate_before_opening_log_output by clearing SOCKGUARD_* variables with clearSockguardEnv(t) before the stderr assertion.
  • 🔧 Changed the test to avoid warnings from SOCKGUARD_TEST_PODMAN_SOCKET in Podman integration jobs.

@vercel

vercel Bot commented Sep 6, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
sockguard-website Ready Ready Preview Sep 6, 2026 1:31pm UTC

@biggest-littlest biggest-littlest 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.

Reviewed against the CHANGELOG entry and the diff; CI green outside the qlty/Vercel quota noise.

@ALARGECOMPANY ALARGECOMPANY 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.

Reviewed against the CHANGELOG entry and the diff; CI green outside the qlty/Vercel quota noise.

@scttbnsn
scttbnsn merged commit 742d12e into dev/v2.1 Sep 6, 2026
19 of 22 checks passed
@scttbnsn
scttbnsn deleted the fix/serve-env-injectable branch September 6, 2026 13:31
@coderabbitai

coderabbitai Bot commented Sep 6, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Team

Run ID: e6f7b2ef-4ed2-4041-89d7-63b7d560788c

📥 Commits

Reviewing files that changed from the base of the PR and between a17d8fc and 9138f28.

⛔ Files ignored due to path filters (1)
  • CHANGELOG.md is excluded by !CHANGELOG.md
📒 Files selected for processing (1)
  • app/internal/cmd/serve_run_test.go

📝 Walkthrough

Walkthrough

The “validate before opening log output” test now calls clearSockguardEnv(t) before dependency setup. This removes externally exported SOCKGUARD_* variables before the test checks stderr output.

✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/serve-env-injectable

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.

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.

3 participants