cloudposse / cloudposse/atmos

Refactor tests to use errors.Is() instead of string matching

Open
#1,616 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

## what

Replace brittle error string matching in tests with robust `errors.Is()` checks.

## why

- String matching is fragile and breaks when error messages change
- `errors.Is()` properly handles wrapped errors (the focus of PR #1599)
- Cross-platform OS errors exist in Go stdlib but we're not using them
- Aligns with the error handling patterns introduced in PR #1599

## details

**High Priority - Our Sentinel Errors (5 cases)**

1. `pkg/downloader/file_downloader_test.go` (4 occurrences)
- Replace `assert.Contains(err.Error(), "failed to download file")`
- With `assert.ErrorIs(err, errUtils.ErrDownloadFile)`

2. `pkg/config/imports_error_paths_test.go` (1 occurrence)
- Replace string check for "failed to download remote config"
- With `assert.ErrorIs(err, errUtils.ErrDownloadFile)`

3. `internal/exec/packer_test.go` (1 occurrence)
- Replace `assert.Contains(err.Error(), "executable file not found")`
- With `assert.ErrorIs(err, exec.ErrNotFound)` (cross-platform OS error)

**Medium Priority - Review Needed**

Files with many error string checks needing case-by-case review:
- `pkg/list/errors/types_test.go` (22 occurrences) - likely OK, testing error message formatting
- `internal/exec/copy_glob_error_paths_test.go` (15 occurrences)
- `internal/exec/stack_processor_process_stacks_helpers_test.go` (7 occurrences)
- `pkg/merge/merge_test.go` (6 occurrences)
- Various store test files

**Total**: 71 test files using error string matching

## guidelines

**Use `errors.Is()` for:**
- Our sentinel errors from `errors/errors.go`
- Standard library errors (`fs.ErrNotExist`, `exec.ErrNotFound`, `io.EOF`)
- Any wrapped error where you care about the type, not the message

**String matching is OK for:**
- Third-party library errors without sentinel errors
- Testing specific interpolated values in error messages
- Explicitly testing user-facing error message formatting

**Cross-platform OS errors:**
- `io/fs`: `fs.ErrNotExist`, `fs.ErrPermission`, `fs.ErrExist`, `fs.ErrInvalid`, `fs.ErrClosed`
- `os/exec`: `exec.ErrNotFound`

## references

- PR #1599 - Error handling implementation

Contributor guide

Open the contributing guide

Research direction

Start with pkg/downloader/file_downloader_test.go, pkg/config/imports_error_paths_test.go, and internal/exec/packer_test.go, then review the listed test files for additional error string checks. Use the sentinel and standard-library errors named in the issue where the error type matters, while retaining message checks for formatting or interpolated values. Done means the appropriate tests use errors.Is-compatible assertions and continue to pass.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
testing-qa
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.