test: add unit tests for PetPolicy#163
Merged
Merged
Conversation
📝 WalkthroughWalkthroughAdds a new JUnit 5 test class Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/test/java/org/example/vet1177/policy/PetPolicyTest.java (1)
54-145: Consider parameterized tests to reduce duplication in role/ownership matrices.This suite is readable as-is, but
@ParameterizedTest+ role/ownership inputs would reduce repeated setup/assertion patterns and make future policy changes easier to update.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/org/example/vet1177/policy/PetPolicyTest.java` around lines 54 - 145, Replace the repetitive role/ownership test methods (e.g., canCreate_owner_shouldReturnTrue, canCreate_admin_shouldReturnTrue, canView_admin_shouldReturnTrue, canView_ownerOfPet_shouldReturnTrue, canView_ownerWithNullPetOwner_shouldReturnFalse, canViewOwnerPets_admin_shouldReturnTrue, canViewOwnerPets_ownerViewingOwnPets_shouldReturnTrue, canDelete_admin_shouldReturnTrue, canDelete_ownerWithNullPetOwner_shouldReturnFalse, etc.) with a few `@ParameterizedTest` methods that drive policy.canCreate(...), policy.canView(...), policy.canViewOwnerPets(...), and policy.canDelete(...) using `@CsvSource` or `@MethodSource`; provide parameters for principal (role identifier), target pet or ownerId (or null marker), and expected boolean result, map the role identifiers to the existing fixtures (admin, owner, vet, otherOwner) inside the provider/helper, and assert the expected boolean — this keeps existing fixtures and assertions but collapses duplicated tests into parameterized tables for easier maintenance.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/test/java/org/example/vet1177/policy/PetPolicyTest.java`:
- Around line 118-147: Add unit tests mirroring the canDelete cases to cover
canUpdate on the PetPolicy: create tests that assert policy.canUpdate(admin,
pet) is true, policy.canUpdate(owner, pet) is true, policy.canUpdate(otherOwner,
pet) is false, policy.canUpdate(vet, pet) is false, and a test where
pet.setOwner(null) then policy.canUpdate(owner, pet) is false; use the same test
structure/naming pattern as the existing canDelete tests and the same fixtures
(policy, admin, owner, otherOwner, vet, pet).
---
Nitpick comments:
In `@src/test/java/org/example/vet1177/policy/PetPolicyTest.java`:
- Around line 54-145: Replace the repetitive role/ownership test methods (e.g.,
canCreate_owner_shouldReturnTrue, canCreate_admin_shouldReturnTrue,
canView_admin_shouldReturnTrue, canView_ownerOfPet_shouldReturnTrue,
canView_ownerWithNullPetOwner_shouldReturnFalse,
canViewOwnerPets_admin_shouldReturnTrue,
canViewOwnerPets_ownerViewingOwnPets_shouldReturnTrue,
canDelete_admin_shouldReturnTrue,
canDelete_ownerWithNullPetOwner_shouldReturnFalse, etc.) with a few
`@ParameterizedTest` methods that drive policy.canCreate(...),
policy.canView(...), policy.canViewOwnerPets(...), and policy.canDelete(...)
using `@CsvSource` or `@MethodSource`; provide parameters for principal (role
identifier), target pet or ownerId (or null marker), and expected boolean
result, map the role identifiers to the existing fixtures (admin, owner, vet,
otherOwner) inside the provider/helper, and assert the expected boolean — this
keeps existing fixtures and assertions but collapses duplicated tests into
parameterized tables for easier maintenance.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b9fb2c3e-6cf7-47c8-9013-0f433711f970
📒 Files selected for processing (1)
src/test/java/org/example/vet1177/policy/PetPolicyTest.java
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/test/java/org/example/vet1177/policy/PetPolicyTest.java (1)
147-168:⚠️ Potential issue | 🟠 MajorAdd missing
canUpdateADMIN test to satisfy issue#143acceptance criteria.Line [148]-Line [168] covers OWNER/VET/null-owner cases, but
canUpdate(admin, pet)is still missing, so thecanUpdatematrix is incomplete.➕ Suggested addition
//canUpdate + `@Test` + void canUpdate_admin_shouldReturnTrue() { + assertThat(policy.canUpdate(admin, pet)).isTrue(); + } + `@Test` void canUpdate_ownerOfPet_shouldReturnTrue() { assertThat(policy.canUpdate(owner, pet)).isTrue(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/org/example/vet1177/policy/PetPolicyTest.java` around lines 147 - 168, The test matrix for PetPolicy.canUpdate is missing the ADMIN case; add a unit test in PetPolicyTest that asserts policy.canUpdate(admin, pet) is true. Create a new test method (e.g., canUpdate_admin_shouldReturnTrue) that uses the existing admin test fixture/variable and the pet instance (ensure pet.owner is set appropriately) and asserts isTrue() like the other canUpdate tests to complete the coverage for ADMIN.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/test/java/org/example/vet1177/policy/PetPolicyTest.java`:
- Around line 147-168: The test matrix for PetPolicy.canUpdate is missing the
ADMIN case; add a unit test in PetPolicyTest that asserts
policy.canUpdate(admin, pet) is true. Create a new test method (e.g.,
canUpdate_admin_shouldReturnTrue) that uses the existing admin test
fixture/variable and the pet instance (ensure pet.owner is set appropriately)
and asserts isTrue() like the other canUpdate tests to complete the coverage for
ADMIN.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e71f5512-1b4f-4d53-9fd4-3f763d0a7cda
📒 Files selected for processing (1)
src/test/java/org/example/vet1177/policy/PetPolicyTest.java
This was referenced Apr 8, 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
2 participants
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.
Closes #143
Täcker canCreate, canView, canViewOwnerPets, canUpdate och canDelete för rollerna ADMIN, OWNER och VET, inklusive edge cases som null-ägare och åtkomst till andras djur.
Summary by CodeRabbit