Skip to content

fix(test): fix for failing test TestNewSignalsForwarderMultipleUnix - #6713

Merged
denis256 merged 3 commits into
mainfrom
lint-errors-fixes
Aug 20, 2026
Merged

fix(test): fix for failing test TestNewSignalsForwarderMultipleUnix#6713
denis256 merged 3 commits into
mainfrom
lint-errors-fixes

Conversation

@denis256

@denis256 denis256 commented Aug 20, 2026

Copy link
Copy Markdown
Member

Description

Fixes #000.

TODOs

Read the Gruntwork contribution guidelines.

  • I authored this code entirely myself
  • I am submitting code based on open source software (e.g. MIT, MPL-2.0, Apache)
  • I am adding or upgrading a dependency or adapted code and confirm it has a compatible open source license
  • Update the docs.
  • Update the changelog in the docs.
  • Run the relevant tests successfully, including pre-commit checks.
  • This change is backwards compatible.
  • If this change is not forwards compatible (e.g. a new feature), it is gated behind a feature flag.

Summary by CodeRabbit

  • Tests
    • Improved test reliability for virtual environment depth scenarios.
    • Strengthened Unix signal-forwarding tests by ensuring interrupt-count state is initialized before signals are sent.
    • Improved test signal tracking and readiness synchronization.

@vercel

vercel Bot commented Aug 20, 2026

Copy link
Copy Markdown

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

Project Deployment Actions Updated (UTC)
terragrunt-docs Ready Ready Preview Aug 20, 2026 4:45pm

Request Review

@coderabbitai

coderabbitai Bot commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 059961ce-12db-4ec8-b0e0-28d85355bdf1

📥 Commits

Reviewing files that changed from the base of the PR and between 0fb4f79 and 14417c1.

📒 Files selected for processing (4)
  • internal/cas/clone_e2e_test.go
  • internal/cas/stacks_test.go
  • internal/os/exec/cmd_unix_test.go
  • internal/os/exec/testdata/test_sigint_multiple.sh

Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.


📝 Walkthrough

Walkthrough

The changes isolate CAS test environments and update Unix signal-test synchronization. The child script publishes interrupt counts after trap handling, and the Go test waits for the initial zero count before sending signals.

Changes

Test reliability updates

Layer / File(s) Summary
Use isolated test environments
internal/cas/clone_e2e_test.go, internal/cas/stacks_test.go
CAS tests now use venvtest.NewOSWithEmptyEnv() instead of venv.OSVenv().
Synchronize signal-count publication
internal/os/exec/testdata/test_sigint_multiple.sh, internal/os/exec/cmd_unix_test.go
The child script publishes changed interrupt counts after trap handling. The Unix test waits for an initial interrupt count of zero before sending signals.

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

Merge Risk: ⚪ Minimal · up to 14417

This test-only change has no actionable merge-blocking risk; the ShellCheck suppression should receive normal owner confirmation.

Suggested reviewers: yhakbar

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description includes the template sections but does not describe the changes, test results, or completed TODO items. Add a concise change summary, replace the placeholder issue reference, and complete the relevant TODO items, including test verification.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the primary change: fixing the failing TestNewSignalsForwarderMultipleUnix test.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch lint-errors-fixes

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.

@denis256 denis256 changed the title fix(test): publish interrupt count outside the INT trap fix(test): fix for failing test TestNewSignalsForwarderMultipleUnix Aug 20, 2026
@denis256
denis256 marked this pull request as ready for review August 20, 2026 17:08
@denis256
denis256 requested a review from yhakbar as a code owner August 20, 2026 17:09
@denis256
denis256 merged commit 27f01f0 into main Aug 20, 2026
87 of 88 checks passed
@denis256
denis256 deleted the lint-errors-fixes branch August 20, 2026 17:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants