test: add unit tests for domain and business logic #4

hermes · 2026-06-27T23:13:22Z

hermes commented

2026-06-27 23:13:22 +00:00

Adds focused unit tests covering domain types and business logic (14 tests, 3 files).

Coverage:

inMemoryMapper: CollectCounts, GetCounts (5 table cases), GetStats (internal map vs deep copy)
listCourseHandler: pagination loop (drain-full multi-batch + limited single call) + name resolution
listCoursesStatsHandler: by-org aggregation via repo + mapper delegation branch
getCourseHandler: name resolution + error wrapping
createCourseHandler / createCoursesHandler: param mapping + error wrapping

All tests pass with go test ./internal/kurious/... under Go 1.26. go vet clean.

Note: the generated mocks.CourseRepository is stale (missing ListStatistics). Tests work around this with a local wrapper; regenerating mocks would be a follow-up.

Adds focused unit tests covering domain types and business logic (14 tests, 3 files). **Coverage:** - `inMemoryMapper`: `CollectCounts`, `GetCounts` (5 table cases), `GetStats` (internal map vs deep copy) - `listCourseHandler`: pagination loop (drain-full multi-batch + limited single call) + name resolution - `listCoursesStatsHandler`: by-org aggregation via repo + mapper delegation branch - `getCourseHandler`: name resolution + error wrapping - `createCourseHandler` / `createCoursesHandler`: param mapping + error wrapping All tests pass with `go test ./internal/kurious/...` under Go 1.26. `go vet` clean. **Note:** the generated `mocks.CourseRepository` is stale (missing `ListStatistics`). Tests work around this with a local wrapper; regenerating mocks would be a follow-up.

hermes added 1 commit 2026-06-27 23:13:22 +00:00

test: add unit tests for domain and business logic 50ee55d83d

Cover the in-memory mapper (count collection, GetCounts branches,
GetStats deep-copy), listCourse pagination loop, listCoursesStats
aggregation (by-org + mapper delegation), getCourse name resolution
and error wrapping, and createCourse(s) param mapping.

14 tests across 3 files. go vet clean.

hermes commented

2026-06-27 23:21:14 +00:00

📋 PR #4 — Triage Review

Files: 3 new test files, 411 lines, 14 tests

✅ What's good

Well-structured — table-driven where appropriate, sentinel errors with ErrorIs + Contains for error wrapping verification
Good coverage of happy + error paths for each handler (create, get, list, stats)
stubMapper in query_test.go is a proper test double implementing the full CourseMapper interface
GetStats deep-copy test is thoughtful — verifies mutation isolation for both copyMap: true and false
Multi-batch pagination test (DrainFull) exercises the real drain loop (2400 items / 3 pages)

⚠️ Issues to flag

#	Severity	Issue
1	Low	`fullRepo` wrapper duplicated 3× — identical boilerplate in `adapters/`, `command/`, `query/` tests. Will be eliminated when mocks are regenerated.
2	Info	Typo codified: `OrganizaitonID` (missing 'n') and `CourseThematicsID` (extra 's') in domain types. Pre-existing, but tests now lock it in. Worth a separate cleanup issue.
3	Low	Direct private field access in `memory_mapper_test.go` — sets `m.stats`, `m.courseThematicByLearningType`, `m.totalCount` directly. Couples tests to internal struct layout.
4	Info	`Limit: 1001` in `CollectCounts` — looks like a potential off-by-one. Test documents existing behavior correctly, but worth investigating.

🔍 Missing coverage (non-blocking)

No nil/empty edge cases (e.g. empty ID, negative limit)
No concurrent access tests for inMemoryMapper (plain maps, no mutex)
No CreateBatch partial failure test

💬 Review focus

Regenerate mocks as follow-up to eliminate fullRepo workaround
File separate issue for OrganizaitonID typo rename
Investigate Limit: 1001 — intentional or bug?

Verdict: ✅ Approve — solid test coverage, correct patterns, no bugs introduced. All issues are pre-existing or follow-up items.

## 📋 PR #4 — Triage Review **Files:** 3 new test files, 411 lines, 14 tests ### ✅ What's good - **Well-structured** — table-driven where appropriate, sentinel errors with `ErrorIs` + `Contains` for error wrapping verification - **Good coverage of happy + error paths** for each handler (create, get, list, stats) - **`stubMapper`** in `query_test.go` is a proper test double implementing the full `CourseMapper` interface - **`GetStats` deep-copy test** is thoughtful — verifies mutation isolation for both `copyMap: true` and `false` - **Multi-batch pagination test** (`DrainFull`) exercises the real drain loop (2400 items / 3 pages) ### ⚠️ Issues to flag | # | Severity | Issue | |---|----------|-------| | 1 | **Low** | **`fullRepo` wrapper duplicated 3×** — identical boilerplate in `adapters/`, `command/`, `query/` tests. Will be eliminated when mocks are regenerated. | | 2 | **Info** | **Typo codified: `OrganizaitonID`** (missing 'n') and `CourseThematicsID` (extra 's') in domain types. Pre-existing, but tests now lock it in. Worth a separate cleanup issue. | | 3 | **Low** | **Direct private field access** in `memory_mapper_test.go` — sets `m.stats`, `m.courseThematicByLearningType`, `m.totalCount` directly. Couples tests to internal struct layout. | | 4 | **Info** | **`Limit: 1001` in `CollectCounts`** — looks like a potential off-by-one. Test documents existing behavior correctly, but worth investigating. | ### 🔍 Missing coverage (non-blocking) - No nil/empty edge cases (e.g. empty ID, negative limit) - No concurrent access tests for `inMemoryMapper` (plain maps, no mutex) - No `CreateBatch` partial failure test ### 💬 Review focus 1. **Regenerate mocks** as follow-up to eliminate `fullRepo` workaround 2. **File separate issue** for `OrganizaitonID` typo rename 3. **Investigate `Limit: 1001`** — intentional or bug? **Verdict:** ✅ **Approve** — solid test coverage, correct patterns, no bugs introduced. All issues are pre-existing or follow-up items.

hermes merged commit 84656c6c56 into master

2026-06-27 23:23:01 +00:00

hermes referenced this issue from a commit

2026-06-27 23:23:03 +00:00

test: add unit tests for domain and business logic (#4)

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: frx/kurious#4