Add repository unit tests for Grid and Location#131
Merged
Conversation
Mirror the existing EntityRepositoryImplTest pattern (@SpringBootTest + @MockBean DbInteractions + mocked ResultSet) to cover every public method of GridRepositoryImpl (12 tests) and LocationRepositoryImpl (17 tests): happy path, empty result set, null result set, SQLException handling, and update-delegation for the entity-location mutators. Characterization tests only — no production code changed. Partially addresses #129 (Grid + Location); EnvironmentRepositoryImpl deferred to a follow-up to respect the scope ceiling. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
dmccoystephenson
left a comment
Member
Author
There was a problem hiding this comment.
Self-review rubric (CI green on head — external anchor satisfied):
- Scope: PASS — diff is exactly 2 new files under
src/test/java/.../repositories/, 457 insertions, zero production or unrelated changes. - Tests-new: PASS — all 12 public methods across the two repos are exercised; 29 assertions-bearing tests, all green locally and in CI.
- Tests-fix: N/A — no bug fix in this PR (characterization tests only).
- Sibling structure: PASS — both classes mirror
EntityRepositoryImplTest(package, imports,@SpringBootTest+@MockBean DbInteractions, mockedResultSet,testMethod_Conditionnaming). - Sibling renames: N/A — no identifiers renamed.
- Docs: PASS — no API/behavior change, so no doc source-of-truth row is affected.
- Issue resolution: PASS — #129's Grid + Location surface is actually added; PR claims only partial progress, not closure.
- DTO boundary / Spec alignment / Java-Python parallelism / Override: N/A — no controller, endpoint, Python, or override changes.
One issue found (deliberate scoping, not a defect): the SQLException error path is covered for findAll on both repos and for GridRepositoryImpl.findById, but not exhaustively for every finder (e.g. findByEnvironmentId, findByEntityId, findByGridId). The catch blocks there are structurally identical boilerplate, so additional cases would be redundant LOC against an already-over-soft-ceiling diff. Flagging so a reviewer can disagree; happy to expand if exhaustive error-path coverage is preferred.
Summary: green, scope-clean, mirrors the real (Mockito, not Testcontainers) sibling pattern; ready pending review.
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
GridRepositoryImplTest(12 tests) andLocationRepositoryImplTest(17 tests), covering every public method of the two repositories.EntityRepositoryImplTestpattern:@SpringBootTest+@MockBean DbInteractions+ a mockedResultSet. No Testcontainers / no real database — so these run anywhere CI does.SQLExceptionhandling, andupdate(...)delegation for theentity_locationmutators (addEntityToLocation,removeEntityFromLocation,removeEntityFromCurrentLocation).Note on the tracking issue
#129 describes the existing test as a "Testcontainers/JDBC approach." That's inaccurate —
EntityRepositoryImplTestactually mocksDbInteractionswith Mockito and uses no container. These new tests follow the real sibling pattern. (The cascade-delete / one-entity-per-location scenarios #129 mentions live in the Environment repository, which is deferred below.)Scope
Partially addresses #129 — Grid + Location only.
EnvironmentRepositoryImpl(16 public methods, ~800 LOC of tests) is deferred to a follow-up to stay within the per-PR scope ceiling; #129 remains open to track it.No Python-client changes: repositories are Java-only JDBC and have no Python mirror.
Test plan
./mvnw test -B(JDK 21) — full suite green: 168 tests, 0 failures, 0 errorsPartially addresses #129