Skip to content

Commit 3b8639a

Browse files
authored
refactor(controller): add anti-fragile test patterns and clean up utilities (#121)
Refactor controller tests to use `controllers_test` package with shared envtest environment. This consolidates test environment setup in `suite_test.go` using `TestMain` and adds anti-fragile testing patterns including a DistributionBuilder, consolidated test constants, and behavioral helpers. This pull request also documents the intent behind these changes in `CONTRIBUTING.md`. This is intended to facilitate an upcoming pull request to add a `TestReconcile()` to the controller's test suite to more fully cover the controller's happy path to reconciliation. Approved-by: VaishnaviHire
1 parent 2b48cb8 commit 3b8639a

File tree

4 files changed

+543
-311
lines changed

4 files changed

+543
-311
lines changed

CONTRIBUTING.md

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,42 @@ For rapid development cycles, you can use focused test runs:
110110
make test TEST_PKGS=./pkg/deploy TEST_FLAGS="-v -run TestRenderManifest"
111111
```
112112

113+
## Anti-Fragile Testing Principles
114+
115+
### Test Structure
116+
- **DAMP and DRY together**: Apply DAMP (Descriptive And Meaningful Phrases) to test scenarios, and DRY to implementation details. Keep test intent explicit while extracting common setup logic.
117+
- **Use AAA pattern**: Arrange, Act, Assert with clear comments separating each phase.
118+
119+
### Test Data
120+
- **Integration tests**: Use production constants to verify the controller applies defaults correctly.
121+
- **E2E tests**: Use test-owned constants to focus on user workflows, not implementation details.
122+
- **Make test intent obvious**: Use descriptive names and explicit values that show what each test is verifying.
123+
124+
### Test Isolation
125+
- **No shared state**: Each test should be independent with unique namespaces and fresh instances.
126+
- **Use builders for variations**: Create test instances with clear intent using the builder pattern.
127+
- **Async operations**: Use `require.Eventually` with appropriate timeouts for Kubernetes operations.
128+
129+
### Example
130+
```go
131+
// Good: Descriptive test names that explain behavior
132+
{
133+
name: "No storage configuration - should use emptyDir",
134+
buildInstance: func(namespace string) *llamav1alpha1.LlamaStackDistribution {
135+
return NewDistributionBuilder().
136+
WithStorage(nil). // Clear intent: testing emptyDir behavior
137+
Build()
138+
},
139+
},
140+
141+
// Bad: Vague test name that doesn't explain expected behavior
142+
{
143+
name: "test nil storage",
144+
// ...
145+
}
146+
```
147+
148+
Focus: Tests should survive refactoring and clearly communicate what behavior they're verifying.
113149

114150
## Questions?
115151

0 commit comments

Comments
 (0)