chore: Reuse a single playerbuilder across test cases#273
Open
Amund211 wants to merge 1 commit into
Open
Conversation
Builders now clone pointer fields and assign DB IDs at build time, so a single builder can be mutated between Build calls. Collapse verbose NewPlayerBuilder(...).With...().Build() repetitions in app, ports, and repository tests to share one builder per test, only setting fields that change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Refactors multiple Go test suites to reuse a single domaintest.NewPlayerBuilder(...) instance per test (or per logical group) instead of repeatedly constructing new builders for each PlayerPIT fixture, leveraging recent builder safety improvements (copy-on-build / pointer cloning, DBID assignment at build time).
Changes:
- Replaced repeated
NewPlayerBuilder(...).With...().Build(...)chains with shared builder variables reused across multipleBuild/BuildPtrcalls. - Centralized common gamemode setup (e.g.,
fours := builder.FromDB().Fours()) to reduce verbosity in session-related tests. - Updated repository milestone-achievement tests to build sequences of players/expectations from a shared builder.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| internal/ports/sessions_test.go | Reuses a single Fours() builder for session handler fixtures. |
| internal/app/sessions_test.go | Reuses a shared Fours() builder across many computeSessions test fixtures. |
| internal/app/playerpits_test.go | Reuses a shared builder to generate repeated PlayerPIT fixtures for get-player-pits tests. |
| internal/app/milestone_test.go | Reuses a shared builder for milestone player fixtures. |
| internal/app/history_test.go | Reuses a shared builder for history player fixtures. |
| internal/adapters/playerrepository/repository_test.go | Reuses shared builders for repository history/milestone fixture generation. |
Comment on lines
+72
to
76
| fours := domaintest.NewPlayerBuilder(uuid).FromDB().Fours() | ||
| stats := []domain.PlayerPIT{ | ||
| domaintest.NewPlayerBuilder(uuid). | ||
| WithExperience(500). | ||
| FromDB(). | ||
| Fours().WithGamesPlayed(10).WithFinalKills(10).Build(start), | ||
| domaintest.NewPlayerBuilder(uuid). | ||
| WithExperience(1000). | ||
| FromDB(). | ||
| Fours().WithGamesPlayed(11).WithFinalKills(11).Build(end), | ||
| fours.WithGamesPlayed(10).WithFinalKills(10).WithExperience(500).Build(start), | ||
| fours.WithGamesPlayed(11).WithFinalKills(11).WithExperience(1000).Build(end), | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
NewPlayerBuilder(...).With...().Build()repetitions in test cases that build a sequence of similar PlayerPITs.fours := builder.FromDB().Fours()) and reuses it betweenBuildcalls, only setting fields that actually change.queriedAttaken at build time) so a single builder is safe to reuse across builds.Touches:
internal/app/{sessions,milestone,playerpits,history}_test.go,internal/ports/sessions_test.go,internal/adapters/playerrepository/repository_test.go.Skipped
internal/ports/wrapped_internal_test.gobecause its builders live inside table-driven literals — pulling them out would hurt readability more than the duplication does.Test plan
go build ./...go vet ./...go test -short -count=1 ./...go test -count=1 ./internal/app/... ./internal/ports/...🤖 Generated with Claude Code