Skip to content

test: add unit tests for AdminPolicy#166

Merged
lindaeskilsson merged 1 commit into
mainfrom
test/adminpolicy
Apr 8, 2026
Merged

test: add unit tests for AdminPolicy#166
lindaeskilsson merged 1 commit into
mainfrom
test/adminpolicy

Conversation

@lindaeskilsson

@lindaeskilsson lindaeskilsson commented Apr 8, 2026

Copy link
Copy Markdown
Contributor

Closes #165
Täcker requireAdmin för rollerna ADMIN, OWNER och VET samt null-användare.

Summary by CodeRabbit

  • Tests
    • Added test coverage for admin access control validation, ensuring proper authorization enforcement for administrator-level operations.

@coderabbitai

coderabbitai Bot commented Apr 8, 2026

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

A new JUnit 5 test class for AdminPolicy has been added, containing test cases that verify the requireAdmin(...) method correctly grants access to ADMIN users while denying it to users with other roles (OWNER, VET) and null values, all raising ForbiddenException with appropriate Swedish error messages.

Changes

Cohort / File(s) Summary
AdminPolicy Test Suite
src/test/java/org/example/vet1177/policy/AdminPolicyTest.java
New test class validating requireAdmin(...) authorization behavior across different user roles (ADMIN, OWNER, VET, and null), ensuring only administrators pass checks and others receive ForbiddenException.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Possibly related PRs

  • PR #125: Introduces AdminPolicy class and its requireAdmin method, which this test directly validates.

Suggested labels

test

Suggested reviewers

  • johanbriger
  • annikaholmqvist94
  • TatjanaTrajkovic

Poem

🐰 With whiskers twitching, fresh tests arise,
AdminPolicy's guard now verified,
Admins pass through, others denied,
Swedish words keep access dignified! 🔐

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning The PR covers requireAdmin behavior for ADMIN, OWNER, and VET roles with null-user edge cases, matching issue #165 requirements, but only tests one method instead of all methods in AdminPolicy. Add tests for all remaining methods in AdminPolicy (beyond requireAdmin) to fully satisfy the requirement to cover all methods in AdminPolicy as specified in issue #165.
Docstring Coverage ⚠️ Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: adding unit tests for AdminPolicy, which directly reflects the changeset content.
Out of Scope Changes check ✅ Passed All changes are focused on adding the AdminPolicyTest class with relevant test cases for AdminPolicy behavior, staying within the scope of the linked issue.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch test/adminpolicy

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
src/test/java/org/example/vet1177/policy/AdminPolicyTest.java (1)

9-10: Minor: Prefer consistent AssertJ import source.

Line 10 imports assertThatThrownBy from AssertionsForClassTypes while line 9 imports from Assertions. Both methods are available from Assertions, so using the same base class improves consistency.

♻️ Suggested fix
 import static org.assertj.core.api.Assertions.assertThatNoException;
-import static org.assertj.core.api.AssertionsForClassTypes.assertThatThrownBy;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
🤖 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/AdminPolicyTest.java` around lines 9
- 10, Replace the inconsistent AssertJ static import for assertThatThrownBy so
both assertions come from the same Assertions class: change the static import of
assertThatThrownBy (currently from org.assertj.core.api.AssertionsForClassTypes)
to use org.assertj.core.api.Assertions.assertThatThrownBy so it matches
assertThatNoException; no other code changes required.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/test/java/org/example/vet1177/policy/AdminPolicyTest.java`:
- Around line 9-10: Replace the inconsistent AssertJ static import for
assertThatThrownBy so both assertions come from the same Assertions class:
change the static import of assertThatThrownBy (currently from
org.assertj.core.api.AssertionsForClassTypes) to use
org.assertj.core.api.Assertions.assertThatThrownBy so it matches
assertThatNoException; no other code changes required.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: bcd17e7e-35dc-490a-8766-fb6595782f09

📥 Commits

Reviewing files that changed from the base of the PR and between a18d1f4 and 8e37849.

📒 Files selected for processing (1)
  • src/test/java/org/example/vet1177/policy/AdminPolicyTest.java

@TatjanaTrajkovic TatjanaTrajkovic left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bra jobbat!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test: AdminPolicyTest – behörighetsregler

2 participants