test: integration coverage for generate-planfile hooks.RunAll branch
- 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
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