Skip to content

feat(tests): speed up CI by parallelizing native-ci acceptance tests with proper isolation#2205

Open
Copilot wants to merge 28 commits intomainfrom
copilot/reduce-acceptance-test-time
Open

feat(tests): speed up CI by parallelizing native-ci acceptance tests with proper isolation#2205
Copilot wants to merge 28 commits intomainfrom
copilot/reduce-acceptance-test-time

Conversation

Copy link
Contributor

Copilot AI commented Mar 15, 2026

  • Fix: header casing, PATH fallback, env sort, applyIgnorePatterns, git timeout, coverage script hygiene
  • Fix(A): generateUnifiedDiff — skip colors when stderr is not a TTY
  • Fix(B): simulateTtyCommand — t.Skipf on PTY failure (EINVAL, ErrUnsupported, pty.ErrUnsupported)
  • Fix(C): macOS CI HOME isolation — always set HOME/XDG_* to t.TempDir(); removed darwin/CI bypass and early PATH assignment
  • Fix(D): findGitRepoRoot — 10s context timeout + CombinedOutput for better error diagnostics
  • Fix(E): .gitconfig gate — copy only when ATMOS_TEST_COPY_GITCONFIG=1; write minimal sanitized .gitconfig otherwise
  • Fix(F): ci_test.go — added "preserves bare non-CI key without value separator" subtest
  • All files ≤600 lines; go test -c ./tests/ and telemetry package compile cleanly

📱 Kick off Copilot coding agent tasks wherever you are with GitHub Mobile, available on iOS and Android.

Summary by CodeRabbit

Release Notes

  • New Features

    • Acceptance tests now run in parallel by default for significantly faster execution. Control parallelism via the PARALLEL environment variable (defaults to CPU count). Individual test cases can opt-out using parallel: false configuration.
  • Documentation

    • Added comprehensive guidance on parallel test execution requirements, including sandbox isolation, environment variable handling, and per-test cleanup strategies.

@mergify mergify bot added triage Needs triage wip Work in Progress: Not ready for final review or merge labels Mar 15, 2026
@nitrocode nitrocode added patch A minor, backward compatible change and removed triage Needs triage labels Mar 15, 2026
- Add FilterCIEnvVars() to pkg/telemetry/ci.go as a pure function safe for parallel tests
- Add parallel: *bool field to TestCase struct (default: true, meaning parallel by default)
- Add parallel: false to exec-command terraform tests that share mutable sandbox state
- Pre-initialize atmosRunner in TestMain to avoid data races at test startup
- Replace t.Setenv() calls with direct subprocess env manipulation (cmd.Env)
- Replace t.Chdir() with cmd.Dir to avoid Go testing framework parallel restrictions
- Replace PreserveCIEnvVars/RestoreCIEnvVars with FilterCIEnvVars on inherited env slice
- Remove removeCacheFile() (no-op with per-test XDG dirs) and xdg.Reload()
- Remove config/xdg imports now unused in cli_test.go
- Add parallel field to test case JSON schema
- Update Makefile and collect-coverage.sh to pass -parallel flag
- Add unit tests for FilterCIEnvVars in ci_test.go

Co-authored-by: nitrocode <7775707+nitrocode@users.noreply.github.com>
@mergify
Copy link

mergify bot commented Mar 15, 2026

Important

Cloud Posse Engineering Team Review Required

This pull request modifies files that require Cloud Posse's review. Please be patient, and a core maintainer will review your changes.

To expedite this process, reach out to us on Slack in the #pr-reviews channel.

@mergify mergify bot added the needs-cloudposse Needs Cloud Posse assistance label Mar 15, 2026
…tract gitHubUsernameVars constant, add blog post and roadmap entry

- Add t.Parallel() to each TestFilterCIEnvVars subtest for proper parallel execution
- Fix test indentation formatting in ci_test.go
- Extract gitHubUsernameVars package-level constant to avoid duplication
- Add nitrocode to website/blog/authors.yml
- Add blog post: website/blog/2026-03-15-parallel-acceptance-tests.mdx
- Update roadmap: add parallel-acceptance-tests milestone as shipped (progress 80->82%)
- Update CLAUDE.md with learnings about parallel test infrastructure
- Add *-context.json to .gitignore for terraform-generate-files fixture

Co-authored-by: nitrocode <7775707+nitrocode@users.noreply.github.com>
Copilot AI changed the title [WIP] Optimize acceptance tests for reduced execution time feat(tests): parallel acceptance tests for 4-8x CI speedup Mar 15, 2026
Copilot AI requested a review from nitrocode March 15, 2026 06:27
@mergify mergify bot removed the wip Work in Progress: Not ready for final review or merge label Mar 15, 2026
@github-actions github-actions bot added the size/m Medium size PR label Mar 15, 2026
@github-actions
Copy link

github-actions bot commented Mar 15, 2026

Dependency Review

✅ No vulnerabilities or license issues found.

Snapshot Warnings

⚠️: No snapshots were found for the head SHA 2411d61.
Ensure that dependencies are being submitted on PR branches and consider enabling retry-on-snapshot-warnings. See the documentation for more information and troubleshooting advice.

Scanned Files

None

The parallel refactor replaced t.Chdir(workdir) with cmd.Dir=workdir to
comply with Go 1.24+ restriction that t.Chdir() cannot be combined with
t.Parallel(). However, this broke verifyFileExists, verifyFileNotExists,
and verifyFileContains which resolve file paths against the test process CWD
(which is no longer the workdir).

Fix by resolving relative file paths in FileExists/FileNotExists/FileContains
against absoluteWorkdir before passing them to the verify functions, using
two new helpers: resolveFilePaths and resolveFilePathsMap.

This fixes:
- generate_files_creates_files
- atmos_docs_generate_for_terraform_docs
- atmos_docs_generate_with_remote_template
- atmos_docs_generate_input_variants

Co-authored-by: nitrocode <7775707+nitrocode@users.noreply.github.com>
Copilot AI changed the title feat(tests): parallel acceptance tests for 4-8x CI speedup fix(tests): resolve file paths against workdir in parallel tests Mar 15, 2026
…dering races

The 4 tests in atmos-functions.yaml share a named sandbox and have an ordering
dependency: test 4 ('!terraform.output from component with !env function test')
reads terraform state for component-5 which is created by test 3 ('!env function
test'). Running these in parallel caused test 4 to fail because component-5 had
no state yet.

Additionally update CLAUDE.md and add clarifying comment explaining cmd.Dir
only affects subprocess CWD, not the test process CWD.

Co-authored-by: nitrocode <7775707+nitrocode@users.noreply.github.com>
Copilot AI changed the title fix(tests): resolve file paths against workdir in parallel tests feat(tests): parallel acceptance tests for 4-8x CI speedup Mar 15, 2026
@codecov
Copy link

codecov bot commented Mar 16, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (main@38bbe99). Learn more about missing BASE report.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #2205   +/-   ##
=======================================
  Coverage        ?   77.20%           
=======================================
  Files           ?     1013           
  Lines           ?    95757           
  Branches        ?        0           
=======================================
  Hits            ?    73930           
  Misses          ?    17652           
  Partials        ?     4175           
Flag Coverage Δ
unittests 77.20% <100.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
pkg/telemetry/ci.go 96.96% <100.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@mergify

This comment was marked as outdated.

@mergify mergify bot added the conflict This PR has conflicts label Mar 24, 2026
@nitrocode

This comment was marked as outdated.

@mergify mergify bot removed the conflict This PR has conflicts label Mar 24, 2026
@coderabbitai

This comment was marked as outdated.

@coderabbitai

This comment was marked as outdated.

@nitrocode

This comment was marked as outdated.

@coderabbitai

This comment was marked as outdated.

@coderabbitai

This comment was marked as outdated.

…g, coverage script

Co-authored-by: nitrocode <7775707+nitrocode@users.noreply.github.com>
Agent-Logs-Url: https://github.com/cloudposse/atmos/sessions/3e449db6-b584-4cf5-be4d-504f3aaa68e8
@nitrocode

This comment was marked as outdated.

@coderabbitai

This comment was marked as outdated.

@coderabbitai

This comment was marked as outdated.

…I filter test

Co-authored-by: nitrocode <7775707+nitrocode@users.noreply.github.com>
Agent-Logs-Url: https://github.com/cloudposse/atmos/sessions/7c80a5d6-c3c1-4558-8af1-6ff6e1a4d1da
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 24, 2026

📝 Walkthrough

Walkthrough

This PR enables parallel test execution for Atmos acceptance tests by default. It introduces parallelism configuration in CI workflows and the Makefile, refactors test infrastructure to support isolation (temp directories, env filtering, coverage merging), and adds extensive documentation of the parallel execution model with opt-out support via YAML test configuration.

Changes

Cohort / File(s) Summary
CI and Build Configuration
.github/workflows/test.yml, Makefile, scripts/collect-coverage.sh
Added PARALLEL environment variable and make target configuration to cap test parallelism at 4 (CI) or detected CPU count (local). Updated testacc and testacc-cover to pass -parallel flag to go test and collect coverage with parallelism awareness.
Test Environment Filtering
pkg/telemetry/ci.go, pkg/telemetry/ci_test.go
Introduced FilterCIEnvVars() helper and concurrency-safe caching via sync.Once to remove CI-related environment variables from subprocess environments. Added 6 test subtests validating filtering behavior, preservation of non-CI vars, and handling of malformed entries.
Test Isolation and Coverage Helpers
tests/cli_coverage_test.go, testhelpers/sandbox.go
Added mergeIntoCoverDir() and copyFile() helpers to merge per-subprocess coverage output into shared directories with atomic metadata handling. Updated copyFile in sandbox.go to use streaming I/O instead of loading entire files into memory.
CLI Output and Verification Utilities
tests/cli_output_test.go, tests/cli_verify_test.go, tests/cli_snapshot_test.go
New output sanitization module for snapshot comparisons (Git root resolution, path normalization, credential masking, timestamp normalization, TTY rendering). Added verification functions for OS patterns, exit codes, output regexes, file existence, and format validation (JSON/YAML). Enhanced snapshot handling with line-ending normalization and separate TTY vs non-TTY diff reporting.
Test Execution Framework
tests/cli_run_test.go, tests/cli_sandbox_test.go, tests/cli_test.go
Moved core test orchestration logic into runCLICommandTest(), enabling parallel execution with per-test temp directories, environment isolation (XDG cache, Git config), dotfile copying, sandbox support, and GitHub output file cleanup. Added sandbox registry and isolated sandbox creation helpers. Refactored TestMain to eagerly build atmosRunner once before tests run.
Test Case Configuration
tests/test-cases/schema.json, tests/test-cases/atmos-functions.yaml, tests/test-cases/exec-command.yaml, tests/test-cases/native-ci-*.yaml
Added parallel field (default true) to schema. Set parallel: false for test cases sharing named sandboxes (atmos-functions, exec-command). Updated native-ci tests to use sandbox: true and scenario-specific GITHUB_OUTPUT/GITHUB_STEP_SUMMARY filenames to avoid collision.
Test Configuration and Fixtures
.gitignore, tests/fixtures/scenarios/terraform-generate-files/.gitignore
Added patterns to ignore CI integration test artifacts: github-output-*.txt and github-step-summary-*.txt in test output directories.
Documentation
CLAUDE.md, website/blog/2026-03-15-parallel-acceptance-tests.mdx, website/src/data/roadmap.js, website/blog/authors.yml
Added detailed developer guidance on parallel test constraints, environment isolation, Terraform state sandboxing, and coverage merging. Published blog post describing parallel test execution model, performance measurements, and opt-out mechanism. Updated roadmap to mark feature shipped in q1-2026. Added author entry.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

The changes span multiple functional areas (CI automation, test infrastructure, test cases, documentation) with substantial new test helpers containing complex logic for output normalization, snapshot handling, and environment isolation. While many files follow consistent patterns, the heterogeneous nature—combining refactored core logic, new isolation helpers, output sanitization pipelines, and configuration updates—demands separate reasoning for each cohort and careful validation of edge cases around concurrent file operations and environment variable filtering.

Possibly related PRs

Suggested labels

minor

Suggested reviewers

  • aknysh
  • osterman
🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 65.57% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: enabling parallel execution of acceptance tests with proper isolation, which is the central objective of this large refactoring across test infrastructure.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • ✅ Committed to branch successfully - (🔄 Check to regenerate)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch copilot/reduce-acceptance-test-time

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

❤️ Share

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

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🧹 Nitpick comments (2)
pkg/telemetry/ci_test.go (1)

329-418: Refactor TestFilterCIEnvVars to a table-driven structure.

This block covers multiple scenarios with repeated setup/assert flow; a table-driven form will reduce duplication and make case expansion safer.

As per coding guidelines "Use table-driven tests for testing multiple scenarios in Go."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@pkg/telemetry/ci_test.go` around lines 329 - 418, The test
TestFilterCIEnvVars contains multiple subtests with repeated setup/assert
patterns; refactor it to a table-driven test that iterates over a slice of cases
(structs with name, input, expectedResult/expectedContains/expectedNotContains,
and any flags like preserveOriginal) and runs t.Run(case.name, func(t
*testing.T){ ... }); keep references to FilterCIEnvVars, and preserve existing
behaviors (empty input, entries without '=', original-slice immutability,
GOCOVERDIR exemption) by encoding those expectations in each case and asserting
the same conditions inside the loop.
tests/cli_sandbox_test.go (1)

73-100: Consider documenting that clean: true should not be used with shared named sandboxes.

cleanDirectory runs git clean -fd on the workdir. If two parallel tests share a named sandbox and both set clean: true, they could race. The current design assumes clean: true is used with isolated sandboxes (which are test-scoped). Consider adding a comment or guard.

📝 Optional: Add clarifying comment
 // Clean up untracked files in the working directory.
 //
 // Uses `git clean -fd` scoped to the workdir subtree, which is significantly faster than
 // calling go-git's Worktree.Status() which scans the entire repository.
+//
+// WARNING: Do not use with named sandboxes shared across parallel tests.
+// Use only with isolated sandboxes (sandbox: true) or sequential tests (parallel: false).
 func cleanDirectory(t *testing.T, workdir string) error {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/cli_sandbox_test.go` around lines 73 - 100, Add documentation and a
runtime guard around cleanDirectory to warn/prevent use of clean: true on shared
named sandboxes: update the comment above the cleanDirectory function (and/or
near the code that parses sandbox options) to state that clean: true must only
be used with isolated, test-scoped sandboxes and can race when multiple tests
share a named sandbox; additionally, add a guard in the sandbox setup logic that
detects a named/shared sandbox and returns an error or ignores clean: true to
avoid concurrent git clean races (reference cleanDirectory and the place where
sandbox options like clean: true and sandbox name are handled).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@CLAUDE.md`:
- Line 242: Update the CI-vars bullet in CLAUDE.md by replacing "e.g." with the
standard punctuation "e.g.," so the sentence reads: "CI vars are filtered from
the **inherited** process env before `tc.Env` is merged, so tests can explicitly
re-add CI vars (e.g., `CI: \"true\"`) without them being filtered out." Locate
the sentence containing "CI vars are filtered from the **inherited** process env
before `tc.Env` is merged..." and apply this punctuation change.

In `@tests/cli_verify_test.go`:
- Around line 123-126: The verifyFileExists test currently treats only
os.ErrNotExist as a failure and ignores other os.Stat errors; change the os.Stat
handling in verifyFileExists so any non-nil err is treated as a failure (mirror
the verifyFileNotExists pattern): call os.Stat(file), if errors.Is(err,
os.ErrNotExist) report the expected-file-missing error and set success=false,
else if err != nil report an unexpected os.Stat error (include err) and set
success=false; ensure you reference verifyFileExists, os.Stat and errors.Is(err,
os.ErrNotExist) when making the change.

In `@website/blog/2026-03-15-parallel-acceptance-tests.mdx`:
- Around line 92-97: The ActionCard instance with prop
actionUrl="/contribute/tests" points to a non-existent docs page; either change
the ActionCard's actionUrl prop to an existing contribute page (for example use
the contributing page route) or add the missing tests doc at the contribute docs
location; update the ActionCard's actionUrl value in the component usage
(ActionCard title="Explore the Atmos test infrastructure") to the correct route
or create the new docs file named tests.mdx under the contribute docs so the
link resolves.

In `@website/blog/authors.yml`:
- Around line 79-84: Remove the duplicate YAML mapping for the author key
"nitrocode" (the second occurrence) so there is only one "nitrocode" entry in
authors.yml; if you intended to change the author metadata (e.g., update title
to "Senior Software Engineer @ Cloud Posse" or adjust url/image_url), apply
those edits to the original "nitrocode" mapping instead of adding a new one,
ensuring the final YAML contains a single, updated "nitrocode" mapping with the
correct name, title, url, and image_url fields.

---

Nitpick comments:
In `@pkg/telemetry/ci_test.go`:
- Around line 329-418: The test TestFilterCIEnvVars contains multiple subtests
with repeated setup/assert patterns; refactor it to a table-driven test that
iterates over a slice of cases (structs with name, input,
expectedResult/expectedContains/expectedNotContains, and any flags like
preserveOriginal) and runs t.Run(case.name, func(t *testing.T){ ... }); keep
references to FilterCIEnvVars, and preserve existing behaviors (empty input,
entries without '=', original-slice immutability, GOCOVERDIR exemption) by
encoding those expectations in each case and asserting the same conditions
inside the loop.

In `@tests/cli_sandbox_test.go`:
- Around line 73-100: Add documentation and a runtime guard around
cleanDirectory to warn/prevent use of clean: true on shared named sandboxes:
update the comment above the cleanDirectory function (and/or near the code that
parses sandbox options) to state that clean: true must only be used with
isolated, test-scoped sandboxes and can race when multiple tests share a named
sandbox; additionally, add a guard in the sandbox setup logic that detects a
named/shared sandbox and returns an error or ignores clean: true to avoid
concurrent git clean races (reference cleanDirectory and the place where sandbox
options like clean: true and sandbox name are handled).
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 27747df9-62c1-4434-82e5-ae9007c597fa

📥 Commits

Reviewing files that changed from the base of the PR and between 01541b6 and 4a9db30.

📒 Files selected for processing (24)
  • .github/workflows/test.yml
  • .gitignore
  • CLAUDE.md
  • Makefile
  • pkg/telemetry/ci.go
  • pkg/telemetry/ci_test.go
  • scripts/collect-coverage.sh
  • tests/cli_coverage_test.go
  • tests/cli_output_test.go
  • tests/cli_run_test.go
  • tests/cli_sandbox_test.go
  • tests/cli_snapshot_test.go
  • tests/cli_test.go
  • tests/cli_verify_test.go
  • tests/fixtures/scenarios/terraform-generate-files/.gitignore
  • tests/test-cases/atmos-functions.yaml
  • tests/test-cases/exec-command.yaml
  • tests/test-cases/native-ci-gha-plan.yaml
  • tests/test-cases/native-ci-planfile.yaml
  • tests/test-cases/schema.json
  • tests/testhelpers/sandbox.go
  • website/blog/2026-03-15-parallel-acceptance-tests.mdx
  • website/blog/authors.yml
  • website/src/data/roadmap.js


**Parallel Rules for TestCLI:**
- `t.Setenv()` and `t.Chdir()` are **incompatible** with `t.Parallel()` in Go 1.24+. Use direct `cmd.Env` manipulation and `cmd.Dir` instead.
- CI vars are filtered from the **inherited** process env before `tc.Env` is merged, so tests can explicitly re-add CI vars (e.g. `CI: "true"`) without them being filtered out.
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Fix punctuation in the CI-vars bullet.

Line 242 should use e.g., for standard punctuation/readability.

✏️ Suggested edit
-- CI vars are filtered from the **inherited** process env before `tc.Env` is merged, so tests can explicitly re-add CI vars (e.g. `CI: "true"`) without them being filtered out.
+- CI vars are filtered from the **inherited** process env before `tc.Env` is merged, so tests can explicitly re-add CI vars (e.g., `CI: "true"`) without them being filtered out.
📝 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.

Suggested change
- CI vars are filtered from the **inherited** process env before `tc.Env` is merged, so tests can explicitly re-add CI vars (e.g. `CI: "true"`) without them being filtered out.
- CI vars are filtered from the **inherited** process env before `tc.Env` is merged, so tests can explicitly re-add CI vars (e.g., `CI: "true"`) without them being filtered out.
🧰 Tools
🪛 LanguageTool

[style] ~242-~242: A comma is missing here.
Context: ...so tests can explicitly re-add CI vars (e.g. CI: "true") without them being filter...

(EG_NO_COMMA)


[typographical] ~242-~242: Consider using typographic quotation marks here.
Context: ...sts can explicitly re-add CI vars (e.g. CI: "true") without them being filtered out. - `a...

(EN_QUOTES)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@CLAUDE.md` at line 242, Update the CI-vars bullet in CLAUDE.md by replacing
"e.g." with the standard punctuation "e.g.," so the sentence reads: "CI vars are
filtered from the **inherited** process env before `tc.Env` is merged, so tests
can explicitly re-add CI vars (e.g., `CI: \"true\"`) without them being filtered
out." Locate the sentence containing "CI vars are filtered from the
**inherited** process env before `tc.Env` is merged..." and apply this
punctuation change.

Comment on lines +123 to +126
if _, err := os.Stat(file); errors.Is(err, os.ErrNotExist) {
t.Errorf("Reason: Expected file does not exist: %q", file)
success = false
}
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

cat -n tests/cli_verify_test.go | sed -n '115,135p'

Repository: cloudposse/atmos

Length of output: 724


🏁 Script executed:

# Check if there are any tests that call verifyFileExists to understand usage
rg -A 5 "verifyFileExists" tests/cli_verify_test.go | head -40

Repository: cloudposse/atmos

Length of output: 309


🏁 Script executed:

# Check the full context of verifyFileNotExists to see the pattern difference
cat -n tests/cli_verify_test.go | sed -n '131,139p'

Repository: cloudposse/atmos

Length of output: 493


🏁 Script executed:

# Verify the context around both functions and check for any other related verification helpers
cat -n tests/cli_verify_test.go | sed -n '120,145p'

Repository: cloudposse/atmos

Length of output: 999


Fail verifyFileExists on any unexpected os.Stat error.

Line 123 treats only ErrNotExist as a failure. Permission or I/O errors are silently treated as success and can mask real failures. The companion function verifyFileNotExists (line 137–139) already handles this correctly—apply the same pattern here.

🛠️ Proposed fix
 func verifyFileExists(t *testing.T, files []string) bool {
 	success := true
 	for _, file := range files {
-		if _, err := os.Stat(file); errors.Is(err, os.ErrNotExist) {
-			t.Errorf("Reason: Expected file does not exist: %q", file)
-			success = false
-		}
+		if _, err := os.Stat(file); err != nil {
+			if errors.Is(err, os.ErrNotExist) {
+				t.Errorf("Reason: Expected file does not exist: %q", file)
+			} else {
+				t.Errorf("Reason: Unexpected error checking file %q: %v", file, err)
+			}
+			success = false
+		}
 	}
 	return success
 }
📝 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.

Suggested change
if _, err := os.Stat(file); errors.Is(err, os.ErrNotExist) {
t.Errorf("Reason: Expected file does not exist: %q", file)
success = false
}
if _, err := os.Stat(file); err != nil {
if errors.Is(err, os.ErrNotExist) {
t.Errorf("Reason: Expected file does not exist: %q", file)
} else {
t.Errorf("Reason: Unexpected error checking file %q: %v", file, err)
}
success = false
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/cli_verify_test.go` around lines 123 - 126, The verifyFileExists test
currently treats only os.ErrNotExist as a failure and ignores other os.Stat
errors; change the os.Stat handling in verifyFileExists so any non-nil err is
treated as a failure (mirror the verifyFileNotExists pattern): call
os.Stat(file), if errors.Is(err, os.ErrNotExist) report the
expected-file-missing error and set success=false, else if err != nil report an
unexpected os.Stat error (include err) and set success=false; ensure you
reference verifyFileExists, os.Stat and errors.Is(err, os.ErrNotExist) when
making the change.

Comment on lines +92 to +97
<ActionCard
title="Explore the Atmos test infrastructure"
description="Learn how to write and run acceptance tests for Atmos."
actionText="View test documentation"
actionUrl="/contribute/tests"
/>
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if the contribute/tests documentation exists
fd -t f "tests" website/docs/contribute/ || echo "No tests doc found in contribute/"
fd -t f "tests.mdx" website/ || echo "No tests.mdx found"

Repository: cloudposse/atmos

Length of output: 113


🏁 Script executed:

# Check the structure of website/docs/contribute directory
find website/docs/contribute -type f -name "*.mdx" | head -20

Repository: cloudposse/atmos

Length of output: 132


🏁 Script executed:

# Also check if there's a tests.mdx file anywhere in docs
fd -t f "tests.mdx" website/docs/

Repository: cloudposse/atmos

Length of output: 42


🏁 Script executed:

# Look for any files with "test" in their name within contribute section
fd "test" website/docs/contribute/

Repository: cloudposse/atmos

Length of output: 42


🏁 Script executed:

# Search for any documentation about tests in the entire website/docs directory
rg -i "tests\|acceptance.*test" website/docs/ --max-count=5 -l

Repository: cloudposse/atmos

Length of output: 42


🏁 Script executed:

# Check the docusaurus config to understand how URLs are generated
cat website/docusaurus.config.js | head -50

Repository: cloudposse/atmos

Length of output: 1746


🏁 Script executed:

# Check the docusaurus config for redirects related to tests or contribute
sed -n '40,150p' website/docusaurus.config.js

Repository: cloudposse/atmos

Length of output: 5498


🏁 Script executed:

# List all .mdx files in website/docs to get full picture of documentation structure
find website/docs -name "*.mdx" -type f | sort

Repository: cloudposse/atmos

Length of output: 25345


Fix broken documentation link.

The actionUrl="/contribute/tests" references a non-existent documentation page. The contribute section only has coc.mdx and contributing.mdx. Either update the link to an existing page or create the missing tests documentation at website/docs/contribute/tests.mdx. The build is configured to throw on broken links, so this will cause the website build to fail.

🧰 Tools
🪛 GitHub Actions: Website Preview Build

[error] Step 'npm run build:site' (runs 'npm run build') failed with exit code 1. Error: Unable to build website for locale en.


[error] Step 'fetch-latest-release' attempted 1/3 failed during site build: 'This operation was aborted. Retrying in 1000ms...'

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@website/blog/2026-03-15-parallel-acceptance-tests.mdx` around lines 92 - 97,
The ActionCard instance with prop actionUrl="/contribute/tests" points to a
non-existent docs page; either change the ActionCard's actionUrl prop to an
existing contribute page (for example use the contributing page route) or add
the missing tests doc at the contribute docs location; update the ActionCard's
actionUrl value in the component usage (ActionCard title="Explore the Atmos test
infrastructure") to the correct route or create the new docs file named
tests.mdx under the contribute docs so the link resolves.

Comment on lines +79 to +84

nitrocode:
name: nitrocode
title: Senior Software Engineer @ Cloud Posse
url: https://github.com/nitrocode
image_url: https://github.com/nitrocode.png
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Duplicate nitrocode key breaks the build.

This entry duplicates the existing nitrocode author defined at lines 8-12. YAML mappings don't allow duplicate keys, and the Docusaurus build is failing because of this.

If the intent is to update the title to "Senior Software Engineer @ Cloud Posse", modify the existing entry at lines 8-12 instead.

Proposed fix: update existing entry and remove duplicate

Remove lines 79-84 entirely, then update the existing entry:

 nitrocode:
   name: nitrocode
-  title: CEO of Infralicious
+  title: Senior Software Engineer @ Cloud Posse
   url: https://github.com/nitrocode
   image_url: https://github.com/nitrocode.png
📝 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.

Suggested change
nitrocode:
name: nitrocode
title: Senior Software Engineer @ Cloud Posse
url: https://github.com/nitrocode
image_url: https://github.com/nitrocode.png
🧰 Tools
🪛 GitHub Actions: Website Preview Build

[error] 80-80: Docusaurus build failed: YAMLException in authors file. File is invalid due to duplicated mapping key (reason: 'duplicated mapping key').


[error] Step 'npm run build:site' (runs 'npm run build') failed with exit code 1. Error: Unable to build website for locale en.


[error] Step 'fetch-latest-release' attempted 1/3 failed during site build: 'This operation was aborted. Retrying in 1000ms...'

🪛 YAMLlint (1.38.0)

[error] 80-80: duplication of key "nitrocode" in mapping

(key-duplicates)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@website/blog/authors.yml` around lines 79 - 84, Remove the duplicate YAML
mapping for the author key "nitrocode" (the second occurrence) so there is only
one "nitrocode" entry in authors.yml; if you intended to change the author
metadata (e.g., update title to "Senior Software Engineer @ Cloud Posse" or
adjust url/image_url), apply those edits to the original "nitrocode" mapping
instead of adding a new one, ensuring the final YAML contains a single, updated
"nitrocode" mapping with the correct name, title, url, and image_url fields.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 24, 2026

Note

Docstrings generation - SUCCESS
Generated docstrings and committed to branch copilot/reduce-acceptance-test-time (commit: 2411d61f00416c9f0eb12890cf840325823b1f45)

Docstrings generation was requested by @nitrocode.

The following files were modified:

* `pkg/telemetry/ci.go`
* `tests/testhelpers/sandbox.go`

These files were ignored:
* `pkg/telemetry/ci_test.go`
* `tests/cli_coverage_test.go`
* `tests/cli_output_test.go`
* `tests/cli_run_test.go`
* `tests/cli_sandbox_test.go`
* `tests/cli_snapshot_test.go`
* `tests/cli_test.go`
* `tests/cli_verify_test.go`

These file types are not supported:
* `.github/workflows/test.yml`
* `.gitignore`
* `CLAUDE.md`
* `Makefile`
* `tests/fixtures/scenarios/terraform-generate-files/.gitignore`
* `tests/test-cases/atmos-functions.yaml`
* `tests/test-cases/exec-command.yaml`
* `tests/test-cases/native-ci-gha-plan.yaml`
* `tests/test-cases/native-ci-planfile.yaml`
* `tests/test-cases/schema.json`
* `website/blog/2026-03-15-parallel-acceptance-tests.mdx`
* `website/blog/authors.yml`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-cloudposse Needs Cloud Posse assistance patch A minor, backward compatible change size/l Large size PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants