fix: Removing filter enabled check on empty stacks #4992

yhakbar · 2025-10-20T14:21:57Z

Description

Prevents errors when users don't discover any configurations as a result of exclusions, etc.

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.
Run the relevant tests successfully, including pre-commit checks.
Include release notes. If this PR is backward incompatible, include a migration guide.

Release Notes (draft)

Added / Removed / Updated [X].

Migration Guide

Summary by CodeRabbit

Release Notes

Bug Fixes
- Improved behavior when no Terragrunt configuration units are discovered. The system now logs a warning and continues gracefully instead of failing with an error.

…mpty runs

vercel · 2025-10-20T14:22:03Z

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

Project	Deployment	Preview	Comments	U 8000 pdated (UTC)
terragrunt-docs	Ready	Preview	Comment	Oct 20, 2025 4:12pm

💡 Enable Vercel Agent with $100 free credit for automated AI reviews

coderabbitai · 2025-10-20T14:22:28Z

📝 Walkthrough

Walkthrough

The PR removes an exported error constant and refactors the runner initialization logic to create an empty runner with a warning instead of returning an error when no Terragrunt configuration units are discovered. Integration tests are updated to reflect the new behavior.

Changes

Cohort / File(s)	Summary
Error definition removal `internal/runner/common/errors.go`	Removed exported global variable `ErrNoUnitsFound`
Runner initialization logic `internal/runner/runnerpool/runner.go`	Changed `NewRunnerPoolStack` to unconditionally create and return an empty runner when no units are found, instead of conditionally returning `ErrNoUnitsFound`; now logs a warning message
Integration tests `test/integration_run_test.go`, `test/integration_stacks_test.go`	Updated test expectations to reflect new behavior: removed error assertions, added warning message assertions, and adjusted test case expectations for scenarios with no units discovered

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant Caller
    participant Runner as NewRunnerPoolStack
    participant Logger
    participant Stack

    rect rgb(200, 220, 255)
    Note over Runner: Old Behavior (Error Path)
    Caller->>Runner: Call with no units found
    alt Filtering enabled
        Runner->>Stack: Build empty stack
        Runner->>Caller: Return runner with empty stack
    else Filtering disabled
        Runner->>Caller: Return ErrNoUnitsFound ❌
    end
    end

    rect rgb(200, 255, 220)
    Note over Runner: New Behavior (Empty Runner Path)
    Caller->>Runner: Call with no units found
    Runner->>Logger: Log warning: "No units discovered"
    Runner->>Stack: Build empty stack
    Stack->>Runner: Empty stack created
    Runner->>Caller: Return runner with empty stack ✓
    end

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

The changes involve a behavioral shift in error handling that affects core runner initialization and requires coordinated test updates. While the individual edits are straightforward, understanding the impact of moving from error-based to empty-runner-based handling across initialization, logging, and test expectations demands moderate review attention.

Suggested reviewers

denis256
wakeful

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Description Check	⚠️ Warning	The pull request description follows the required template structure with sections for Description, TODOs, Release Notes, and Migration Guide. However, critical sections are incomplete: the issue reference (Fixes #000) is missing from the Description section, and the Release Notes section contains only placeholder text ("Added / Removed / Updated [X].") rather than an actual one-line description of what was changed. While the TODOs checklist is fully completed and the general intent is clear from the description text, the missing issue reference and unfilled release notes represent incomplete required fields that prevent this from being a complete submission.	To resolve this, please: (1) add the GitHub issue number to the Description section in the format "Fixes #XXXX", (2) replace the placeholder Release Notes text with an actual one-line description of the changes (for example, "Fixed empty stack discovery to always create an empty runner instead of throwing an error when no configurations are found"). The Migration Guide section may remain empty if this change is not backward incompatible.
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Title Check	✅ Passed	The pull request title "fix: Removing filter enabled check on empty stacks" is directly related to the core changes made in this PR. The changeset removes the filter-enabled conditional check that previously returned an error when no units were discovered, and instead now always creates an empty runner. The title accurately captures what was removed and references the relevant functionality (empty stacks handling). While the title could be slightly more specific about the resulting behavior, it is clear enough that a teammate scanning the commit history would understand the primary change involves modifying error handling for empty stacks.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch fix/removing-filter-enabled-check

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c7c7296 and 67ba102.

📒 Files selected for processing (4)

internal/runner/common/errors.go (0 hunks)
internal/runner/runnerpool/runner.go (1 hunks)
test/integration_run_test.go (1 hunks)
test/integration_stacks_test.go (1 hunks)

💤 Files with no reviewable changes (1)

internal/runner/common/errors.go

🧰 Additional context used

📓 Path-based instructions (1)

**/*.go

⚙️ CodeRabbit configuration file

Review the Go code for quality and correctness. Make sure that the Go code follows best practices, is performant, and is easy to understand and maintain.

Files:

test/integration_run_test.go
test/integration_stacks_test.go
internal/runner/runnerpool/runner.go

🧬 Code graph analysis (2)

test/integration_stacks_test.go (1)

test/helpers/package.go (1)

RunTerragruntCommandWithOutput (993-997)

internal/runner/runnerpool/runner.go (5)

options/options.go (1)

TerragruntOptions (100-334)

internal/runner/common/options.go (1)

Option (23-25)

internal/runner/common/stack.go (1)

Stack (15-21)

config/config.go (1)

DefaultParserOptions (101-129)

internal/queue/queue.go (1)

NewQueue (213-291)

⏰ 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). (1)

GitHub Check: Pull Request has non-contributor approval

🔇 Additional comments (4)

internal/runner/runnerpool/runner.go (1)

53-75: LGTM! Empty runner creation logic is sound.

The refactored logic correctly handles the case when no units are discovered by:

Logging an appropriate warning message

Creating an empty stack with required options

Creating an empty queue with proper error handling

Returning a properly configured runner

This change aligns with the PR objective to prevent errors when filtering results in empty discovery.

test/integration_run_test.go (1)

80-80: LGTM! Test expectation correctly updated.

The change from shouldFail: true to shouldFail: false correctly reflects the new behavior where empty discovery creates an empty runner with a warning instead of returning an error.

test/integration_stacks_test.go (2)

212-212: LGTM! Variable shadowing removed.

Good cleanup - the inner err variable is no longer shadowed, correctly reusing the outer variable declared at line 208.

216-219: LGTM! Test correctly verifies empty runner behavior.

The test properly validates the new behavior by:

Expecting no error when no units are discovered

Asserting that stderr contains the warning message "No units discovered. Creating an empty runner."

This ensures the empty runner path is functioning as intended.

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

yhakbar added 2 commits October 20, 2025 10:18

fix: Removing filter enabled check, along with associated error for e…

726a875

…mpty runs

fix: Adding warning for empty runners

47afa56

vercel bot deployed to Preview October 20, 2025 14:22 View deployment

fix: Fixing assertions for TestStacksNoGenerate

d4bd64c

vercel bot deployed to Preview October 20, 2025 15:51 View deployment

fix: Fixing TestRunNoStacksGenerate

67ba102

vercel bot deployed to Preview October 20, 2025 16:12 View deployment

yhakbar marked this pull request as ready for review October 20, 2025 20:44

yhakbar requested a review from denis256 as a code owner October 20, 2025 20:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix: Removing filter enabled check on empty stacks #4992

fix: Removing filter enabled check on empty stacks #4992

Uh oh!

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Suggested reviewers

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

fix: Removing filter enabled check on empty stacks #4992

Are you sure you want to change the base?

fix: Removing filter enabled check on empty stacks #4992

Conversation

Uh oh!

Description

TODOs

Release Notes (draft)

Migration Guide

Summary by CodeRabbit

Release Notes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Suggested reviewers

Pre-merge checks and finishing touches

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant