Added tests #84

goruha · 2025-10-23T20:01:41Z

what

Added tests
Resolve cycled dependency between cloud trail and cloud trail bucket logs

Why

Ensure that the component works stably
Solve Deploy Fails - CreateTrail InsufficientS3BucketPolicyException: Incorrect S3 #70

Summary by CodeRabbit

Release Notes

New Features
- S3 object lock configuration is now conditionally applied based on configuration settings—disabled when lock days are set to zero.
Tests
- Added comprehensive test suite for component validation with multiple test scenarios.
- Established test fixtures and configuration infrastructure for integration testing.
Chores
- Updated test infrastructure and build configuration.
- Enhanced git ignore rules for test artifacts and temporary files.

coderabbitai · 2025-10-23T20:02:05Z

Walkthrough

The PR renames the CloudTrail label module to cloudtrail_bucket_label, updates all related ARN and context references in IAM policies, adds conditional object lock configuration based on days variables, introduces a default null value for query_override, and establishes comprehensive test infrastructure with Go tests and Atmos fixture configurations.

Changes

Cohort / File(s)	Summary
Terraform Configuration `src/main.tf`, `src/variables.tf`	Module renamed from cloudtrail_label to cloudtrail_bucket_label with updated IAM policy ARN references. Object lock configuration made conditional (null when days variable equals 0). Added default = null to query_override variable.
Test Suite Implementation `test/component_test.go`	New Go test file introducing ComponentSuite struct with TestBasic, TestEnabledFlag, SetupSuite, TearDownSuite methods. Tests deploy component, manage Datadog API credentials via SSM, verify enabled flag behavior, and handle cleanup via helper.Run.
Test Fixtures - Configuration `test/fixtures/atmos.yaml`, `test/fixtures/vendor.yaml`, `test/fixtures/stacks/catalog/.yaml`, `test/fixtures/stacks/orgs/default/test/.yaml`	Added Atmos base configuration, vendor manifest for component sources, catalog components (account-map, datadog-configuration, datadog-integration), use-case fixtures (basic/disabled), stack defaults with account mapping, and tests.yaml importing all fixtures.
Test Module and Metadata `test/.gitignore`, `test/go.mod`, `test/run.sh`	Updated .gitignore with state/, .cache, .atmos, test-suite.json, test_suite.yaml entries. Added Go module definition (go 1.23.4) with dependencies. Emptied run.sh script.
Documentation `test/README.md`	Deleted placeholder README file.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

The change involves moderate heterogeneity across Terraform configuration updates (module rename with dependent reference changes), new Go test logic with multi-method test suite implementation, and extensive configuration fixture files. While individual pieces follow clear patterns, the scope spans multiple file types and requires verification of consistency across module references and correctness of test orchestration logic.

Possibly related PRs

Fix cloudtrail s3 bucket name #82: Introduces cloudtrail_label module and IAM policy references; this PR renames and updates those references throughout.

Suggested labels

needs-test, minor

Poem

🐰 A refactor hops through the code with grace,
cloudtrail_bucket_label takes its place,
With fixtures stacked and tests that gleam,
The CloudTrail streams now follow the scheme!
Lock and key dance when days align,
Test fixtures glow—infrastructure divine! ✨

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Title Check	⚠️ Warning	The title "Added tests" is only partially related to the changeset. While the PR does add test files (test/component_test.go, test fixtures, and test configuration), this represents a minor portion of the overall changes. The primary modifications in this PR are substantial refactoring in src/main.tf—including renaming the cloudtrail_label module to cloudtrail_bucket_label, updating IAM policy documents, and adding conditional object lock configuration—plus changes to the public API in src/variables.tf (adding a default value to query_override). According to the PR description, the PR also aims to resolve a cyclic dependency issue (#70). The title completely omits these significant infrastructure and API changes, making it misleading to someone scanning the commit history.	Update the title to highlight the primary infrastructure changes and refactoring, such as "Refactor cloudtrail label module and add tests" or "Resolve cyclic dependency with conditional object lock configuration and tests." This would accurately convey that the PR contains significant production code changes beyond just test additions, helping reviewers and future developers understand the scope of the changes at a glance.
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.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
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

✨ 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 add-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.

❤️ Share

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

mergify · 2025-10-23T20:02:22Z

Important

Do not edit the README.md directly. It's auto-generated from the README.yaml

Please update the README.yaml file instead.

Could you fix it @goruha? 🙏

goruha · 2025-10-23T20:06:05Z

/terratest

coderabbitai

Actionable comments posted: 3

🧹 Nitpick comments (1)

test/go.mod (1)

17-17: Remove commented-out replace directive if not needed.

The commented replace directive for aws-nuke appears to be unused. If it's not needed, remove it to keep the module file clean. If it's needed for documentation, consider adding a comment explaining why it's kept.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d8a88e8 and 215bee1.

⛔ Files ignored due to path filters (1)

test/go.sum is excluded by !**/*.sum

📒 Files selected for processing (16)

src/main.tf (10 hunks)
src/variables.tf (1 hunks)
test/.gitignore (1 hunks)
test/README.md (0 hunks)
test/component_test.go (1 hunks)
test/fixtures/atmos.yaml (1 hunks)
test/fixtures/stacks/catalog/account-map.yaml (1 hunks)
test/fixtures/stacks/catalog/datadog-configuration.yaml (1 hunks)
test/fixtures/stacks/catalog/datadog-integration.yaml (1 hunks)
test/fixtures/stacks/catalog/usecase/basic.yaml (1 hunks)
test/fixtures/stacks/catalog/usecase/disabled.yaml (1 hunks)
test/fixtures/stacks/orgs/default/test/_defaults.yaml (1 hunks)
test/fixtures/stacks/orgs/default/test/tests.yaml (1 hunks)
test/fixtures/vendor.yaml (1 hunks)
test/go.mod (1 hunks)
test/run.sh (0 hunks)

💤 Files with no reviewable changes (2)

test/run.sh
test/README.md

🧰 Additional context used

📓 Path-based instructions (6)

**/*.{yaml,yml,md}