Add unit tests for EnvironmentRepositoryImpl#133
Merged
Conversation
Cover all 16 public methods of EnvironmentRepositoryImpl with Mockito-based unit tests mirroring EntityRepositoryImplTest: find* (found/empty/SQLException), the id-list queries, save passthrough, and the boolean delete/update delegators (true/false). DbInteractions is mocked via @MockBean; no real database. Null-ResultSet cases are intentionally omitted because, unlike its sibling repositories, EnvironmentRepositoryImpl does not guard against a null ResultSet and currently throws NullPointerException for that input; tracked separately. Closes #129 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
dmccoystephenson
left a comment
Member
Author
There was a problem hiding this comment.
Self-review rubric (adversarial; anchored on green CI run 27466755010 + the diff):
- Scope: PASS —
gh pr diff --name-onlyshows exactly one new file undersrc/test/java/.../repositories/; no production, formatting, or unrelated changes. - Tests-new: PASS — all 16 public methods of
EnvironmentRepositoryImplare exercised (verified by grepping distinctrepository.<m>(call sites: findAll, findById, findByName, findByEntityId, save, deleteById, updateName, the three id-list finders, and the six delete delegators). - Tests-fix: N/A — test-only PR, no bug fix.
- Sibling structure: PASS — mirrors
GridRepositoryImplTest(section comments,@SpringBootTest+@MockBean DbInteractions, compact arrange/act/assert). - Sibling renames: N/A — no identifiers renamed.
- Docs: N/A — no API/DTO/endpoint surface changed; no Phase 7 doc source-of-truth row is affected.
- Issue resolution: PASS — #129's remaining surface (per its own comment, only
EnvironmentRepositoryImpl) is fully covered. - CI: PASS —
buildgreen on PR head. - DTO boundary / Spec alignment: N/A — no controller or endpoint change.
- Java/Python parallelism: PASS — repository layer is Java-only; the Python client has no repository mirror, so no parallel change is required.
- Override correctness: PASS — zero
@Overrideannotations in the test file.
Substantive finding (filed, not silenced): EnvironmentRepositoryImpl lacks the null-ResultSet guard its sibling repositories have and throws NullPointerException on a null ResultSet. Per test-cycle rules production code is not changed here; the tests characterize current behaviour and omit null cases. Filed as #132 for a dedicated fix cycle.
Summary: ready to merge as a characterization/regression baseline; the one real issue surfaced during review is tracked in #132.
This was referenced Jun 13, 2026
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
EnvironmentRepositoryImplTestcovering all 16 public methods ofEnvironmentRepositoryImpl, the last repository implementation without tests.EntityRepositoryImplTest/GridRepositoryImplTest(@SpringBootTest+@MockBean DbInteractions); no real database, no Testcontainers.findAll/findById/findByName/findByEntityId— row(s) present, empty result, andSQLException-handled-gracefully paths.findEntityIdsByEnvironmentId/findLocationIdsByEnvironmentId/findGridIdsByEnvironmentId— non-empty, empty, andSQLExceptionpaths.save— returns the same instance and touches no DB (current passthrough behaviour).deleteById,updateName, and the six delete delegators (deleteEntityLocation,deleteLocationGrid,deleteGridEnvironment,deleteEntity,deleteLocation,deleteGrid) — true/false delegation, asserting the exact SQL string passed toDbInteractions.update.Characterization note
Unlike its sibling repositories,
EnvironmentRepositoryImpldoes not guard against anullResultSetand currently throwsNullPointerExceptionfor that input. These tests characterize the current behaviour, so null-ResultSet cases are intentionally omitted rather than asserting an incorrect "returns empty". The underlying inconsistency is filed as #132 for a separate fix cycle (no production code is changed here).Scope note
The diff is a single new test file of ~561 LOC — above the soft ~400 LOC guideline but a single cohesive test class for one repository, with zero production changes and well under the hard ceiling. Splitting one class's tests across two PRs would add churn without benefit.
Test plan
./mvnw test -Dtest=EnvironmentRepositoryImplTest— 38/38 pass (Java 21)./mvnw test(full suite) — 206/206 pass, no regressionssrc/main/pythonchange)Closes #129
🤖 Generated with Claude Code