fix: redirect telemetry notice to stderr instead of stdout #1426
Conversation
- Updated PrintTelemetryDisclosure() to use PrintfMessageToTUI() which outputs to stderr - Telemetry notices no longer interfere with piped command output - Added test coverage to verify telemetry outputs to stderr - Updated test snapshots to reflect telemetry now appearing in stderr - Resolves DEV-3597 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
…-to-stdout-instead-of-stderr
📝 WalkthroughWalkthroughRoutes telemetry disclosure printing to stderr via the TUI print helper, adds a unit test asserting stderr output, updates many CLI golden snapshots and test expectations to reflect stderr telemetry, relaxes some tests' stderr assertions, adds binary-presence test guards, and updates .gitignore. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant CLI as Atmos CLI
participant Telemetry as pkg/telemetry.PrintTelemetryDisclosure
participant TUI as utils.PrintfMessageToTUI (stderr)
User->>CLI: run command / --help
CLI->>Telemetry: PrintTelemetryDisclosure()
Telemetry->>TUI: Print disclosure ("%s")
Note right of TUI #f0f4ff: Disclosure routed to stderr via TUI helper
CLI-->>User: stdout (help/output) — telemetry omitted
TUI-->>User: stderr (telemetry notice)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
Suggested reviewers
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/snapshots/TestCLICommands_atmos_warns_if_not_in_git_repo_with_no_atmos_config.stdout.golden (1)
1-17: Telemetry Output Found in stdout Snapshots
The verification script uncovered telemetry entries in several.stdout.goldenfiles. Telemetry should only be emitted to stderr, so these snapshots need updating.Affected files and lines:
- tests/snapshots/TestCLICommands_indentation.stdout.golden: line 57 (
telemetry:)- tests/snapshots/TestCLICommands_atmos_describe_configuration.stdout.golden: line 70 (
telemetry:)- tests/snapshots/TestCLICommands_atmos_describe_config_imports.stdout.golden: line 151 (
telemetry:)- tests/snapshots/TestCLICommands_atmos_describe_config_-f_yaml.stdout.golden: line 57 (
telemetry:)- tests/snapshots/TestCLICommands_atmos_describe_config.stdout.golden: line 139 (
"telemetry": {)- tests/snapshots/TestCLICommands_Valid_log_file_in_flag_should_be_priortized_over_env_and_config.stdout.golden: line 8 (
DEBU Telemetry event captured)- tests/snapshots/TestCLICommands_Valid_log_file_in_env_should_be_priortized_over_config.stdout.golden: line 8 (
DEBU Telemetry event captured)Please update these snapshots so that any telemetry-related output is redirected to stderr only, and regenerate the
.stdout.goldenfiles accordingly.
🧹 Nitpick comments (11)
tests/snapshots/TestCLICommands_atmos_warns_if_not_in_git_repo_with_no_atmos_config.stdout.golden (1)
4-5: Empty stacks dir placeholder — please confirm it's expected.Message shows “set to ,”. If that blank is intentional for “no config” scenarios, all good (not blocking). Per prior learnings, we won’t suggest changing snapshot text that mirrors real output.
I can help open a follow-up to improve the runtime message (not the snapshot) if this wasn’t intentional.
tests/snapshots/TestCLICommands_atmos_atlantis_generate_repo-config_help.stderr.golden (1)
3-3: Trim trailing space at EOL (nit).Line ends with a space after the comma; this can cause brittle snapshots. If intentional per snapshot policy, ignore.
tests/snapshots/TestCLICommands_atmos_terraform_--help.stderr.golden (1)
3-3: Trailing space at EOL (nit).Same minor whitespace at the end of the line.
tests/snapshots/TestCLICommands_atmos_atlantis_generate_help.stderr.golden (1)
3-3: Trim EOL space (nit).Minor trailing whitespace here as well.
tests/snapshots/TestCLICommands_atmos_atlantis_generate_repo-config_--help.stderr.golden (1)
3-3: Trailing whitespace (nit).Same note on the space after the comma.
tests/snapshots/TestCLICommands_atmos_doesnt_warn_if_in_git_repo_with_no_atmos_config.stderr.golden (1)
3-3: Minor: remove trailing space (nit).Consistent nit with other snapshots.
tests/snapshots/TestCLICommands_atmos.stdout.golden (1)
2-3: Handle empty vs. missing stacks directory distinctlyCurrently, when the
stacksDirsetting is unset or empty, the CLI emits:CLI config file specifies the directory for Atmos stacks as , but the directory does not exist.That leaves an awkward blank before the comma. To improve clarity and UX:
- If no stacks directory is configured, emit:
CLI config file does not specify the stacks directory.- If a path is configured but doesn’t exist, emit:
CLI config file specifies the directory for Atmos stacks as '<path>', but the directory does not exist.This change is optional but will make the output more readable. All stdout snapshots have been verified to be telemetry-free.
tests/snapshots/TestCLICommands_atmos_atlantis_generate_repo-config.stdout.golden (1)
2-3: Empty stacks directory placeholderThis produces “as , but the directory does not exist.” Consider conditional phrasing for empty values to avoid dangling punctuation and improve clarity.
Happy to propose a small helper that formats this message based on presence/absence of the path.
tests/snapshots/TestCLICommands_atmos_doesnt_warn_if_in_git_repo_with_no_atmos_config.stdout.golden (1)
4-5: Tighten messaging when default stacks dir is unset“The default Atmos stacks directory is set to , but…” reads odd when empty. Suggest: if unset → “Default stacks directory is not configured.” If set-but-missing → include the path.
Want me to open a follow-up PR to centralize this formatting?
pkg/telemetry/utils.go (1)
135-166: Optional: guard against repeat prints when cache is unavailable.If LoadCache/SaveCache fails, disclosure may print more than once per process. Consider a process-level guard (sync.Once or atomic.Bool) to suppress repeats even when persistence fails.
Example:
+var disclosureShownInProcess atomic.Bool + func disclosureMessage() string { + if disclosureShownInProcess.Load() { + return "" + } // Do not show disclosure if running in CI if isCI() { return "" } // Only show disclosure if telemetry is enabled telemetry := getTelemetryFromConfig() if telemetry == nil || !telemetry.isEnabled { return "" } TelemetryDisclosureShown := getOrInitializeCacheValue( func(cfg *cfg.CacheConfig) bool { return cfg.TelemetryDisclosureShown }, func(cfg *cfg.CacheConfig, _ bool) { cfg.TelemetryDisclosureShown = true }, false, ) if TelemetryDisclosureShown { return "" } + disclosureShownInProcess.Store(true) return DisclosureMessage }pkg/telemetry/utils_test.go (1)
544-612: Make the test capture both streams in one call to avoid leaking output; use assert.Contains.Current approach prints to real stderr during the stdout capture phase. Capture both streams in a single invocation and assert with testify helpers for clarity.
Apply:
func TestPrintTelemetryDisclosureOutputsToStderr(t *testing.T) { @@ - // Helper function to capture stdout - captureStdout := func(f func()) string { - old := os.Stdout - r, w, _ := os.Pipe() - os.Stdout = w - - f() - - w.Close() - os.Stdout = old - - var buf bytes.Buffer - io.Copy(&buf, r) - return buf.String() - } - - // Helper function to capture stderr - captureStderr := func(f func()) string { - old := os.Stderr - r, w, _ := os.Pipe() - os.Stderr = w - - f() - - w.Close() - os.Stderr = old - - var buf bytes.Buffer - io.Copy(&buf, r) - return buf.String() - } + // Helper to capture stdout and stderr together to prevent leaking to test logs + captureStreams := func(f func()) (string, string) { + oldOut, oldErr := os.Stdout, os.Stderr + rOut, wOut, _ := os.Pipe() + rErr, wErr, _ := os.Pipe() + os.Stdout, os.Stderr = wOut, wErr + + var stdoutBuf, stderrBuf bytes.Buffer + done := make(chan struct{}, 2) + go func() { io.Copy(&stdoutBuf, rOut); rOut.Close(); done <- struct{}{} }() + go func() { io.Copy(&stderrBuf, rErr); rErr.Close(); done <- struct{}{} }() + + f() + + wOut.Close() + wErr.Close() + os.Stdout, os.Stderr = oldOut, oldErr + <-done + <-done + return stdoutBuf.String(), stderrBuf.String() + } @@ - // Capture stdout - should be empty - stdoutOutput := captureStdout(func() { - PrintTelemetryDisclosure() - }) - - // Reset cache for stderr test - cacheCfg.TelemetryDisclosureShown = false - cfg.SaveCache(cacheCfg) - - // Capture stderr - should contain the disclosure message - stderrOutput := captureStderr(func() { - PrintTelemetryDisclosure() - }) + // Capture both streams in a single call + stdoutOutput, stderrOutput := captureStreams(func() { + PrintTelemetryDisclosure() + }) @@ - assert.Empty(t, stdoutOutput, "Telemetry disclosure should not write to stdout") + assert.Empty(t, stdoutOutput, "Telemetry disclosure should not write to stdout") @@ - assert.True(t, strings.Contains(stderrOutput, "Notice: Atmos now collects"), - "Telemetry disclosure should write to stderr") + assert.Contains(t, stderrOutput, "Notice: Atmos now collects", + "Telemetry disclosure should write to stderr") }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (54)
pkg/telemetry/utils.go(1 hunks)pkg/telemetry/utils_test.go(2 hunks)tests/snapshots/TestCLICommands_atmos.stderr.golden(1 hunks)tests/snapshots/TestCLICommands_atmos.stdout.golden(1 hunks)tests/snapshots/TestCLICommands_atmos_--help.stderr.golden(1 hunks)tests/snapshots/TestCLICommands_atmos_--help.stdout.golden(0 hunks)tests/snapshots/TestCLICommands_atmos_about_--help.stderr.golden(1 hunks)tests/snapshots/TestCLICommands_atmos_about_--help.stdout.golden(0 hunks)tests/snapshots/TestCLICommands_atmos_atlantis_--help.stderr.golden(1 hunks)tests/snapshots/TestCLICommands_atmos_atlantis_--help.stdout.golden(0 hunks)tests/snapshots/TestCLICommands_atmos_atlantis_generate_--help.stderr.golden(1 hunks)tests/snapshots/TestCLICommands_atmos_atlantis_generate_--help.stdout.golden(0 hunks)tests/snapshots/TestCLICommands_atmos_atlantis_generate_help.stderr.golden(1 hunks)tests/snapshots/TestCLICommands_atmos_atlantis_generate_help.stdout.golden(0 hunks)tests/snapshots/TestCLICommands_atmos_atlantis_generate_repo-config.stderr.golden(1 hunks)tests/snapshots/TestCLICommands_atmos_atlantis_generate_repo-config.stdout.golden(1 hunks)tests/snapshots/TestCLICommands_atmos_atlantis_generate_repo-config_--help.stderr.golden(1 hunks)tests/snapshots/TestCLICommands_atmos_atlantis_generate_repo-config_--help.stdout.golden(0 hunks)tests/snapshots/TestCLICommands_atmos_atlantis_generate_repo-config_help.stderr.golden(1 hunks)tests/snapshots/TestCLICommands_atmos_atlantis_generate_repo-config_help.stdout.golden(0 hunks)tests/snapshots/TestCLICommands_atmos_atlantis_help.stderr.golden(1 hunks)tests/snapshots/TestCLICommands_atmos_atlantis_help.stdout.golden(0 hunks)tests/snapshots/TestCLICommands_atmos_doesnt_warn_if_in_git_repo_with_no_atmos_config.stderr.golden(1 hunks)tests/snapshots/TestCLICommands_atmos_doesnt_warn_if_in_git_repo_with_no_atmos_config.stdout.golden(1 hunks)tests/snapshots/TestCLICommands_atmos_helmfile_--help.stderr.golden(1 hunks)tests/snapshots/TestCLICommands_atmos_helmfile_--help.stdout.golden(0 hunks)tests/snapshots/TestCLICommands_atmos_helmfile_apply_--help.stderr.golden(1 hunks)tests/snapshots/TestCLICommands_atmos_helmfile_apply_--help.stdout.golden(0 hunks)tests/snapshots/TestCLICommands_atmos_helmfile_apply_help.stderr.golden(1 hunks)tests/snapshots/TestCLICommands_atmos_helmfile_apply_help.stdout.golden(0 hunks)tests/snapshots/TestCLICommands_atmos_helmfile_help.stderr.golden(1 hunks)tests/snapshots/TestCLICommands_atmos_helmfile_help.stdout.golden(0 hunks)tests/snapshots/TestCLICommands_atmos_terraform_--help.stderr.golden(1 hunks)tests/snapshots/TestCLICommands_atmos_terraform_--help.stdout.golden(0 hunks)tests/snapshots/TestCLICommands_atmos_terraform_--help_alias_subcommand_check.stderr.golden(1 hunks)tests/snapshots/TestCLICommands_atmos_terraform_--help_alias_subcommand_check.stdout.golden(0 hunks)tests/snapshots/TestCLICommands_atmos_terraform_apply_--help.stderr.golden(1 hunks)tests/snapshots/TestCLICommands_atmos_terraform_apply_--help.stdout.golden(0 hunks)tests/snapshots/TestCLICommands_atmos_terraform_apply_help.stderr.golden(1 hunks)tests/snapshots/TestCLICommands_atmos_terraform_apply_help.stdout.golden(0 hunks)tests/snapshots/TestCLICommands_atmos_terraform_help.stderr.golden(1 hunks)tests/snapshots/TestCLICommands_atmos_terraform_help.stdout.golden(0 hunks)tests/snapshots/TestCLICommands_atmos_terraform_plan_non-existent_in_non_workspace.stderr.golden(1 hunks)tests/snapshots/TestCLICommands_atmos_terraform_plan_non-existent_in_non_workspace.stdout.golden(1 hunks)tests/snapshots/TestCLICommands_atmos_validate_editorconfig_--help.stderr.golden(1 hunks)tests/snapshots/TestCLICommands_atmos_validate_editorconfig_--help.stdout.golden(0 hunks)tests/snapshots/TestCLICommands_atmos_validate_editorconfig_help.stderr.golden(1 hunks)tests/snapshots/TestCLICommands_atmos_validate_editorconfig_help.stdout.golden(0 hunks)tests/snapshots/TestCLICommands_atmos_warns_if_not_in_git_repo_with_no_atmos_config.stderr.golden(1 hunks)tests/snapshots/TestCLICommands_atmos_warns_if_not_in_git_repo_with_no_atmos_config.stdout.golden(1 hunks)tests/snapshots/TestCLICommands_check_atmos_in_empty-dir.stderr.golden(1 hunks)tests/snapshots/TestCLICommands_check_atmos_in_empty-dir.stdout.golden(0 hunks)tests/test-cases/demo-stacks.yaml(0 hunks)tests/test-cases/empty-dir.yaml(0 hunks)
💤 Files with no reviewable changes (22)
- tests/snapshots/TestCLICommands_atmos_atlantis_generate_repo-config_help.stdout.golden
- tests/snapshots/TestCLICommands_atmos_helmfile_apply_help.stdout.golden
- tests/snapshots/TestCLICommands_atmos_helmfile_help.stdout.golden
- tests/test-cases/demo-stacks.yaml
- tests/snapshots/TestCLICommands_atmos_terraform_--help_alias_subcommand_check.stdout.golden
- tests/snapshots/TestCLICommands_atmos_terraform_apply_help.stdout.golden
- tests/snapshots/TestCLICommands_atmos_atlantis_generate_--help.stdout.golden
- tests/snapshots/TestCLICommands_atmos_helmfile_--help.stdout.golden
- tests/snapshots/TestCLICommands_atmos_validate_editorconfig_--help.stdout.golden
- tests/test-cases/empty-dir.yaml
- tests/snapshots/TestCLICommands_atmos_--help.stdout.golden
- tests/snapshots/TestCLICommands_atmos_atlantis_generate_repo-config_--help.stdout.golden
- tests/snapshots/TestCLICommands_atmos_about_--help.stdout.golden
- tests/snapshots/TestCLICommands_atmos_terraform_help.stdout.golden
- tests/snapshots/TestCLICommands_atmos_atlantis_generate_help.stdout.golden
- tests/snapshots/TestCLICommands_atmos_terraform_apply_--help.stdout.golden
- tests/snapshots/TestCLICommands_atmos_atlantis_--help.stdout.golden
- tests/snapshots/TestCLICommands_atmos_atlantis_help.stdout.golden
- tests/snapshots/TestCLICommands_atmos_validate_editorconfig_help.stdout.golden
- tests/snapshots/TestCLICommands_atmos_helmfile_apply_--help.stdout.golden
- tests/snapshots/TestCLICommands_atmos_terraform_--help.stdout.golden
- tests/snapshots/TestCLICommands_check_atmos_in_empty-dir.stdout.golden
🧰 Additional context used
📓 Path-based instructions (2)
**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*.go: Use Viper for managing configuration, environment variables, and flags
Use interfaces for external dependencies to facilitate mocking
All code must pass golangci-lint checks
Follow Go's error handling idioms
Use meaningful error messages
Wrap errors with context using fmt.Errorf("context: %w", err)
Consider using a custom error type for domain-specific errors
Follow standard Go coding style
Use gofmt and goimports to format code
Prefer short, descriptive variable names
Use snake_case for environment variables
Document all exported functions, types, and methods
Document complex logic with inline comments
Follow Go's documentation conventions
Use Viper for configuration management
Support configuration via files, environment variables, and flags
Follow the precedence order: flags > environment variables > config file > defaults
Files:
pkg/telemetry/utils.gopkg/telemetry/utils_test.go
**/*_test.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*_test.go: Every new feature must include comprehensive unit tests
Test both happy paths and error conditions
Use table-driven tests for testing multiple scenarios
Include integration tests for command flows
Test CLI end-to-end when possible
Use test fixtures for complex inputs
Consider using testify/mock for creating mock implementations
Files:
pkg/telemetry/utils_test.go
🧠 Learnings (17)
📚 Learning: 2025-01-19T15:49:15.593Z
Learnt from: samtholiya
PR: cloudposse/atmos#955
File: tests/snapshots/TestCLICommands_atmos_validate_editorconfig_--help.stdout.golden:0-0
Timestamp: 2025-01-19T15:49:15.593Z
Learning: In future commits, the help text for Atmos CLI commands should be limited to only show component and stack parameters for commands that actually use them. This applies to the example usage section in command help text.
Applied to files:
tests/snapshots/TestCLICommands_atmos_about_--help.stderr.goldentests/snapshots/TestCLICommands_atmos_terraform_apply_help.stderr.goldentests/snapshots/TestCLICommands_atmos_helmfile_apply_--help.stderr.goldentests/snapshots/TestCLICommands_atmos_atlantis_generate_repo-config_--help.stderr.goldentests/snapshots/TestCLICommands_atmos_terraform_--help_alias_subcommand_check.stderr.goldentests/snapshots/TestCLICommands_atmos_validate_editorconfig_--help.stderr.goldentests/snapshots/TestCLICommands_atmos_terraform_apply_--help.stderr.goldentests/snapshots/TestCLICommands_atmos_helmfile_apply_help.stderr.goldentests/snapshots/TestCLICommands_atmos_doesnt_warn_if_in_git_repo_with_no_atmos_config.stderr.goldentests/snapshots/TestCLICommands_atmos_atlantis_generate_repo-config.stderr.goldentests/snapshots/TestCLICommands_atmos_terraform_help.stderr.goldentests/snapshots/TestCLICommands_atmos_atlantis_help.stderr.goldentests/snapshots/TestCLICommands_atmos_terraform_plan_non-existent_in_non_workspace.stderr.goldentests/snapshots/TestCLICommands_atmos_helmfile_--help.stderr.goldentests/snapshots/TestCLICommands_atmos_atlantis_--help.stderr.goldentests/snapshots/TestCLICommands_atmos.stderr.goldentests/snapshots/TestCLICommands_atmos_atlantis_generate_repo-config.stdout.goldentests/snapshots/TestCLICommands_atmos.stdout.goldentests/snapshots/TestCLICommands_atmos_atlantis_generate_--help.stderr.goldentests/snapshots/TestCLICommands_atmos_warns_if_not_in_git_repo_with_no_atmos_config.stdout.goldentests/snapshots/TestCLICommands_atmos_terraform_--help.stderr.goldentests/snapshots/TestCLICommands_atmos_terraform_plan_non-existent_in_non_workspace.stdout.goldentests/snapshots/TestCLICommands_atmos_--help.stderr.goldentests/snapshots/TestCLICommands_atmos_doesnt_warn_if_in_git_repo_with_no_atmos_config.stdout.goldentests/snapshots/TestCLICommands_atmos_warns_if_not_in_git_repo_with_no_atmos_config.stderr.golden
📚 Learning: 2025-03-18T12:26:25.329Z
Learnt from: Listener430
PR: cloudposse/atmos#1149
File: tests/snapshots/TestCLICommands_atmos_vendor_pull_ssh.stderr.golden:7-7
Timestamp: 2025-03-18T12:26:25.329Z
Learning: In the Atmos project, typos or inconsistencies in test snapshot files (such as "terrafrom" instead of "terraform") may be intentional as they capture the exact output of commands and should not be flagged as issues requiring correction.
Applied to files:
tests/snapshots/TestCLICommands_atmos_about_--help.stderr.goldentests/snapshots/TestCLICommands_atmos_terraform_apply_help.stderr.goldentests/snapshots/TestCLICommands_atmos_helmfile_apply_--help.stderr.goldentests/snapshots/TestCLICommands_atmos_atlantis_generate_repo-config_help.stderr.goldentests/snapshots/TestCLICommands_atmos_terraform_--help_alias_subcommand_check.stderr.goldentests/snapshots/TestCLICommands_atmos_validate_editorconfig_--help.stderr.goldentests/snapshots/TestCLICommands_atmos_terraform_apply_--help.stderr.goldentests/snapshots/TestCLICommands_atmos_helmfile_apply_help.stderr.goldentests/snapshots/TestCLICommands_atmos_doesnt_warn_if_in_git_repo_with_no_atmos_config.stderr.goldentests/snapshots/TestCLICommands_atmos_atlantis_generate_repo-config.stderr.goldentests/snapshots/TestCLICommands_atmos_terraform_help.stderr.goldentests/snapshots/TestCLICommands_atmos_helmfile_help.stderr.goldentests/snapshots/TestCLICommands_atmos_atlantis_help.stderr.goldentests/snapshots/TestCLICommands_atmos_terraform_plan_non-existent_in_non_workspace.stderr.goldentests/snapshots/TestCLICommands_atmos_helmfile_--help.stderr.goldentests/snapshots/TestCLICommands_check_atmos_in_empty-dir.stderr.goldentests/snapshots/TestCLICommands_atmos_atlantis_--help.stderr.goldentests/snapshots/TestCLICommands_atmos.stderr.goldentests/snapshots/TestCLICommands_atmos_atlantis_generate_repo-config.stdout.goldentests/snapshots/TestCLICommands_atmos.stdout.goldentests/snapshots/TestCLICommands_atmos_atlantis_generate_--help.stderr.goldentests/snapshots/TestCLICommands_atmos_warns_if_not_in_git_repo_with_no_atmos_config.stdout.goldentests/snapshots/TestCLICommands_atmos_terraform_--help.stderr.goldentests/snapshots/TestCLICommands_atmos_terraform_plan_non-existent_in_non_workspace.stdout.goldentests/snapshots/TestCLICommands_atmos_--help.stderr.goldentests/snapshots/TestCLICommands_atmos_validate_editorconfig_help.stderr.goldentests/snapshots/TestCLICommands_atmos_doesnt_warn_if_in_git_repo_with_no_atmos_config.stdout.goldentests/snapshots/TestCLICommands_atmos_warns_if_not_in_git_repo_with_no_atmos_config.stderr.golden
📚 Learning: 2025-02-14T23:12:38.030Z
Learnt from: Listener430
PR: cloudposse/atmos#1061
File: tests/snapshots/TestCLICommands_atmos_vendor_pull_ssh.stderr.golden:8-8
Timestamp: 2025-02-14T23:12:38.030Z
Learning: Test snapshots in the Atmos project, particularly for dry run scenarios, may be updated during the development process, and temporary inconsistencies in their content should not be flagged as issues.
Applied to files:
tests/snapshots/TestCLICommands_atmos_about_--help.stderr.goldentests/snapshots/TestCLICommands_atmos_terraform_apply_help.stderr.goldentests/snapshots/TestCLICommands_atmos_terraform_--help_alias_subcommand_check.stderr.goldentests/snapshots/TestCLICommands_atmos_validate_editorconfig_--help.stderr.goldentests/snapshots/TestCLICommands_atmos_helmfile_apply_help.stderr.goldentests/snapshots/TestCLICommands_atmos_doesnt_warn_if_in_git_repo_with_no_atmos_config.stderr.goldentests/snapshots/TestCLICommands_atmos_terraform_help.stderr.goldentests/snapshots/TestCLICommands_atmos_helmfile_help.stderr.goldentests/snapshots/TestCLICommands_atmos_atlantis_help.stderr.goldentests/snapshots/TestCLICommands_atmos_terraform_plan_non-existent_in_non_workspace.stderr.goldentests/snapshots/TestCLICommands_atmos_helmfile_--help.stderr.goldentests/snapshots/TestCLICommands_check_atmos_in_empty-dir.stderr.goldentests/snapshots/TestCLICommands_atmos.stderr.goldentests/snapshots/TestCLICommands_atmos.stdout.goldentests/snapshots/TestCLICommands_atmos_atlantis_generate_--help.stderr.goldentests/snapshots/TestCLICommands_atmos_warns_if_not_in_git_repo_with_no_atmos_config.stdout.goldentests/snapshots/TestCLICommands_atmos_terraform_plan_non-existent_in_non_workspace.stdout.goldentests/snapshots/TestCLICommands_atmos_validate_editorconfig_help.stderr.goldentests/snapshots/TestCLICommands_atmos_doesnt_warn_if_in_git_repo_with_no_atmos_config.stdout.goldentests/snapshots/TestCLICommands_atmos_warns_if_not_in_git_repo_with_no_atmos_config.stderr.golden
📚 Learning: 2025-01-30T19:30:59.120Z
Learnt from: samtholiya
PR: cloudposse/atmos#959
File: cmd/workflow.go:74-74
Timestamp: 2025-01-30T19:30:59.120Z
Learning: Error handling for `cmd.Usage()` is not required in the Atmos CLI codebase, as confirmed by the maintainer.
Applied to files:
tests/snapshots/TestCLICommands_atmos_about_--help.stderr.goldentests/snapshots/TestCLICommands_atmos_terraform_apply_help.stderr.goldentests/snapshots/TestCLICommands_atmos_helmfile_apply_--help.stderr.goldentests/snapshots/TestCLICommands_atmos_atlantis_generate_repo-config_--help.stderr.goldentests/snapshots/TestCLICommands_atmos_atlantis_generate_repo-config_help.stderr.goldentests/snapshots/TestCLICommands_atmos_terraform_--help_alias_subcommand_check.stderr.goldentests/snapshots/TestCLICommands_atmos_validate_editorconfig_--help.stderr.goldentests/snapshots/TestCLICommands_atmos_terraform_apply_--help.stderr.goldentests/snapshots/TestCLICommands_atmos_helmfile_apply_help.stderr.goldentests/snapshots/TestCLICommands_atmos_atlantis_generate_help.stderr.goldentests/snapshots/TestCLICommands_atmos_terraform_help.stderr.goldentests/snapshots/TestCLICommands_atmos_helmfile_help.stderr.goldentests/snapshots/TestCLICommands_atmos_atlantis_help.stderr.goldentests/snapshots/TestCLICommands_atmos_helmfile_--help.stderr.goldentests/snapshots/TestCLICommands_check_atmos_in_empty-dir.stderr.goldentests/snapshots/TestCLICommands_atmos_atlantis_--help.stderr.goldentests/snapshots/TestCLICommands_atmos_atlantis_generate_--help.stderr.goldentests/snapshots/TestCLICommands_atmos_warns_if_not_in_git_repo_with_no_atmos_config.stdout.goldentests/snapshots/TestCLICommands_atmos_terraform_--help.stderr.goldentests/snapshots/TestCLICommands_atmos_--help.stderr.goldentests/snapshots/TestCLICommands_atmos_validate_editorconfig_help.stderr.goldentests/snapshots/TestCLICommands_atmos_doesnt_warn_if_in_git_repo_with_no_atmos_config.stdout.golden
📚 Learning: 2024-12-07T16:16:13.038Z
Learnt from: Listener430
PR: cloudposse/atmos#825
File: internal/exec/helmfile_generate_varfile.go:28-31
Timestamp: 2024-12-07T16:16:13.038Z
Learning: In `internal/exec/helmfile_generate_varfile.go`, the `--help` command (`./atmos helmfile generate varfile --help`) works correctly without requiring stack configurations, and the only change needed was to make `ProcessCommandLineArgs` exportable by capitalizing its name.
Applied to files:
tests/snapshots/TestCLICommands_atmos_terraform_apply_help.stderr.goldentests/snapshots/TestCLICommands_atmos_helmfile_apply_--help.stderr.goldentests/snapshots/TestCLICommands_atmos_terraform_--help_alias_subcommand_check.stderr.goldentests/snapshots/TestCLICommands_atmos_helmfile_apply_help.stderr.goldentests/snapshots/TestCLICommands_atmos_helmfile_help.stderr.goldentests/snapshots/TestCLICommands_atmos_helmfile_--help.stderr.goldentests/snapshots/TestCLICommands_atmos_warns_if_not_in_git_repo_with_no_atmos_config.stdout.goldentests/snapshots/TestCLICommands_atmos_--help.stderr.golden
📚 Learning: 2025-07-01T02:22:25.901Z
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: Ensure consistency between CLI help text and website documentation
Applied to files:
tests/snapshots/TestCLICommands_atmos_atlantis_generate_repo-config_--help.stderr.golden
📚 Learning: 2025-07-05T20:59:02.914Z
Learnt from: aknysh
PR: cloudposse/atmos#1363
File: internal/exec/template_utils.go:18-18
Timestamp: 2025-07-05T20:59:02.914Z
Learning: In the Atmos project, gomplate v4 is imported with a blank import (`_ "github.com/hairyhenderson/gomplate/v4"`) alongside v3 imports to resolve AWS SDK version conflicts. V3 uses older AWS SDK versions that conflict with newer AWS modules used by Atmos. A full migration to v4 requires extensive refactoring due to API changes and should be handled in a separate PR.
Applied to files:
pkg/telemetry/utils.go
📚 Learning: 2025-02-21T20:56:05.539Z
Learnt from: samtholiya
PR: cloudposse/atmos#1077
File: pkg/downloader/custom_github_detector.go:0-0
Timestamp: 2025-02-21T20:56:05.539Z
Learning: The `github.com/charmbracelet/log` package should be imported with the alias `log` according to the project's import alias configuration.
Applied to files:
pkg/telemetry/utils.go
📚 Learning: 2024-11-25T17:17:15.703Z
Learnt from: RoseSecurity
PR: cloudposse/atmos#797
File: pkg/list/atmos.yaml:213-214
Timestamp: 2024-11-25T17:17:15.703Z
Learning: The file `pkg/list/atmos.yaml` is primarily intended for testing purposes.
Applied to files:
tests/snapshots/TestCLICommands_atmos_doesnt_warn_if_in_git_repo_with_no_atmos_config.stderr.goldentests/snapshots/TestCLICommands_atmos_atlantis_generate_repo-config.stderr.goldentests/snapshots/TestCLICommands_atmos_terraform_plan_non-existent_in_non_workspace.stderr.goldentests/snapshots/TestCLICommands_atmos.stderr.goldentests/snapshots/TestCLICommands_atmos_atlantis_generate_repo-config.stdout.goldentests/snapshots/TestCLICommands_atmos.stdout.goldentests/snapshots/TestCLICommands_atmos_warns_if_not_in_git_repo_with_no_atmos_config.stdout.goldentests/snapshots/TestCLICommands_atmos_terraform_plan_non-existent_in_non_workspace.stdout.goldentests/snapshots/TestCLICommands_atmos_warns_if_not_in_git_repo_with_no_atmos_config.stderr.golden
📚 Learning: 2025-01-25T15:21:40.413Z
Learnt from: osterman
PR: cloudposse/atmos#808
File: examples/demo-atmos-cli-imports/atmos.yaml:8-8
Timestamp: 2025-01-25T15:21:40.413Z
Learning: In Atmos, when a directory is specified for configuration loading (e.g., in the `import` section of atmos.yaml), all files within that directory should be treated as Atmos configurations. Do not suggest restricting file extensions in directory-based glob patterns.
Applied to files:
tests/snapshots/TestCLICommands_atmos_atlantis_generate_repo-config.stdout.goldentests/snapshots/TestCLICommands_atmos.stdout.goldentests/snapshots/TestCLICommands_atmos_terraform_plan_non-existent_in_non_workspace.stdout.golden
📚 Learning: 2024-12-11T18:40:12.808Z
Learnt from: Listener430
PR: cloudposse/atmos#844
File: cmd/helmfile.go:37-37
Timestamp: 2024-12-11T18:40:12.808Z
Learning: In the atmos project, `cliConfig` is initialized within the `cmd` package in `root.go` and can be used in other command files.
Applied to files:
tests/snapshots/TestCLICommands_atmos_atlantis_generate_repo-config.stdout.goldentests/snapshots/TestCLICommands_atmos.stdout.goldentests/snapshots/TestCLICommands_atmos_warns_if_not_in_git_repo_with_no_atmos_config.stdout.goldentests/snapshots/TestCLICommands_atmos_terraform_plan_non-existent_in_non_workspace.stdout.goldentests/snapshots/TestCLICommands_atmos_doesnt_warn_if_in_git_repo_with_no_atmos_config.stdout.goldentests/snapshots/TestCLICommands_atmos_warns_if_not_in_git_repo_with_no_atmos_config.stderr.golden
📚 Learning: 2024-10-20T13:12:46.499Z
Learnt from: haitham911
PR: cloudposse/atmos#736
File: pkg/config/const.go:6-6
Timestamp: 2024-10-20T13:12:46.499Z
Learning: In `cmd/cmd_utils.go`, it's acceptable to have hardcoded references to `atmos.yaml` in logs, and it's not necessary to update them to use the `CliConfigFileName` constant.
Applied to files:
tests/snapshots/TestCLICommands_atmos_warns_if_not_in_git_repo_with_no_atmos_config.stdout.goldentests/snapshots/TestCLICommands_atmos_doesnt_warn_if_in_git_repo_with_no_atmos_config.stdout.golden
📚 Learning: 2024-10-23T21:36:40.262Z
Learnt from: osterman
PR: cloudposse/atmos#740
File: cmd/cmd_utils.go:340-359
Timestamp: 2024-10-23T21:36:40.262Z
Learning: In the Go codebase for Atmos, when reviewing functions like `checkAtmosConfig` in `cmd/cmd_utils.go`, avoid suggesting refactoring to return errors instead of calling `os.Exit` if such changes would significantly increase the scope due to the need to update multiple call sites.
Applied to files:
tests/snapshots/TestCLICommands_atmos_warns_if_not_in_git_repo_with_no_atmos_config.stdout.goldentests/snapshots/TestCLICommands_atmos_doesnt_warn_if_in_git_repo_with_no_atmos_config.stdout.golden
📚 Learning: 2025-02-03T05:57:18.407Z
Learnt from: samtholiya
PR: cloudposse/atmos#959
File: cmd/cmd_utils.go:121-148
Timestamp: 2025-02-03T05:57:18.407Z
Learning: The Atmos CLI should fail fast (exit) when encountering configuration errors, including command alias configuration issues, to prevent undefined behavior. Use LogErrorAndExit instead of returning errors in such cases.
Applied to files:
tests/snapshots/TestCLICommands_atmos_warns_if_not_in_git_repo_with_no_atmos_config.stdout.goldentests/snapshots/TestCLICommands_atmos_doesnt_warn_if_in_git_repo_with_no_atmos_config.stdout.golden
📚 Learning: 2024-12-01T00:33:20.298Z
Learnt from: aknysh
PR: cloudposse/atmos#810
File: examples/tests/stacks/catalog/terraform/template-functions-test2/defaults.yaml:28-32
Timestamp: 2024-12-01T00:33:20.298Z
Learning: In `examples/tests/stacks/catalog/terraform/template-functions-test2/defaults.yaml`, `!exec atmos terraform output` is used in examples to demonstrate its usage, even though `!terraform.output` is the recommended approach according to the documentation.
Applied to files:
tests/snapshots/TestCLICommands_atmos_terraform_plan_non-existent_in_non_workspace.stdout.golden
📚 Learning: 2024-12-02T21:26:32.337Z
Learnt from: osterman
PR: cloudposse/atmos#808
File: pkg/config/config.go:478-483
Timestamp: 2024-12-02T21:26:32.337Z
Learning: In the 'atmos' project, when reviewing Go code like `pkg/config/config.go`, avoid suggesting file size checks after downloading remote configs if such checks aren't implemented elsewhere in the codebase.
Applied to files:
tests/snapshots/TestCLICommands_atmos_doesnt_warn_if_in_git_repo_with_no_atmos_config.stdout.golden
📚 Learning: 2025-02-18T13:13:11.497Z
Learnt from: samtholiya
PR: cloudposse/atmos#1068
File: tests/snapshots/TestCLICommands_atmos_terraform_help.stdout.golden:59-64
Timestamp: 2025-02-18T13:13:11.497Z
Learning: For Atmos CLI help text, angle brackets in command examples and flag descriptions should be escaped using HTML entities (e.g., `<component>`) rather than converted to backticks or other markdown formatting.
Applied to files:
tests/snapshots/TestCLICommands_atmos_doesnt_warn_if_in_git_repo_with_no_atmos_config.stdout.golden
🧬 Code graph analysis (2)
pkg/telemetry/utils.go (1)
pkg/utils/log_utils.go (1)
PrintfMessageToTUI(29-31)
pkg/telemetry/utils_test.go (3)
pkg/telemetry/ci.go (2)
PreserveCIEnvVars(86-115)RestoreCIEnvVars(124-129)pkg/config/cache.go (2)
LoadCache(49-70)SaveCache(90-104)pkg/telemetry/utils.go (1)
PrintTelemetryDisclosure(54-58)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Build (windows-latest, windows)
- GitHub Check: Analyze (go)
- GitHub Check: Lint (golangci)
- GitHub Check: Summary
🔇 Additional comments (41)
tests/snapshots/TestCLICommands_atmos_warns_if_not_in_git_repo_with_no_atmos_config.stdout.golden (1)
2-2: Stdout no longer includes telemetry — aligned with DEV-3597.This keeps piped output clean. Nice.
tests/snapshots/TestCLICommands_atmos_terraform_plan_non-existent_in_non_workspace.stdout.golden (1)
1-15: Telemetry now absent from stdout — aligns with PR goalThis snapshot no longer includes the telemetry notice on stdout. Matches the stderr redirection intent.
tests/snapshots/TestCLICommands_atmos_atlantis_generate_repo-config_help.stderr.golden (2)
1-4: Telemetry notice correctly routed to stderr.Snapshot reflects the move from stdout to stderr and aligns with the PR intent.
1-5: Telemetry notice placement verifiedAll snapshot tests now conform: no telemetry notice appears in any
*stdout.golden, and the notice remains correctly in all*--help.stderr.golden. No further changes needed here.tests/snapshots/TestCLICommands_atmos_terraform_--help.stderr.golden (1)
1-4: LGTM: help stderr snapshot updated with telemetry block.Matches the new stderr routing; no other diffs apparent.
tests/snapshots/TestCLICommands_atmos_atlantis_generate_help.stderr.golden (1)
1-4: LGTM: telemetry disclosure now on stderr for this help path.Consistent with the new behavior.
tests/snapshots/TestCLICommands_atmos_atlantis_generate_repo-config_--help.stderr.golden (1)
1-4: LGTM: stderr snapshot includes telemetry notice.Behavior aligns with routing change.
tests/snapshots/TestCLICommands_atmos_doesnt_warn_if_in_git_repo_with_no_atmos_config.stderr.golden (2)
1-5: LGTM: telemetry block added to stderr.Placement at the top with a separating blank line looks right.
6-9: Non-telemetry headers correctly belong on stderrI checked the snapshots for this test case and found:
- No occurrences of the help headers (“Atmos CLI Configuration”, “Atmos Components”, “Atmos Stacks”, “Quick Start”) in the
.stdout.goldenfile.- All of those headers appear only in the
.stderr.goldenfile, alongside the telemetry notice.This confirms that the help output (including the new section headers) is intentionally being emitted on stderr for this invocation, not regressed from stdout. Everything looks expected here.
tests/snapshots/TestCLICommands_atmos_validate_editorconfig_--help.stderr.golden (2)
1-5: Telemetry notice now on stderr — snapshot update looks correct.Matches PR intent and keeps stdout clean. No issues.
1-5: No ANSI escape codes found in stderr snapshots
The ripgrep scan returned no matches for\x1b\[, confirming there are no accidental ANSI codes in any*stderr.goldenfiles. All clear!tests/snapshots/TestCLICommands_atmos_terraform_help.stderr.golden (1)
1-5: LGTM: Help stderr now includes telemetry disclosure.Aligned with redirect to stderr; content matches other snapshots.
tests/snapshots/TestCLICommands_atmos.stderr.golden (2)
1-5: LGTM: Telemetry disclosure captured on stderr.Consistent with new output routing.
6-9: Confirm newline betweenatmos.yamlandAtmos CLI Configuration:headingsI’ve checked and this concatenated heading appears consistently across all related snapshots:
- tests/snapshots/TestCLICommands_atmos_warns_if_not_in_git_repo_with_no_atmos_config.stderr.golden (line 7)
- tests/snapshots/TestCLICommands_atmos_terraform_plan_non-existent_in_non_workspace.stderr.golden (line 6)
- tests/snapshots/TestCLICommands_atmos_doesnt_warn_if_in_git_repo_with_no_atmos_config.stderr.golden (line 6)
- tests/snapshots/TestCLICommands_atmos_atlantis_generate_repo-config.stderr.golden (line 6)
- tests/snapshots/TestCLICommands_atmos.stderr.golden (line 6)
Please confirm whether this merged “atmos.yamlstacksAtmos CLI Configuration:” is intended. If not, we should add a newline (or space) after the config filename so the heading renders on its own line.
tests/snapshots/TestCLICommands_atmos_terraform_--help_alias_subcommand_check.stderr.golden (1)
1-5: LGTM: Alias help stderr includes telemetry notice.Change aligns with stderr-only disclosure.
tests/snapshots/TestCLICommands_atmos_atlantis_--help.stderr.golden (1)
1-5: LGTM: Atlantis help stderr includes telemetry notice.Consistent with the rest of the snapshots.
tests/snapshots/TestCLICommands_check_atmos_in_empty-dir.stderr.golden (2)
1-5: Telemetry disclosure correctly moved to stderr.Snapshot aligns with the PR objective and prevents contaminating piped stdout.
1-5: Telemetry text placement verifiedThe grep results confirm the telemetry notice appears in stderr snapshots only (≥1 occurrence), with zero occurrences in stdout snapshots. No further changes are needed.
tests/snapshots/TestCLICommands_atmos_--help.stderr.golden (1)
1-5: Help output snapshot reflects stderr routing for telemetry.Looks consistent with the new behavior.
tests/snapshots/TestCLICommands_atmos_terraform_apply_--help.stderr.golden (1)
1-5: Apply help snapshot updated for stderr telemetry.Change is correct and scoped to stderr.
tests/snapshots/TestCLICommands_atmos_helmfile_apply_help.stderr.golden (1)
1-5: Helmfile apply help snapshot updated appropriately.Matches the stderr-only disclosure intent.
tests/snapshots/TestCLICommands_atmos_helmfile_help.stderr.golden (1)
1-5: Helmfile help snapshot aligns with stderr-only telemetry notice.All good.
tests/snapshots/TestCLICommands_atmos_helmfile_apply_--help.stderr.golden (2)
1-5: Telemetry notice correctly routed to stderrSnapshot reflects the intended move of the disclosure to stderr for help output. Looks good.
1-5: Guard against snapshot flakiness due to telemetry stateThe
tests/snapshots/TestCLICommands_atmos_helmfile_apply_--help.stderr.goldensnapshot currently relies on the globalTelemetryDisclosureShownflag and theATMOS_TELEMETRY_ENABLEDenvironment variable, but neither is reset nor explicitly set in that test suite. Depending on test ordering, the disclosure notice may or may not appear, leading to intermittent failures.– In
pkg/telemetry/utils_test.go, other tests reliably reset the cache before checking the disclosure (e.g. lines 597–600 resetcacheCfg.TelemetryDisclosureShown = falseand save) and toggleATMOS_TELEMETRY_ENABLEDviaos.Setenv/os.Unsetenv.
– Mirroring that setup, the CLI snapshot test should:
• Load and clear the cache flag before invoking the CLI, e.g.
go cacheCfg, _ := cfg.LoadCache() cacheCfg.TelemetryDisclosureShown = false cfg.SaveCache(cacheCfg)
• Explicitly enable telemetry for the test, e.g.
go os.Setenv("ATMOS_TELEMETRY_ENABLED", "true") defer os.Unsetenv("ATMOS_TELEMETRY_ENABLED")
– Optionally, extract this boilerplate into a helper to ensure all CLI snapshot tests start from a clean telemetry state.Applying these changes will guarantee that the disclosure notice is always present in stderr snapshots.
tests/snapshots/TestCLICommands_atmos_atlantis_generate_repo-config.stderr.golden (1)
1-5: Disclosure on stderr acknowledgedMatches the PR objective; content/order align with other snapshots.
tests/snapshots/TestCLICommands_atmos_atlantis_generate_--help.stderr.golden (1)
1-5: Help stderr snapshot updated correctlyDisclosure present on stderr as intended; consistent with the new behavior.
tests/snapshots/TestCLICommands_atmos_helmfile_--help.stderr.golden (1)
1-5: LGTM: stderr-only telemetry noticeMatches routing change; no other stderr noise introduced here.
tests/snapshots/TestCLICommands_atmos_validate_editorconfig_help.stderr.golden (1)
1-5: Correct stderr placement for telemetry noticeSnapshot aligns with the stated goal and test strategy.
tests/snapshots/TestCLICommands_atmos_warns_if_not_in_git_repo_with_no_atmos_config.stderr.golden (2)
1-5: Telemetry disclosure on stderr — aligned with PR intentGood move routing the notice to stderr; this avoids contaminating piped stdout.
7-10: Confirm these config headers are intended for stderrAre these “Atmos CLI Configuration/Components/Stacks/Quick Start” headers expected on stderr, or should they remain on stdout while only telemetry moves to stderr?
If unintended, we should adjust the emitter so only the telemetry block is redirected.
tests/snapshots/TestCLICommands_atmos_terraform_plan_non-existent_in_non_workspace.stderr.golden (2)
1-5: Telemetry disclosure now on stderr — looks goodMatches the goal to isolate notices from stdout streams.
6-9: Double-check stderr vs stdout ownership of these headersSame question as elsewhere: should these configuration/help headers be on stderr?
If not, we should constrain stderr output strictly to warnings/errors/telemetry.
tests/snapshots/TestCLICommands_atmos_doesnt_warn_if_in_git_repo_with_no_atmos_config.stdout.golden (1)
2-2: LGTM: concise “config not found” messageClear and actionable.
tests/snapshots/TestCLICommands_atmos_atlantis_help.stderr.golden (1)
1-5: Telemetry now on stderr — snapshot aligned.Snapshot reflects routing the disclosure to stderr. Matches DisclosureMessage verbatim.
tests/snapshots/TestCLICommands_atmos_terraform_apply_help.stderr.golden (1)
1-5: Snapshot correctly expects telemetry on stderr.Content mirrors the DisclosureMessage constant. Good.
pkg/telemetry/utils.go (1)
52-57: Correctly route disclosure to stderr via TUI.Doc comment and implementation now use PrintfMessageToTUI (stderr). This directly solves DEV-3597.
tests/snapshots/TestCLICommands_atmos_about_--help.stderr.golden (1)
1-5: stderr snapshot updated — looks good.Telemetry notice appears only on stderr as intended.
pkg/telemetry/utils_test.go (4)
4-13: Imports for stream capture are fine.bytes, io, and strings are appropriate for the new stderr/stdout capture logic.
448-469: Good coverage of disclosure message content and idempotency.Validates first/second call behavior and exact message. Solid.
498-516: CI suppression test is on point.Correctly asserts no message under CI.
520-539: Disabled-telemetry suppression test looks good.Asserts no disclosure when ATMOS_TELEMETRY_ENABLED=false.
| atmos.yamlstacksAtmos CLI Configuration: | ||
| Atmos Components: | ||
| Atmos Stacks: | ||
| Quick Start: |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Non-telemetry content on stderr looks unintended; verify output routing
These headings (“Atmos CLI Configuration”, “Atmos Components”, etc.) appearing on stderr may indicate other messages are going through the TUI/stderr path. The concatenation “atmos.yamlstacksAtmos” also suggests a missing newline. Please confirm these are meant for stderr; otherwise, adjust the print path or formatting.
To locate sources:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find emitters of these headings and inspect how they print (stdout vs TUI/stderr)
rg -nP 'Atmos CLI Configuration:|Atmos Components:|Atmos Stacks:|Quick Start:' -C3 --type go
# List all uses of TUI printing (likely stderr) to spot accidental reroutes
rg -nP '\bPrintfMessageToTUI\(' -n -C3 --type go
# Also check generic print helpers used by CLI help/config output
rg -nP '\bPrint(Message|f)?\w*\(' -n -C1 --type go | sed -n '1,200p'Length of output: 13890
Heads-up: Documentation headings are going to stdout, not stderr
Our investigation shows that in cmd/cmd_utils.go (around lines 536, 539, 542, 545) the links and headings are emitted via
u.PrintMessage("\nTo configure…\n")
u.PrintMessageInColor("Atmos CLI Configuration:\n", c2)
…both of which write to stdout, whereas only u.PrintfMessageToTUI writes to stderr. Because of this:
- These headings shouldn’t be captured in the
*.stderr.goldensnapshot foratmos atlantis generate repo-config; they belong in the stdout snapshot. - If your intent is to emit them on stderr (e.g., separate documentation from machine-readable output), switch the calls to
u.PrintfMessageToTUI.
Recommended fixes:
• In cmd/cmd_utils.go, change each heading call from:
- u.PrintMessageInColor("Atmos CLI Configuration:\n", c2)
+ u.PrintfMessageToTUI("Atmos CLI Configuration:\n")(and similarly for “Atmos Components:”, “Atmos Stacks:”, “Quick Start:”)
• Or, if they should remain on stdout, update tests/snapshots/TestCLICommands_atmos_atlantis_generate_repo-config.stderr.golden to remove those lines and capture them in the corresponding .stdout.golden instead.
Let me know if you’d like any help updating the code or the snapshots!
🤖 Prompt for AI Agents
In cmd/cmd_utils.go around lines 536–545 the documentation headings are being
printed with u.PrintMessage / u.PrintMessageInColor which write to stdout but
the snapshot expects them on stderr; change those heading calls (e.g., the
"Atmos CLI Configuration:", "Atmos Components:", "Atmos Stacks:", "Quick Start:"
prints) to use u.PrintfMessageToTUI (or the equivalent stderr-emitting helper)
so they go to stderr, rebuild tests, and update snapshots if necessary.
| by visiting the following URL: https://atmos.tools/cli/telemetry | ||
|
|
||
| atmos.yaml CLI config file specifies the directory for Atmos stacks as stacks, | ||
| CLI config file specifies the directory for Atmos stacks as , |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Suspicious empty placeholder in message ("as ,")
Looks like the stacks directory value didn’t render. If the value is empty, avoid the dangling comma or tailor the message for the empty case.
Run to locate generator and other occurrences:
Optionally handle empty value in the emitter:
if stacksDir == "" {
fmt.Fprintln(out, "CLI config file does not specify the directory for Atmos stacks; the directory does not exist.")
} else {
fmt.Fprintf(out, "CLI config file specifies the directory for Atmos stacks as %s, but the directory does not exist.\n", stacksDir)
}Note: Aware snapshots mirror exact output (per past learnings); calling this out only because it likely indicates a formatting bug rather than an intentional typo.
🏁 Script executed:
#!/bin/bash
# Find the message template in code
rg -n -C2 --type go 'specifies the directory for Atmos stacks as'
# Check snapshots for the dangling "as ,"
rg -n 'as ,( |$)' tests/snapshotsLength of output: 1010
Refactor CLI stacks directory message to handle empty value
The current implementation always prints a comma placeholder when stacksDir is empty, resulting in messages like “as ,”. To improve UX, tailor the output for the empty case and avoid a dangling comma:
• In cmd/cmd_utils.go (around lines 529–531), replace the three separate print calls with:
stacksDir := filepath.Join(atmosConfig.BasePath, atmosConfig.Stacks.BasePath)
if stacksDir == "" {
fmt.Fprintln(out, "CLI config file does not specify the directory for Atmos stacks; the directory does not exist.")
} else {
fmt.Fprintf(out, "CLI config file specifies the directory for Atmos stacks as %s, but the directory does not exist.\n", stacksDir)
}• Update the affected snapshot files to match the new behavior (tests/snapshots):
– TestCLICommands_atmos_atlantis_generate_repo-config.stdout.golden
– TestCLICommands_atmos_terraform_plan_non-existent_in_non_workspace.stdout.golden
– TestCLICommands_atmos.stdout.golden
🤖 Prompt for AI Agents
In cmd/cmd_utils.go around lines 529 to 531, the CLI message always prints a
dangling comma when stacksDir is empty; change the logic to compute stacksDir
(filepath.Join(atmosConfig.BasePath, atmosConfig.Stacks.BasePath)) and branch:
if stacksDir is empty print a message that the CLI config does not specify the
directory for Atmos stacks and that the directory does not exist, otherwise
print the existing message that the CLI config specifies the directory as
<stacksDir> but the directory does not exist. After changing the output, update
the three snapshot files under tests/snapshots
(TestCLICommands_atmos_atlantis_generate_repo-config.stdout.golden,
TestCLICommands_atmos_terraform_plan_non-existent_in_non_workspace.stdout.golden,
TestCLICommands_atmos.stdout.golden) to match the new wording for the empty
case.
…instead-of-stderr' of github.com:cloudposse/atmos into feature/dev-3597-telemetry-notice-outputting-to-stdout-instead-of-stderr
…-to-stdout-instead-of-stderr
…instead-of-stderr' of github.com:cloudposse/atmos into feature/dev-3597-telemetry-notice-outputting-to-stdout-instead-of-stderr
- Add skipIfPackerNotInstalled helper function to check for packer availability - Update all packer tests to skip gracefully instead of failing when packer is missing - Tests now show SKIP status with informative message when packer is not in PATH
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
cmd/cmd_utils_test.go (1)
196-202: Consider memoizing the packer presence check to avoid repeated LookPath calls.
This keeps test startup a bit snappier when many tests use the guard.Apply this diff to the helper:
func skipIfPackerNotInstalled(t *testing.T) { t.Helper() - if _, err := exec.LookPath("packer"); err != nil { - t.Skip("Skipping test: packer is not installed or not in PATH") - } + packerOnce.Do(func() { + _, err := exec.LookPath("packer") + packerInPath = (err == nil) + }) + if !packerInPath { + t.Skip("Skipping test: packer is not installed or not in PATH") + } }Also add the following outside this range:
// add to imports import "sync" // add near top-level in this file var ( packerOnce sync.Once packerInPath bool )
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
cmd/cmd_utils_test.go(2 hunks)cmd/packer_init_test.go(1 hunks)cmd/packer_inspect_test.go(1 hunks)cmd/packer_output_test.go(1 hunks)cmd/packer_validate_test.go(1 hunks)cmd/packer_version_test.go(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
cmd/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
cmd/*.go: Implement each Cobra command in a separate file under the cmd/ directory
Use kebab-case for command-line flags
Provide comprehensive help text for all commands and flags
Include examples in command help
Provide meaningful feedback to users in command implementation
Include progress indicators for long-running operations
Provide clear error messages to users
Include troubleshooting hints when appropriate
Log detailed errors for debugging
Files:
cmd/packer_validate_test.gocmd/packer_inspect_test.gocmd/packer_output_test.gocmd/packer_version_test.gocmd/cmd_utils_test.gocmd/packer_init_test.go
**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*.go: Use Viper for managing configuration, environment variables, and flags
Use interfaces for external dependencies to facilitate mocking
All code must pass golangci-lint checks
Follow Go's error handling idioms
Use meaningful error messages
Wrap errors with context using fmt.Errorf("context: %w", err)
Consider using a custom error type for domain-specific errors
Follow standard Go coding style
Use gofmt and goimports to format code
Prefer short, descriptive variable names
Use snake_case for environment variables
Document all exported functions, types, and methods
Document complex logic with inline comments
Follow Go's documentation conventions
Use Viper for configuration management
Support configuration via files, environment variables, and flags
Follow the precedence order: flags > environment variables > config file > defaults
Files:
cmd/packer_validate_test.gocmd/packer_inspect_test.gocmd/packer_output_test.gocmd/packer_version_test.gocmd/cmd_utils_test.gocmd/packer_init_test.go
**/*_test.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*_test.go: Every new feature must include comprehensive unit tests
Test both happy paths and error conditions
Use table-driven tests for testing multiple scenarios
Include integration tests for command flows
Test CLI end-to-end when possible
Use test fixtures for complex inputs
Consider using testify/mock for creating mock implementations
Files:
cmd/packer_validate_test.gocmd/packer_inspect_test.gocmd/packer_output_test.gocmd/packer_version_test.gocmd/cmd_utils_test.gocmd/packer_init_test.go
🧠 Learnings (2)
📚 Learning: 2025-05-23T19:51:47.091Z
Learnt from: samtholiya
PR: cloudposse/atmos#1255
File: cmd/describe_affected_test.go:15-15
Timestamp: 2025-05-23T19:51:47.091Z
Learning: The atmos codebase has a custom extension to *testing.T that provides a Chdir method, allowing test functions to call t.Chdir() to change working directories during tests. This is used consistently across test files in the codebase.
Applied to files:
cmd/packer_inspect_test.gocmd/packer_output_test.gocmd/packer_init_test.go
📚 Learning: 2025-07-01T02:22:25.901Z
Learnt from: CR
PR: cloudposse/atmos#0
File: .cursor/rules/atmos-rules.mdc:0-0
Timestamp: 2025-07-01T02:22:25.901Z
Learning: Applies to **/*_test.go : Test both happy paths and error conditions
Applied to files:
cmd/cmd_utils_test.go
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Build (windows-latest, windows)
- GitHub Check: Lint (golangci)
- GitHub Check: Summary
🔇 Additional comments (6)
cmd/cmd_utils_test.go (1)
5-5: LGTM on importing os/exec for the helper.cmd/packer_inspect_test.go (1)
14-15: Good guard placement.
Early skip prevents side effects before environment setup.cmd/packer_validate_test.go (1)
14-15: Guard reads well and matches the shared helper.cmd/packer_output_test.go (1)
14-15: LGTM — consistent with other packer tests.cmd/packer_version_test.go (1)
14-15: All packer tests now includeskipIfPackerNotInstalled(t)— approved.
Everycmd/packer_*_test.gofile was checked and contains the early skip guard. Ready to merge.cmd/packer_init_test.go (1)
11-12: All TestPacker tests include the Packer‐installed guard*Verified that:
- The helper
skipIfPackerNotInstalled(t *testing.T)is implemented incmd/cmd_utils_test.goand usesexec.LookPath("packer")to detect absence.- Every
TestPacker*function incmd/packer_*_test.gobegins with a call toskipIfPackerNotInstalled(t), preventing CI failures when Packer isn’t installed.Great job—no further changes needed here.
- Add skipIfTerraformNotInstalled helper function to check for terraform availability - Update all terraform tests to skip gracefully instead of failing when terraform is missing - Tests now show SKIP status with informative message when terraform is not in PATH - Similar to the packer test skip mechanism
…instead-of-stderr' of github.com:cloudposse/atmos into feature/dev-3597-telemetry-notice-outputting-to-stdout-instead-of-stderr
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
cmd/terraform_test.go (3)
19-29: Prefer t.Setenv over os.Setenv + manual cleanup.Reduces boilerplate and auto-restores env.
- err := os.Setenv("ATMOS_CLI_CONFIG_PATH", stacksPath) - assert.NoError(t, err, "Setting 'ATMOS_CLI_CONFIG_PATH' environment variable should execute without error") - - err = os.Setenv("ATMOS_BASE_PATH", stacksPath) - assert.NoError(t, err, "Setting 'ATMOS_BASE_PATH' environment variable should execute without error") - - // Unset env values after testing - defer func() { - os.Unsetenv("ATMOS_BASE_PATH") - os.Unsetenv("ATMOS_CLI_CONFIG_PATH") - }() + t.Setenv("ATMOS_CLI_CONFIG_PATH", stacksPath) + t.Setenv("ATMOS_BASE_PATH", stacksPath)
65-69: Apply t.Setenv here too.Same simplification for TestTerraformRun2.
- // Unset env values after testing - defer func() { - os.Unsetenv("ATMOS_BASE_PATH") - os.Unsetenv("ATMOS_CLI_CONFIG_PATH") - }() + // t.Setenv will auto-clean these
99-109: Use t.Setenv in TestTerraformRun3 as well.Keeps all three tests consistent and cleaner.
- err := os.Setenv("ATMOS_CLI_CONFIG_PATH", stacksPath) - assert.NoError(t, err, "Setting 'ATMOS_CLI_CONFIG_PATH' environment variable should execute without error") - - err = os.Setenv("ATMOS_BASE_PATH", stacksPath) - assert.NoError(t, err, "Setting 'ATMOS_BASE_PATH' environment variable should execute without error") - - // Unset env values after testing - defer func() { - os.Unsetenv("ATMOS_BASE_PATH") - os.Unsetenv("ATMOS_CLI_CONFIG_PATH") - }() + t.Setenv("ATMOS_CLI_CONFIG_PATH", stacksPath) + t.Setenv("ATMOS_BASE_PATH", stacksPath)cmd/terraform_generate_varfile_test.go (1)
17-24: Use t.Setenv to simplify env setup and teardown.Less noise; auto cleanup.
- err := os.Setenv("ATMOS_CLI_CONFIG_PATH", stacksPath) - assert.NoError(t, err, "Setting 'ATMOS_CLI_CONFIG_PATH' environment variable should execute without error") - - err = os.Setenv("ATMOS_BASE_PATH", stacksPath) - assert.NoError(t, err, "Setting 'ATMOS_BASE_PATH' environment variable should execute without error") - - err = os.Setenv("ATMOS_LOGS_LEVEL", "Debug") - assert.NoError(t, err, "Setting 'ATMOS_LOGS_LEVEL' environment variable should execute without error") + t.Setenv("ATMOS_CLI_CONFIG_PATH", stacksPath) + t.Setenv("ATMOS_BASE_PATH", stacksPath) + t.Setenv("ATMOS_LOGS_LEVEL", "Debug")Outside the selected lines, also change the first later assignment to declare err:
// before err = Execute() // after err := Execute()Also applies to: 36-41
cmd/terraform_generate_backend_test.go (1)
17-24: Likewise, switch to t.Setenv for brevity.Consistent with other tests.
- err := os.Setenv("ATMOS_CLI_CONFIG_PATH", stacksPath) - assert.NoError(t, err, "Setting 'ATMOS_CLI_CONFIG_PATH' environment variable should execute without error") - - err = os.Setenv("ATMOS_BASE_PATH", stacksPath) - assert.NoError(t, err, "Setting 'ATMOS_BASE_PATH' environment variable should execute without error") - - err = os.Setenv("ATMOS_LOGS_LEVEL", "Debug") - assert.NoError(t, err, "Setting 'ATMOS_LOGS_LEVEL' environment variable should execute without error") + t.Setenv("ATMOS_CLI_CONFIG_PATH", stacksPath) + t.Setenv("ATMOS_BASE_PATH", stacksPath) + t.Setenv("ATMOS_LOGS_LEVEL", "Debug")Outside the selected lines, also declare err at first use:
// before err = Execute() // after err := Execute()Also applies to: 36-41
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
cmd/cmd_utils_test.go(2 hunks)cmd/terraform_generate_backend_test.go(1 hunks)cmd/terraform_generate_varfile_test.go(1 hunks)cmd/terraform_test.go(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- cmd/cmd_utils_test.go
🧰 Additional context used
📓 Path-based instructions (3)
cmd/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
cmd/*.go: Implement each Cobra command in a separate file under the cmd/ directory
Use kebab-case for command-line flags
Provide comprehensive help text for all commands and flags
Include examples in command help
Provide meaningful feedback to users in command implementation
Include progress indicators for long-running operations
Provide clear error messages to users
Include troubleshooting hints when appropriate
Log detailed errors for debugging
Files:
cmd/terraform_generate_varfile_test.gocmd/terraform_generate_backend_test.gocmd/terraform_test.go
**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*.go: Use Viper for managing configuration, environment variables, and flags
Use interfaces for external dependencies to facilitate mocking
All code must pass golangci-lint checks
Follow Go's error handling idioms
Use meaningful error messages
Wrap errors with context using fmt.Errorf("context: %w", err)
Consider using a custom error type for domain-specific errors
Follow standard Go coding style
Use gofmt and goimports to format code
Prefer short, descriptive variable names
Use snake_case for environment variables
Document all exported functions, types, and methods
Document complex logic with inline comments
Follow Go's documentation conventions
Use Viper for configuration management
Support configuration via files, environment variables, and flags
Follow the precedence order: flags > environment variables > config file > defaults
Files:
cmd/terraform_generate_varfile_test.gocmd/terraform_generate_backend_test.gocmd/terraform_test.go
**/*_test.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*_test.go: Every new feature must include comprehensive unit tests
Test both happy paths and error conditions
Use table-driven tests for testing multiple scenarios
Include integration tests for command flows
Test CLI end-to-end when possible
Use test fixtures for complex inputs
Consider using testify/mock for creating mock implementations
Files:
cmd/terraform_generate_varfile_test.gocmd/terraform_generate_backend_test.gocmd/terraform_test.go
🧠 Learnings (2)
📚 Learning: 2024-12-07T16:19:01.683Z
Learnt from: aknysh
PR: cloudposse/atmos#825
File: internal/exec/terraform.go:30-30
Timestamp: 2024-12-07T16:19:01.683Z
Learning: In `internal/exec/terraform.go`, skipping stack validation when help flags are present is not necessary.
Applied to files:
cmd/terraform_generate_varfile_test.go
📚 Learning: 2024-12-07T16:16:13.038Z
Learnt from: Listener430
PR: cloudposse/atmos#825
File: internal/exec/helmfile_generate_varfile.go:28-31
Timestamp: 2024-12-07T16:16:13.038Z
Learning: In `internal/exec/helmfile_generate_varfile.go`, the `--help` command (`./atmos helmfile generate varfile --help`) works correctly without requiring stack configurations, and the only change needed was to make `ProcessCommandLineArgs` exportable by capitalizing its name.
Applied to files:
cmd/terraform_generate_varfile_test.go
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Build (windows-latest, windows)
- GitHub Check: Lint (golangci)
- GitHub Check: Summary
🔇 Additional comments (5)
cmd/terraform_test.go (3)
14-14: Good guard to avoid env-dependent flakes.Early skip keeps these tests stable on hosts without Terraform.
53-53: Same here—nice pre-check.Consistent with TestTerraformRun1.
95-95: Ditto for TestTerraformRun3.Consistent application across tests.
cmd/terraform_generate_varfile_test.go (1)
13-13: Solid pre-check.Skips cleanly when Terraform isn’t present.
cmd/terraform_generate_backend_test.go (1)
13-13: Good addition.Avoids false negatives on machines without Terraform.
| ) | ||
|
|
||
| func TestTerraformRun1(t *testing.T) { | ||
| skipIfTerraformNotInstalled(t) |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Confirm helper behavior (t.Helper, informative Skipf).
Please ensure skipIfTerraformNotInstalled(t) calls t.Helper() and uses t.Skipf with a clear message (e.g., “terraform not found in PATH”). Quick check script:
Also applies to: 53-53, 95-95
🏁 Script executed:
#!/bin/bash
# Verify helper has t.Helper() and uses exec.LookPath("terraform")
rg -nC2 'func\s+skipIfTerraformNotInstalled\(\s*t\s+\*testing\.T\s*\)' -g 'cmd/**_test.go'
rg -nC2 't\.Helper\(\)|exec\.LookPath\("terraform"\)|t\.Skipf?\(' -g 'cmd/**_test.go'Length of output: 1414
Use t.Skipf for clearer skip messages
The skipIfTerraformNotInstalled helper already calls t.Helper() (line 206) and uses exec.LookPath("terraform") (line 207), but invokes t.Skip rather than t.Skipf. Please switch to t.Skipf with a concise, informative message. Likewise, update skipIfPackerNotInstalled.
• cmd/cmd_utils_test.go:
– skipIfTerraformNotInstalled (currently at line 208)
– skipIfPackerNotInstalled (currently at line 200)
Suggested diff snippets:
@@ cmd/cmd_utils_test.go:207
- t.Skip("Skipping test: terraform is not installed or not in PATH")
+ t.Skipf("terraform not found in PATH, skipping test")@@ cmd/cmd_utils_test.go:199
- t.Skip("Skipping test: packer is not installed or not in PATH")
+ t.Skipf("packer not found in PATH, skipping test")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| skipIfTerraformNotInstalled(t) | |
| // cmd/cmd_utils_test.go | |
| func skipIfTerraformNotInstalled(t *testing.T) { | |
| t.Helper() | |
| if _, err := exec.LookPath("terraform"); err != nil { | |
| t.Skipf("terraform not found in PATH, skipping test") | |
| } | |
| } | |
| func skipIfPackerNotInstalled(t *testing.T) { | |
| t.Helper() | |
| if _, err := exec.LookPath("packer"); err != nil { | |
| t.Skipf("packer not found in PATH, skipping test") | |
| } | |
| } |
🤖 Prompt for AI Agents
cmd/cmd_utils_test.go around lines 200 to 208 (and update call sites such as
cmd/terraform_test.go line 14): the helper functions skipIfTerraformNotInstalled
and skipIfPackerNotInstalled currently call t.Skip; change them to call t.Skipf
with a concise informative message that includes the underlying error (e.g.,
t.Skipf("terraform not installed: %v", err) and similarly for packer) while
preserving t.Helper() and exec.LookPath usage so skipped tests report why they
were skipped.
- Add skipIfHelmfileNotInstalled() helper function to cmd/cmd_utils_test.go - Update cmd/helmfile_test.go to skip tests when helmfile is not available - Update internal/exec/helmfile_test.go to skip tests when helmfile is not available - Update internal/exec/terraform_test.go to skip all terraform tests when terraform is not available This makes the test suite more portable and prevents failures on systems where helmfile or terraform are not installed.
…instead-of-stderr' of github.com:cloudposse/atmos into feature/dev-3597-telemetry-notice-outputting-to-stdout-instead-of-stderr
- Update internal/exec/terraform_generate_planfile_test.go to skip tests when terraform is not available - Update internal/exec/terraform_utils_test.go to skip tests when terraform is not available This ensures all terraform-dependent tests properly skip when terraform is not installed, making the test suite fully portable.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
internal/exec/helmfile_test.go (1)
6-6: Alias os/exec to avoid confusion with package exec.Within package exec, importing os/exec as exec is easy to misread. Prefer osexec.
-import ( +import ( "bytes" "os" - "os/exec" + osexec "os/exec" "strings" "testing" ) @@ - if _, err := exec.LookPath("helmfile"); err != nil { + if _, err := osexec.LookPath("helmfile"); err != nil {cmd/helmfile_test.go (1)
11-11: Ensure the helper marks itself as a test helper.In cmd/cmd_utils_test.go, add t.Helper() inside skipIfHelmfileNotInstalled for cleaner failure locations.
func skipIfHelmfileNotInstalled(t *testing.T) { + t.Helper() if _, err := exec.LookPath("helmfile"); err != nil { t.Skip("Skipping test: helmfile is not installed or not in PATH") } }internal/exec/terraform_test.go (3)
21-24: Good skips; consider DRYing with a helper and aliasing os/exec.
- Create a local test helper skipIfTerraformNotInstalled(t) and reuse it in each test.
- Alias os/exec to osexec for readability inside package exec.
-import ( +import ( "bytes" "os" - "os/exec" + osexec "os/exec" "strings" "testing" @@ - if _, err := exec.LookPath("terraform"); err != nil { + if _, err := osexec.LookPath("terraform"); err != nil { t.Skip("Skipping test: terraform is not installed or not in PATH") }Add once in this file:
// test-only helper func skipIfTerraformNotInstalled(t *testing.T) { t.Helper() if _, err := osexec.LookPath("terraform"); err != nil { t.Skip("Skipping test: terraform is not installed or not in PATH") } }Then replace the repeated guards with skipIfTerraformNotInstalled(t).
Also applies to: 115-118, 183-186, 253-256, 318-321, 359-362, 427-430, 471-474, 547-550
6-6: Alias import to avoid name ambiguity.Within package exec, prefer osexec "os/exec".
- "os/exec" + osexec "os/exec"
547-550: Prefer a single sink; also restore logger output.You capture stderr into a pipe and also redirect logger to a buffer, then merge both. Consider capturing just one, or keep both but restore logger output.
- oldStderr := os.Stderr - r, w, _ := os.Pipe() - os.Stderr = w + oldStderr := os.Stderr + r, w, _ := os.Pipe() + os.Stderr = w + defer func() { + os.Stderr = oldStderr + log.SetOutput(os.Stderr) + }() @@ - var buf bytes.Buffer - log.SetOutput(&buf) + var buf bytes.Buffer + log.SetOutput(&buf) + // Alternatively, remove the pipe and capture only the logger: + // (drop the r/w pipe entirely if all messages are via log)
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
cmd/cmd_utils_test.go(2 hunks)cmd/helmfile_test.go(1 hunks)internal/exec/helmfile_test.go(1 hunks)internal/exec/terraform_test.go(10 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- cmd/cmd_utils_test.go
🧰 Additional context used
📓 Path-based instructions (3)
cmd/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
cmd/*.go: Implement each Cobra command in a separate file under the cmd/ directory
Use kebab-case for command-line flags
Provide comprehensive help text for all commands and flags
Include examples in command help
Provide meaningful feedback to users in command implementation
Include progress indicators for long-running operations
Provide clear error messages to users
Include troubleshooting hints when appropriate
Log detailed errors for debugging
Files:
cmd/helmfile_test.go
**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*.go: Use Viper for managing configuration, environment variables, and flags
Use interfaces for external dependencies to facilitate mocking
All code must pass golangci-lint checks
Follow Go's error handling idioms
Use meaningful error messages
Wrap errors with context using fmt.Errorf("context: %w", err)
Consider using a custom error type for domain-specific errors
Follow standard Go coding style
Use gofmt and goimports to format code
Prefer short, descriptive variable names
Use snake_case for environment variables
Document all exported functions, types, and methods
Document complex logic with inline comments
Follow Go's documentation conventions
Use Viper for configuration management
Support configuration via files, environment variables, and flags
Follow the precedence order: flags > environment variables > config file > defaults
Files:
cmd/helmfile_test.gointernal/exec/helmfile_test.gointernal/exec/terraform_test.go
**/*_test.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*_test.go: Every new feature must include comprehensive unit tests
Test both happy paths and error conditions
Use table-driven tests for testing multiple scenarios
Include integration tests for command flows
Test CLI end-to-end when possible
Use test fixtures for complex inputs
Consider using testify/mock for creating mock implementations
Files:
cmd/helmfile_test.gointernal/exec/helmfile_test.gointernal/exec/terraform_test.go
🧠 Learnings (14)
📚 Learning: 2024-12-07T16:16:13.038Z
Learnt from: Listener430
PR: cloudposse/atmos#825
File: internal/exec/helmfile_generate_varfile.go:28-31
Timestamp: 2024-12-07T16:16:13.038Z
Learning: In `internal/exec/helmfile_generate_varfile.go`, the `--help` command (`./atmos helmfile generate varfile --help`) works correctly without requiring stack configurations, and the only change needed was to make `ProcessCommandLineArgs` exportable by capitalizing its name.
Applied to files:
cmd/helmfile_test.gointernal/exec/helmfile_test.go
📚 Learning: 2025-08-15T14:43:41.030Z
Learnt from: aknysh
PR: cloudposse/atmos#1352
File: pkg/store/artifactory_store_test.go:108-113
Timestamp: 2025-08-15T14:43:41.030Z
Learning: In test files for the atmos project, it's acceptable to ignore errors from os.Setenv/Unsetenv operations during test environment setup and teardown, as these are controlled test scenarios.
Applied to files:
cmd/helmfile_test.gointernal/exec/terraform_test.go
📚 Learning: 2024-12-12T15:17:45.245Z
Learnt from: osterman
PR: cloudposse/atmos#808
File: examples/demo-atmos.d/atmos.d/tools/helmfile.yml:10-10
Timestamp: 2024-12-12T15:17:45.245Z
Learning: In `examples/demo-atmos.d/atmos.d/tools/helmfile.yml`, when suggesting changes to `kubeconfig_path`, ensure that the values use valid Go template syntax.
Applied to files:
cmd/helmfile_test.go
📚 Learning: 2024-12-07T16:19:01.683Z
Learnt from: aknysh
PR: cloudposse/atmos#825
File: internal/exec/terraform.go:30-30
Timestamp: 2024-12-07T16:19:01.683Z
Learning: In `internal/exec/terraform.go`, skipping stack validation when help flags are present is not necessary.
Applied to files:
internal/exec/terraform_test.go
📚 Learning: 2024-10-27T04:41:49.199Z
Learnt from: haitham911
PR: cloudposse/atmos#727
File: internal/exec/terraform_clean.go:215-223
Timestamp: 2024-10-27T04:41:49.199Z
Learning: In `internal/exec/terraform_clean.go`, the function `determineCleanPath` is necessary and should not be removed.
Applied to files:
internal/exec/terraform_test.go
📚 Learning: 2024-11-02T15:35:09.958Z
Learnt from: aknysh
PR: cloudposse/atmos#759
File: internal/exec/terraform.go:366-368
Timestamp: 2024-11-02T15:35:09.958Z
Learning: In `internal/exec/terraform.go`, the workspace cleaning code under both the general execution path and within the `case "init":` block is intentionally duplicated because the code execution paths are different. The `.terraform/environment` file should be deleted before executing `terraform init` in both scenarios to ensure a clean state.
Applied to files:
internal/exec/terraform_test.go
📚 Learning: 2025-04-26T15:54:10.506Z
Learnt from: haitham911
PR: cloudposse/atmos#1195
File: internal/exec/terraform_clean.go:99-99
Timestamp: 2025-04-26T15:54:10.506Z
Learning: The error variable `ErrRelPath` is defined in `internal/exec/terraform_clean_util.go` and is used across files in the `exec` package, including in `terraform_clean.go`. This is part of an approach to standardize error handling in the codebase.
Applied to files:
internal/exec/terraform_test.go
📚 Learning: 2024-10-27T04:28:40.966Z
Learnt from: haitham911
PR: cloudposse/atmos#727
File: internal/exec/terraform_clean.go:155-175
Timestamp: 2024-10-27T04:28:40.966Z
Learning: In the `CollectDirectoryObjects` function in `internal/exec/terraform_clean.go`, recursive search through all subdirectories is not needed.
Applied to files:
internal/exec/terraform_test.go
📚 Learning: 2025-05-23T19:51:47.091Z
Learnt from: samtholiya
PR: cloudposse/atmos#1255
File: cmd/describe_affected_test.go:15-15
Timestamp: 2025-05-23T19:51:47.091Z
Learning: The atmos codebase has a custom extension to *testing.T that provides a Chdir method, allowing test functions to call t.Chdir() to change working directories during tests. This is used consistently across test files in the codebase.
Applied to files:
internal/exec/terraform_test.go
📚 Learning: 2025-05-23T19:51:47.091Z
Learnt from: samtholiya
PR: cloudposse/atmos#1255
File: cmd/describe_affected_test.go:15-15
Timestamp: 2025-05-23T19:51:47.091Z
Learning: In the atmos codebase, t.Chdir() is a valid method call on *testing.T objects and works correctly for changing directories in tests. This is implemented through custom testing framework extensions and is used consistently throughout the test suite.
Applied to files:
internal/exec/terraform_test.go
📚 Learning: 2025-05-23T19:51:47.091Z
Learnt from: samtholiya
PR: cloudposse/atmos#1255
File: cmd/describe_affected_test.go:15-15
Timestamp: 2025-05-23T19:51:47.091Z
Learning: In the atmos codebase, t.Chdir() is a valid method that can be called on *testing.T objects. This functionality is implemented through custom testing framework extensions and is used consistently throughout the test suite for changing working directories during tests.
Applied to files:
internal/exec/terraform_test.go
📚 Learning: 2025-05-23T19:51:47.091Z
Learnt from: samtholiya
PR: cloudposse/atmos#1255
File: cmd/describe_affected_test.go:15-15
Timestamp: 2025-05-23T19:51:47.091Z
Learning: In the atmos codebase, t.Chdir() is a valid method call on *testing.T objects and works correctly for changing directories in tests.
Applied to files:
internal/exec/terraform_test.go
📚 Learning: 2024-11-12T03:16:02.910Z
Learnt from: aknysh
PR: cloudposse/atmos#775
File: internal/exec/template_funcs_component.go:157-159
Timestamp: 2024-11-12T03:16:02.910Z
Learning: In the Go code for `componentFunc` in `internal/exec/template_funcs_component.go`, the function `cleanTerraformWorkspace` does not return errors, and it's acceptable if the file does not exist. Therefore, error handling for `cleanTerraformWorkspace` is not needed.
Applied to files:
internal/exec/terraform_test.go
📚 Learning: 2024-11-24T19:13:10.287Z
Learnt from: haitham911
PR: cloudposse/atmos#727
File: internal/exec/terraform_clean.go:407-416
Timestamp: 2024-11-24T19:13:10.287Z
Learning: In `internal/exec/terraform_clean.go`, when `getStackTerraformStateFolder` returns an error in the `handleCleanSubCommand` function, the error is logged, and the process continues without returning the error.
Applied to files:
internal/exec/terraform_test.go
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Build (macos-latest, macos)
- GitHub Check: Build (windows-latest, windows)
- GitHub Check: Analyze (go)
- GitHub Check: Lint (golangci)
- GitHub Check: Summary
🔇 Additional comments (2)
internal/exec/helmfile_test.go (1)
14-17: Good guard: skip when helmfile is absent.This prevents spurious CI failures on hosts without helmfile.
cmd/helmfile_test.go (1)
11-11: LGTM: centralized guard improves test stability.Calling skipIfHelmfileNotInstalled first keeps the test cheap and deterministic.
| // Skip if helmfile is not installed | ||
| if _, err := exec.LookPath("helmfile"); err != nil { | ||
| t.Skip("Skipping test: helmfile is not installed or not in PATH") | ||
| } |
There was a problem hiding this comment.
Add defers to always restore stdio on test failure.
If ExecuteHelmfile ever returns an error and t.Fatalf triggers, os.Stdout may remain redirected for subsequent tests. Defer the restore right after redirection.
- oldStdout := os.Stdout
- r, w, _ := os.Pipe()
- os.Stdout = w
+ oldStdout := os.Stdout
+ r, w, _ := os.Pipe()
+ os.Stdout = w
+ defer func() {
+ os.Stdout = oldStdout
+ _ = w.Close() // safe even if already closed below
+ }()Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In internal/exec/helmfile_test.go around lines 14 to 17, ensure that when the
test redirects os.Stdout/os.Stderr before calling ExecuteHelmfile you
immediately save the original file descriptors and add defer statements to
restore them (both stdout and stderr if redirected) so they are restored even if
t.Fatalf or any other failure occurs; place the defers right after performing
the redirection and before invoking ExecuteHelmfile to guarantee restoration on
error.
| // Skip if terraform is not installed | ||
| if _, err := exec.LookPath("terraform"); err != nil { | ||
| t.Skip("Skipping test: terraform is not installed or not in PATH") | ||
| } |
There was a problem hiding this comment.
Always restore stdio on failure to prevent cross-test pollution.
Several tests redirect os.Stdout/os.Stderr without a defer before calling ExecuteTerraform, so a t.Fatalf leaves global stdio redirected. Add immediate defers after redirection. Example for TestExecuteTerraform_ExportEnvVar:
- oldStdout := os.Stdout
- r, w, _ := os.Pipe()
- os.Stdout = w
+ oldStdout := os.Stdout
+ r, w, _ := os.Pipe()
+ os.Stdout = w
+ defer func() {
+ os.Stdout = oldStdout
+ _ = w.Close() // safe even if closed later
+ }()Apply the same pattern to all tests that redirect stdout/stderr.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
internal/exec/terraform_test.go lines ~21-24: tests that redirect
os.Stdout/os.Stderr (e.g., TestExecuteTerraform_ExportEnvVar) must immediately
defer restoration of global stdio to avoid cross-test pollution on failure;
after you reassign os.Stdout/os.Stderr save the old value to a local variable
and immediately call defer to restore it (and defer closing any pipe/writer)
before invoking ExecuteTerraform or any t.Fatalf; apply the same immediate-defer
pattern to all tests in this file that redirect stdout/stderr.
| // Skip if terraform is not installed | ||
| if _, err := exec.LookPath("terraform"); err != nil { | ||
| t.Skip("Skipping test: terraform is not installed or not in PATH") | ||
| } |
There was a problem hiding this comment.
Reset charmbracelet/log output after the test.
log.SetOutput(w) persists globally; subsequent tests may write to a closed pipe. Ensure you restore it.
- log.SetLevel(log.DebugLevel)
- log.SetOutput(w)
+ log.SetLevel(log.DebugLevel)
+ log.SetOutput(w)
+ defer log.SetOutput(os.Stderr)Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In internal/exec/terraform_test.go around lines 359 to 362, the test sets
log.SetOutput(w) but does not restore the previous global logger output, which
can cause other tests to write to a closed pipe; save the current log output
before calling log.SetOutput(w) (e.g. prev := log.Writer()), then immediately
defer restoring it with log.SetOutput(prev) and defer closing the temporary
writer (close(w)) after the restore so the global logger is returned to its
original writer when the test finishes.
- Use 'osexec' alias for 'os/exec' imports in internal/exec test files to avoid namespace collision with package name - Fixed imports in: - internal/exec/terraform_test.go - internal/exec/terraform_generate_planfile_test.go - internal/exec/terraform_utils_test.go - internal/exec/helmfile_test.go - internal/exec/packer_test.go - Added skip logic to packer tests that require packer to be installed - All tests now properly skip when required tools (packer, terraform, helmfile) are not available This resolves test failures caused by namespace collision between the 'exec' package name and 'os/exec' import.
- Add t.Setenv("AWS_PROFILE", "") to AWS tests to prevent conflicts with local AWS configuration
- Fixes TestLoadAWSConfig in internal/aws_utils
- Fixes TestNewSSMStore in pkg/store
- Fixes TestReadTerraformBackendS3_InvalidConfig in internal/terraform_backend
- These tests were failing due to AWS_PROFILE=cplive-core-gbl-identity being set in the environment
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
internal/aws_utils/aws_utils_test.go (1)
53-55: Optional: switch other env setup to t.Setenv and drop cleanupEnv.Reduces risk of leaks if cleanup is skipped and simplifies the table.
Apply a small refactor:
@@ - setupEnv func() - cleanupEnv func() + setupEnv func(t *testing.T) @@ - setupEnv: func() { - os.Setenv("AWS_ACCESS_KEY_ID", "test-key") - os.Setenv("AWS_SECRET_ACCESS_KEY", "test-secret") - }, - cleanupEnv: func() { - os.Unsetenv("AWS_ACCESS_KEY_ID") - os.Unsetenv("AWS_SECRET_ACCESS_KEY") - }, + setupEnv: func(t *testing.T) { + t.Setenv("AWS_ACCESS_KEY_ID", "test-key") + t.Setenv("AWS_SECRET_ACCESS_KEY", "test-secret") + }, @@ - setupEnv: func() { - os.Setenv("AWS_ACCESS_KEY_ID", "test-key") - os.Setenv("AWS_SECRET_ACCESS_KEY", "test-secret") - }, - cleanupEnv: func() { - os.Unsetenv("AWS_ACCESS_KEY_ID") - os.Unsetenv("AWS_SECRET_ACCESS_KEY") - }, + setupEnv: func(t *testing.T) { + t.Setenv("AWS_ACCESS_KEY_ID", "test-key") + t.Setenv("AWS_SECRET_ACCESS_KEY", "test-secret") + }, @@ - // Setup - if tt.setupEnv != nil { - tt.setupEnv() - } + // Setup + if tt.setupEnv != nil { + tt.setupEnv(t) + } @@ - // Cleanup - if tt.cleanupEnv != nil { - defer tt.cleanupEnv() - } + // No explicit cleanup needed; t.Setenv handles restoration.internal/terraform_backend/terraform_backend_s3_test.go (1)
98-100: Optional: also hard-disable shared config autoloading.Guard against environments exporting AWS_SDK_LOAD_CONFIG=1.
// Clear AWS_PROFILE to prevent conflicts with local AWS configuration. t.Setenv("AWS_PROFILE", "") + // Ensure SDK does not read shared config/credentials. + t.Setenv("AWS_SDK_LOAD_CONFIG", "0")pkg/store/aws_ssm_param_store_test.go (1)
588-590: Optional: add a tiny test helper to DRY this across files.Keeps tests consistent and minimizes repetition.
Example helper (new file: internal/testutil/env.go):
package testutil import "testing" func ClearAWSProfile(t *testing.T) { t.Helper() t.Setenv("AWS_PROFILE", "") }Usage here:
- t.Setenv("AWS_PROFILE", "") + testutil.ClearAWSProfile(t)
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
internal/aws_utils/aws_utils_test.go(1 hunks)internal/terraform_backend/terraform_backend_s3_test.go(1 hunks)pkg/store/aws_ssm_param_store_test.go(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*.go: Use Viper for managing configuration, environment variables, and flags
Use interfaces for external dependencies to facilitate mocking
All code must pass golangci-lint checks
Follow Go's error handling idioms
Use meaningful error messages
Wrap errors with context using fmt.Errorf("context: %w", err)
Consider using a custom error type for domain-specific errors
Follow standard Go coding style
Use gofmt and goimports to format code
Prefer short, descriptive variable names
Use snake_case for environment variables
Document all exported functions, types, and methods
Document complex logic with inline comments
Follow Go's documentation conventions
Use Viper for configuration management
Support configuration via files, environment variables, and flags
Follow the precedence order: flags > environment variables > config file > defaults
Files:
internal/aws_utils/aws_utils_test.gopkg/store/aws_ssm_param_store_test.gointernal/terraform_backend/terraform_backend_s3_test.go
**/*_test.go
📄 CodeRabbit inference engine (.cursor/rules/atmos-rules.mdc)
**/*_test.go: Every new feature must include comprehensive unit tests
Test both happy paths and error conditions
Use table-driven tests for testing multiple scenarios
Include integration tests for command flows
Test CLI end-to-end when possible
Use test fixtures for complex inputs
Consider using testify/mock for creating mock implementations
Files:
internal/aws_utils/aws_utils_test.gopkg/store/aws_ssm_param_store_test.gointernal/terraform_backend/terraform_backend_s3_test.go
🧠 Learnings (1)
📚 Learning: 2025-08-15T14:43:41.030Z
Learnt from: aknysh
PR: cloudposse/atmos#1352
File: pkg/store/artifactory_store_test.go:108-113
Timestamp: 2025-08-15T14:43:41.030Z
Learning: In test files for the atmos project, it's acceptable to ignore errors from os.Setenv/Unsetenv operations during test environment setup and teardown, as these are controlled test scenarios.
Applied to files:
internal/terraform_backend/terraform_backend_s3_test.go
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build (windows-latest, windows)
- GitHub Check: Summary
🔇 Additional comments (3)
internal/aws_utils/aws_utils_test.go (1)
53-55: LGTM: isolate AWS_PROFILE per subtest with t.Setenv.Prevents local profile bleed; aligns with the repo’s test hygiene.
internal/terraform_backend/terraform_backend_s3_test.go (1)
98-100: LGTM: clearing AWS_PROFILE per subtest.Prevents accidental use of local profiles.
pkg/store/aws_ssm_param_store_test.go (1)
588-590: LGTM: AWS_PROFILE isolation using t.Setenv.Good defense against flaky, env-dependent tests.
Summary
This PR fixes an issue where the telemetry notice was being output to
stdoutinstead ofstderr, which could interfere with command output piping.PrintTelemetryDisclosure()to usePrintfMessageToTUI()which outputs tostderrstdoutas before (unchanged)Test Plan
TestPrintTelemetryDisclosureOutputsToStderrto verify telemetry outputs to stderrReferences
🤖 Generated with Claude Code
Summary by CodeRabbit