cloudposse / cloudposse/atmos

test: integration coverage for generate-planfile hooks.RunAll branch

Open
#2,523 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Go
Stars
1.4k
Forks
175
Avg merge
2d 3h
Merged PRs (30d)
134

Description

### Describe the Test Gap

`cmd/terraform/generate/hooks.go:runGeneratePlanfileHooks` (added in #2497 PR A) calls `hooks.RunAll(event, ...)` when `hooks.HasHooks()` returns true. The `RunAll` branch is currently not exercised by any test — the existing tests use the `examples/demo-stacks` fixture which has no `hooks:` configuration, so `HasHooks()` returns false and the branch is skipped.

This is the most important behavioral contract of the PR (user-defined hooks for `before/after.terraform.generate.planfile`), and it is uncovered.

### Why a unit test isn't enough

`Hooks.RunAll` calls `Hooks.preflight` ([`pkg/hooks/hooks.go#L193-L216`](https://github.com/cloudposse/atmos/blob/main/pkg/hooks/hooks.go#L193-L216)) which has real side effects:

- `resolveDeps` walks atmos config for tool dependency declarations.
- `installDeps` writes installed binaries to disk (toolchain cache).
- `verifyAllBinaries` shells out to verify each hook binary is on PATH.

Triggering this from a `go test` in `cmd/terraform/generate/` would either leak filesystem state, fail in offline environments, or depend on host-installed binaries — none of which is acceptable for a unit test.

### Proposed Solution

Add an integration test under `tests/test-cases/` (or extend an existing scenario) that:

1. Uses a `kind: store` hook (which has no external binary dependency) configured for `before-terraform-generate-planfile` or `after-terraform-generate-planfile`.
2. Configures a `redis` or `inline` store backend with no external dependencies.
3. Invokes `atmos terraform generate planfile -s ` against a mock terraform component.
4. Asserts the hook fired (e.g., the store contains the expected key/value).

Reuse pattern: see `tests/fixtures/scenarios/hooks-test/` ([stacks/stack.yaml](https://github.com/cloudposse/atmos/blob/main/tests/fixtures/scenarios/hooks-test/stacks/stack.yaml)) which already wires a `kind: store` hook for `after-terraform-apply`. A parallel fixture for the generate-planfile events would be a one-file addition.

### Acceptance Criteria

- [ ] New integration test exercises `hooks.RunAll` for `before.terraform.generate.planfile` and `after.terraform.generate.planfile`.
- [ ] Test uses `kind: store` (or another side-effect-free kind) to avoid filesystem / network dependencies in CI.
- [ ] Coverage of `cmd/terraform/generate/hooks.go` reaches ~90% (currently 76.5%).

### References

- Coverage gap noted in #2497 PR A code review.
- Existing pattern: `tests/fixtures/scenarios/hooks-test/stacks/stack.yaml`.
- `Hooks.RunAll` and `preflight`: [`pkg/hooks/hooks.go`](https://github.com/cloudposse/atmos/blob/main/pkg/hooks/hooks.go).

Contributor guide

Open the contributing guide

Research direction

Start by reading cmd/terraform/generate/hooks.go:runGeneratePlanfileHooks and the existing scenario under tests/fixtures/scenarios/hooks-test/stacks/stack.yaml. Run the hooks integration scenario to follow its fixture and store assertions, then add a parallel generate-planfile setup covering both before and after events. Done means the integration test proves both hooks.RunAll branches execute without external binaries and coverage reaches about 90%.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
cli, testing-qa
Issue type
Refactor
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
72/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.