tests: agnostic tests#124
Merged
omatheusmesmo merged 1 commit intoApr 20, 2026
Merged
Conversation
Contributor
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 45 minutes and 31 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (27)
📝 WalkthroughWalkthroughReplaced hardcoded test fixtures with domain-specific test factories and centralized Faker (FakerUtil); added Datafaker to Maven test dependencies. Many tests updated to consume factory-generated entities/DTOs and dynamic IDs. Introduced a new JwtServiceTest covering token lifecycle scenarios. Updated docs to recommend factory usage. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (12)
backend/src/test/java/com/omatheusmesmo/shoppmate/shared/service/AuditServiceTest.java (1)
1-4: Remove redundant self-package import.After moving the test into
com.omatheusmesmo.shoppmate.shared.service, the explicit import ofAuditServiceon line 4 refers to a class in the same package and is now redundant.♻️ Proposed cleanup
package com.omatheusmesmo.shoppmate.shared.service; import com.omatheusmesmo.shoppmate.category.entity.Category; -import com.omatheusmesmo.shoppmate.shared.service.AuditService; import org.junit.jupiter.api.Test;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/test/java/com/omatheusmesmo/shoppmate/shared/service/AuditServiceTest.java` around lines 1 - 4, The import of AuditService is redundant because the test class resides in the same package; remove the explicit import statement referencing AuditService to clean up unused/self-package imports (look for the AuditService import declaration in AuditServiceTest and delete it).backend/src/test/java/com/omatheusmesmo/shoppmate/item/service/ItemServiceTest.java (1)
208-210: Optional: inlinecreateSampleItem().Now that the body is a single delegation to
ItemTestFactory.createValidItem(), the private helper adds a layer without value. Call sites could invoke the factory directly for clarity.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/test/java/com/omatheusmesmo/shoppmate/item/service/ItemServiceTest.java` around lines 208 - 210, Remove the trivial private helper createSampleItem() in ItemServiceTest and replace its call sites to use ItemTestFactory.createValidItem() directly; update any tests referencing createSampleItem() to call ItemTestFactory.createValidItem() and delete the createSampleItem() method to eliminate the unnecessary indirection.backend/src/test/java/com/omatheusmesmo/shoppmate/shared/testutils/UnitTestFactory.java (1)
9-18: Minor: ID generation can return 0 and collide across fixtures.
FakerUtil.getFaker().random().nextLong(1000)yields values in[0, 1000), so twocreateValidUnit()calls within the same test can produce the same id (birthday problem kicks in fast), and0can conflict with unset-ID conventions. For unit tests relying on Mockito stubs keyed byunit.getId()this is usually fine, but using a larger range (orfaker.number().positive()) would make collisions negligible.Also, since this is a pure test fixture, consider whether
createdAt/updatedAt/deletedneed to be set here at all — the service under test is responsible for audit fields, and pre-populating them can mask bugs inAuditService.setAuditData(...)paths.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/test/java/com/omatheusmesmo/shoppmate/shared/testutils/UnitTestFactory.java` around lines 9 - 18, The fixture createValidUnit() currently uses FakerUtil.getFaker().random().nextLong(1000) which can return 0 and causes id collisions; change the id generation in createValidUnit() to produce a non-zero, much larger range (e.g., use FakerUtil.getFaker().number().numberBetween(1, 1_000_000) or equivalent) so ids are positive and collisions are negligible, and optionally remove/stop setting audit fields (createdAt, updatedAt, deleted) inside createValidUnit() so tests exercise the AuditService/AuditService.setAuditData(...) paths instead of masking them.backend/src/test/java/com/omatheusmesmo/shoppmate/unit/service/UnitServiceTest.java (1)
3-22: Run formatter/organize imports on the touched Java tests.This import block still has project imports before
java.*/third-party imports and uses a wildcard static import. The same cleanup likely applies across the moved/refactored test files, so running the formatter once should handle it. As per coding guidelines, use specific imports instead of wildcards and order imports asjava.*,jakarta.*,org.springframework.*, third-party, then project packages.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/test/java/com/omatheusmesmo/shoppmate/unit/service/UnitServiceTest.java` around lines 3 - 22, The imports in UnitServiceTest are misordered and include a wildcard static import; run the formatter or manually reorder imports so java.* come first, then jakarta.*, org.springframework.*, third-party, and finally project packages, and replace the wildcard static import (e.g., "import static org.junit.jupiter.api.Assertions.*" / "import static org.mockito.Mockito.*") with the specific static imports actually used in UnitServiceTest (like assertEquals/assertNotNull or the exact Mockito methods used such as when, verify, times, any) to satisfy the project's import ordering and explicit-import guideline; locate the import block at the top of the UnitServiceTest class to make these changes.backend/src/test/java/com/omatheusmesmo/shoppmate/item/controller/ItemControllerWithIntegrationTest.java (1)
191-197: Return the saved item entity fromcreateItemToTest()to maintain consistency with sibling helpers.The
createCategoryToTest()andcreateUnitToTest()methods correctly assign thesave()return value, butcreateItemToTest()discards it. Sincesave()may return a modified instance with persistence-generated fields (e.g., the ID set by the database), use the returned entity rather than the pre-save variable:Proposed fix
Item createItemToTest() { Item itemEntity = ItemTestFactory.createValidItem(); itemEntity.setId(null); // Let DB generate ID itemEntity.setCategory(createCategoryToTest()); itemEntity.setUnit(createUnitToTest()); - itemRepository.save(itemEntity); - return itemEntity; + return itemRepository.save(itemEntity); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/test/java/com/omatheusmesmo/shoppmate/item/controller/ItemControllerWithIntegrationTest.java` around lines 191 - 197, The createItemToTest() helper currently calls itemRepository.save(itemEntity) but returns the pre-save itemEntity, losing persistence-generated fields; update createItemToTest() to capture the result of itemRepository.save(...) (e.g., savedItem = itemRepository.save(itemEntity)) and return that saved instance so the method returns the persisted entity with DB-generated fields like id (refer to createItemToTest(), itemRepository.save(...), and how createCategoryToTest()/createUnitToTest() already assign and return their save() results).backend/src/test/java/com/omatheusmesmo/shoppmate/category/controller/CategoryControllerTest.java (1)
62-70: Consider regenerating until unique instead of mutating the second category's ID.
CategoryTestFactory.createValidCategory()usesFakerUtil.getFaker().random().nextLong(1000), so collisions are possible (birthday-paradox ~1/1000). The current fix (category2.setId(category1.getId() + 1)) works but (a) silently mutates a factory-produced fixture and (b) could theoretically still collide if that generator ever producescategory1.getId() + 1for another entity in the same test. A small loop would be more robust:♻️ Proposed refactor
- category1 = CategoryTestFactory.createValidCategory(); - category2 = CategoryTestFactory.createValidCategory(); - // Ensure unique IDs for test sorting logic if any - if (category1.getId().equals(category2.getId())) { - category2.setId(category1.getId() + 1); - } + category1 = CategoryTestFactory.createValidCategory(); + do { + category2 = CategoryTestFactory.createValidCategory(); + } while (category1.getId().equals(category2.getId()));A cleaner long-term fix would be for
CategoryTestFactoryto draw from a wider ID range (e.g.,nextLong(Long.MAX_VALUE)or anAtomicLongsequence) so consumers don't need uniqueness workarounds.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/test/java/com/omatheusmesmo/shoppmate/category/controller/CategoryControllerTest.java` around lines 62 - 70, In CategoryControllerTest replace the ad-hoc mutation of category2's id with a regenerate loop: call CategoryTestFactory.createValidCategory() repeatedly for category2 until category2.getId() is not equal to category1.getId() (i.e., while (category2.getId().equals(category1.getId())) category2 = CategoryTestFactory.createValidCategory()); then build category1ResponseDTO and category2ResponseDTO as before; this avoids mutating factory fixtures and ensures unique ids without relying on +1.backend/src/test/java/com/omatheusmesmo/shoppmate/list/service/ListItemServiceTest.java (2)
48-55: Prefer@ExtendWith(MockitoExtension.class)over manualopenMocks.The rest of the refactored suite (e.g.,
CustomUserDetailsServiceTest,ListPermissionServiceTest) moved to@ExtendWith(MockitoExtension.class). This class still usesMockitoAnnotations.openMocks(this)without capturing/closing the returnedAutoCloseable, which leaks mock state between tests and is inconsistent with the new pattern. Also — strict stubbing from the extension would have flagged any unused stubs introduced by the refactor.♻️ Suggested change
+import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.junit.jupiter.MockitoExtension; @@ +@ExtendWith(MockitoExtension.class) class ListItemServiceTest { @@ `@BeforeEach` void setUp() { - MockitoAnnotations.openMocks(this); shoppingList = ListTestFactory.createValidShoppingList();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/test/java/com/omatheusmesmo/shoppmate/list/service/ListItemServiceTest.java` around lines 48 - 55, Replace the manual mock initialization in ListItemServiceTest by annotating the test class with `@ExtendWith`(MockitoExtension.class) and remove the MockitoAnnotations.openMocks(this) call in setUp(); specifically, delete the call to MockitoAnnotations.openMocks(this) in the setUp() method and add the MockitoExtension to the test class declaration so that mocks are managed and closed automatically (this will apply to the setUp(), ListItemServiceTest class and any fields annotated with `@Mock/`@InjectMocks).
111-117:nonExistingIdconstruction assumes the factory's ID range.
listItem.getId() + 1000only guarantees "non-existing" becauseListTestFactorycurrently caps IDs at1000. If that range is ever widened (recommended elsewhere), this +1000 offset could coincide with a real ID. A safer pattern is to mock the repository to returnOptional.empty()for any non-matching id, or use a value guaranteed to differ (e.g.,-1L). Same applies to lines 135 and to the analogous spots inListPermissionServiceTest.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/test/java/com/omatheusmesmo/shoppmate/list/service/ListItemServiceTest.java` around lines 111 - 117, The test builds nonExistingId as listItem.getId() + 1000 which assumes ListTestFactory ID bounds; change tests to use a guaranteed-different value (e.g., -1L) or configure the mock to return Optional.empty() for any id using when(listItemRepository.findByIdAndDeletedFalse(anyLong())).thenReturn(Optional.empty()) so the assertion on service.findListItemById still triggers NoSuchElementException; update the occurrences referring to nonExistingId (the variable and its use with listItem.getId()), replicate the same change at the other occurrence around line 135 and the analogous spots in ListPermissionServiceTest to avoid relying on factory ID ranges.backend/src/test/java/com/omatheusmesmo/shoppmate/shared/testutils/UserTestFactory.java (1)
10-21: Narrow random ID range (0–999) is collision-prone across factories.All
*TestFactoryclasses useFakerUtil.getFaker().random().nextLong(1000), producing IDs in[0, 1000). When a test creates several users (or users + lists + items, all with overlapping ranges), collisions are likely enough to require ad-hoc workarounds (see e.g.CategoryControllerTest.setUpadjustingcategory2ID). Consider widening the range or using an incrementing sequence so consumers don't have to dedupe manually.♻️ Suggested change
- user.setId(FakerUtil.getFaker().random().nextLong(1000)); + user.setId(FakerUtil.getFaker().random().nextLong(1L, Long.MAX_VALUE));Also note
nextLong(1000)can return0, which may not be a valid primary-key value in some downstream assumptions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/test/java/com/omatheusmesmo/shoppmate/shared/testutils/UserTestFactory.java` around lines 10 - 21, createValidUser in UserTestFactory currently uses FakerUtil.getFaker().random().nextLong(1000) which yields collisions and can return 0; replace that call with a stable, non-colliding generator (either widen to a very large range and exclude 0 or—preferably—use a shared incrementing sequence) so IDs are unique across test factories; implement a shared AtomicLong counter (start at 1) accessible to UserTestFactory and other *TestFactory classes and use counter.incrementAndGet() for the id instead of FakerUtil.getFaker().random().nextLong(1000) to avoid collisions and zero IDs.backend/src/test/java/com/omatheusmesmo/shoppmate/user/service/UserServiceTest.java (1)
7-7: Redundant self-package import.
UserServicelives in the same package as this test (com.omatheusmesmo.shoppmate.user.service), so this import is unnecessary and some IDE/CI lint checks will flag it.-import com.omatheusmesmo.shoppmate.user.service.UserService;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/test/java/com/omatheusmesmo/shoppmate/user/service/UserServiceTest.java` at line 7, Remove the redundant import of UserService from the test file: since UserService is declared in the same package com.omatheusmesmo.shoppmate.user.service as UserServiceTest, delete the line importing com.omatheusmesmo.shoppmate.user.service.UserService and leave the rest of the file unchanged so the class is referenced directly without an unnecessary self-package import.backend/src/test/java/com/omatheusmesmo/shoppmate/shared/testutils/ListTestFactory.java (2)
18-18: Mark utility factory as non-instantiable.
ListTestFactoryexposes onlystaticmethods but has an implicit public constructor, which invites accidental instantiation and is inconsistent with the "utility holder" intent. Consider making the classfinalwith a private constructor (same pattern as the other new*TestFactoryclasses should follow):-public class ListTestFactory { +public final class ListTestFactory { + + private ListTestFactory() {}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/test/java/com/omatheusmesmo/shoppmate/shared/testutils/ListTestFactory.java` at line 18, ListTestFactory is a utility holder with only static methods but has a public implicit constructor; make it non-instantiable by declaring the class final and adding a private constructor (e.g., private ListTestFactory() { throw new AssertionError("no instances"); }) to match the other *TestFactory classes and prevent accidental instantiation; update the class declaration and add the private constructor inside ListTestFactory.
39-39: AvoidBigDecimal.valueOf(double)for monetary values.Converting a
randomDoublethroughBigDecimal.valueOf(double)inherits binary floating-point artifacts (e.g., unexpected scale or trailing digits), which can make equality assertions onunitPrice/price DTOs flaky depending on the producer's rounding. SincerandomDouble(2, 1, 100)already returns a string-backed value, prefer constructing theBigDecimalfrom a string or using faker'sNumber#randomNumber+ explicit scale:- listItem.setUnitPrice(BigDecimal.valueOf(FakerUtil.getFaker().number().randomDouble(2, 1, 100))); + listItem.setUnitPrice(new BigDecimal( + String.valueOf(FakerUtil.getFaker().number().randomDouble(2, 1, 100))) + .setScale(2, java.math.RoundingMode.HALF_UP));Also applies to: 70-70, 80-80
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/test/java/com/omatheusmesmo/shoppmate/shared/testutils/ListTestFactory.java` at line 39, Replace the use of BigDecimal.valueOf(double) in ListTestFactory when setting unitPrice to avoid floating-point artifacts: instead construct the BigDecimal from a string or from an integer+scale so the value is exact (e.g., use the string form of Faker's generated number or build from randomNumber with an explicit scale) for the call to listItem.setUnitPrice(...) and apply the same change to the other occurrences flagged (lines around 70 and 80); locate the Faker call FakerUtil.getFaker().number().randomDouble(...) and replace its double-based conversion with a string-based BigDecimal constructor or BigDecimal.valueOf(long, scale) built from an integer representation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@backend/src/test/java/com/omatheusmesmo/shoppmate/list/service/ShoppingListServiceTest.java`:
- Around line 119-132: Test mutates the same testList instance so it masks
failures in ShoppingListService.editList; update the test to use two distinct
ShoppingList objects (e.g., originalList returned by
shoppingListRepository.findByIdAndDeletedFalse and updatedList passed into
shoppingListService.editList) so the repository mock returns originalList while
the service receives updatedList, then assert the returned result has the
updated fields, verify auditService.setAuditData(originalList, false) or as
appropriate and that shoppingListRepository.save(...) was called with the
merged/updated entity; reference methods/classes: editList,
ShoppingListServiceTest, shoppingListRepository.findByIdAndDeletedFalse,
auditService.setAuditData, shoppingListRepository.save to locate and replace the
single-instance setup with separate existing and update instances.
In
`@backend/src/test/java/com/omatheusmesmo/shoppmate/shared/testutils/CategoryTestFactory.java`:
- Around line 10-18: The factory uses bounded random IDs (nextLong(1000))
causing collisions; replace that with a per-factory monotonic AtomicLong counter
and use its incrementAndGet() to assign IDs in createValidCategory() in
CategoryTestFactory (and mirror the same change in ItemTestFactory,
UserTestFactory, UnitTestFactory and all ListTestFactory methods) so each
factory has a private static AtomicLong and assigns IDs deterministically via
atomic.incrementAndGet() instead of
FakerUtil.getFaker().random().nextLong(1000).
In
`@backend/src/test/java/com/omatheusmesmo/shoppmate/shared/testutils/FakerUtil.java`:
- Line 12: The FAKER constant in FakerUtil is constructed with new
Locale("en-US") which incorrectly treats "en-US" as a language; update the
construction to use a proper BCP-47 locale—replace new Locale("en-US") with
Locale.forLanguageTag("en-US") (or alternatively new Locale("en", "US")) so the
Faker is initialized with the correct locale; update the FAKER declaration
accordingly in the FakerUtil class.
In
`@backend/src/test/java/com/omatheusmesmo/shoppmate/shared/testutils/ItemTestFactory.java`:
- Around line 10-20: The ItemTestFactory.createValidItem currently uses
FakerUtil.getFaker().random().nextLong(1000) which yields bounded IDs and causes
collisions; change the factory to use a class-level AtomicLong sequence (e.g., a
private static AtomicLong idSequence initialized once) and assign ids via
idSequence.getAndIncrement() when building items so each call to createValidItem
produces a unique synthetic id without callers needing collision handling.
In
`@backend/src/test/java/com/omatheusmesmo/shoppmate/shared/testutils/ListTestFactory.java`:
- Line 22: The test factory sets fixture IDs using
FakerUtil.getFaker().random().nextLong(1000) which can produce 0 and causes
collisions across ShoppingList, ListItem, and ListPermission; update
ListTestFactory to produce strictly positive, non-colliding IDs (e.g., use
FakerUtil.getFaker().random().nextLong(minInclusive, maxExclusive) with min=1
and a much larger max, or replace with a per-factory AtomicLong counter that
increments for each created entity), and apply the same change to the other ID
assignments referenced in the file (the occurrences at the other mentioned
locations) so fixtures are unique and never zero.
In
`@backend/src/test/java/com/omatheusmesmo/shoppmate/user/service/UserServiceTest.java`:
- Around line 166-178: The tests named
editUser_EmailNull_ThrowsIllegalArgumentException and
editUser_PasswordNull_ThrowsIllegalArgumentException currently call
assertValidationException(...) which exercises
userService.validateIfDataIsNullOrEmpty(user) and therefore never uses the
when(userRepository.findById(...)) stub; update each test to actually call
userService.editUser(userMock) in the act step (so the findById stub is consumed
and the test name matches the behavior) and keep the existing assertion for the
thrown IllegalArgumentException, or alternatively remove the unused when(...)
stubs and rename the tests to
validateIfDataIsNullOrEmpty_EmailNull_ThrowsIllegalArgumentException /
validateIfDataIsNullOrEmpty_PasswordNull_ThrowsIllegalArgumentException if you
intend to test validation directly.
---
Nitpick comments:
In
`@backend/src/test/java/com/omatheusmesmo/shoppmate/category/controller/CategoryControllerTest.java`:
- Around line 62-70: In CategoryControllerTest replace the ad-hoc mutation of
category2's id with a regenerate loop: call
CategoryTestFactory.createValidCategory() repeatedly for category2 until
category2.getId() is not equal to category1.getId() (i.e., while
(category2.getId().equals(category1.getId())) category2 =
CategoryTestFactory.createValidCategory()); then build category1ResponseDTO and
category2ResponseDTO as before; this avoids mutating factory fixtures and
ensures unique ids without relying on +1.
In
`@backend/src/test/java/com/omatheusmesmo/shoppmate/item/controller/ItemControllerWithIntegrationTest.java`:
- Around line 191-197: The createItemToTest() helper currently calls
itemRepository.save(itemEntity) but returns the pre-save itemEntity, losing
persistence-generated fields; update createItemToTest() to capture the result of
itemRepository.save(...) (e.g., savedItem = itemRepository.save(itemEntity)) and
return that saved instance so the method returns the persisted entity with
DB-generated fields like id (refer to createItemToTest(),
itemRepository.save(...), and how createCategoryToTest()/createUnitToTest()
already assign and return their save() results).
In
`@backend/src/test/java/com/omatheusmesmo/shoppmate/item/service/ItemServiceTest.java`:
- Around line 208-210: Remove the trivial private helper createSampleItem() in
ItemServiceTest and replace its call sites to use
ItemTestFactory.createValidItem() directly; update any tests referencing
createSampleItem() to call ItemTestFactory.createValidItem() and delete the
createSampleItem() method to eliminate the unnecessary indirection.
In
`@backend/src/test/java/com/omatheusmesmo/shoppmate/list/service/ListItemServiceTest.java`:
- Around line 48-55: Replace the manual mock initialization in
ListItemServiceTest by annotating the test class with
`@ExtendWith`(MockitoExtension.class) and remove the
MockitoAnnotations.openMocks(this) call in setUp(); specifically, delete the
call to MockitoAnnotations.openMocks(this) in the setUp() method and add the
MockitoExtension to the test class declaration so that mocks are managed and
closed automatically (this will apply to the setUp(), ListItemServiceTest class
and any fields annotated with `@Mock/`@InjectMocks).
- Around line 111-117: The test builds nonExistingId as listItem.getId() + 1000
which assumes ListTestFactory ID bounds; change tests to use a
guaranteed-different value (e.g., -1L) or configure the mock to return
Optional.empty() for any id using
when(listItemRepository.findByIdAndDeletedFalse(anyLong())).thenReturn(Optional.empty())
so the assertion on service.findListItemById still triggers
NoSuchElementException; update the occurrences referring to nonExistingId (the
variable and its use with listItem.getId()), replicate the same change at the
other occurrence around line 135 and the analogous spots in
ListPermissionServiceTest to avoid relying on factory ID ranges.
In
`@backend/src/test/java/com/omatheusmesmo/shoppmate/shared/service/AuditServiceTest.java`:
- Around line 1-4: The import of AuditService is redundant because the test
class resides in the same package; remove the explicit import statement
referencing AuditService to clean up unused/self-package imports (look for the
AuditService import declaration in AuditServiceTest and delete it).
In
`@backend/src/test/java/com/omatheusmesmo/shoppmate/shared/testutils/ListTestFactory.java`:
- Line 18: ListTestFactory is a utility holder with only static methods but has
a public implicit constructor; make it non-instantiable by declaring the class
final and adding a private constructor (e.g., private ListTestFactory() { throw
new AssertionError("no instances"); }) to match the other *TestFactory classes
and prevent accidental instantiation; update the class declaration and add the
private constructor inside ListTestFactory.
- Line 39: Replace the use of BigDecimal.valueOf(double) in ListTestFactory when
setting unitPrice to avoid floating-point artifacts: instead construct the
BigDecimal from a string or from an integer+scale so the value is exact (e.g.,
use the string form of Faker's generated number or build from randomNumber with
an explicit scale) for the call to listItem.setUnitPrice(...) and apply the same
change to the other occurrences flagged (lines around 70 and 80); locate the
Faker call FakerUtil.getFaker().number().randomDouble(...) and replace its
double-based conversion with a string-based BigDecimal constructor or
BigDecimal.valueOf(long, scale) built from an integer representation.
In
`@backend/src/test/java/com/omatheusmesmo/shoppmate/shared/testutils/UnitTestFactory.java`:
- Around line 9-18: The fixture createValidUnit() currently uses
FakerUtil.getFaker().random().nextLong(1000) which can return 0 and causes id
collisions; change the id generation in createValidUnit() to produce a non-zero,
much larger range (e.g., use FakerUtil.getFaker().number().numberBetween(1,
1_000_000) or equivalent) so ids are positive and collisions are negligible, and
optionally remove/stop setting audit fields (createdAt, updatedAt, deleted)
inside createValidUnit() so tests exercise the
AuditService/AuditService.setAuditData(...) paths instead of masking them.
In
`@backend/src/test/java/com/omatheusmesmo/shoppmate/shared/testutils/UserTestFactory.java`:
- Around line 10-21: createValidUser in UserTestFactory currently uses
FakerUtil.getFaker().random().nextLong(1000) which yields collisions and can
return 0; replace that call with a stable, non-colliding generator (either widen
to a very large range and exclude 0 or—preferably—use a shared incrementing
sequence) so IDs are unique across test factories; implement a shared AtomicLong
counter (start at 1) accessible to UserTestFactory and other *TestFactory
classes and use counter.incrementAndGet() for the id instead of
FakerUtil.getFaker().random().nextLong(1000) to avoid collisions and zero IDs.
In
`@backend/src/test/java/com/omatheusmesmo/shoppmate/unit/service/UnitServiceTest.java`:
- Around line 3-22: The imports in UnitServiceTest are misordered and include a
wildcard static import; run the formatter or manually reorder imports so java.*
come first, then jakarta.*, org.springframework.*, third-party, and finally
project packages, and replace the wildcard static import (e.g., "import static
org.junit.jupiter.api.Assertions.*" / "import static org.mockito.Mockito.*")
with the specific static imports actually used in UnitServiceTest (like
assertEquals/assertNotNull or the exact Mockito methods used such as when,
verify, times, any) to satisfy the project's import ordering and explicit-import
guideline; locate the import block at the top of the UnitServiceTest class to
make these changes.
In
`@backend/src/test/java/com/omatheusmesmo/shoppmate/user/service/UserServiceTest.java`:
- Line 7: Remove the redundant import of UserService from the test file: since
UserService is declared in the same package
com.omatheusmesmo.shoppmate.user.service as UserServiceTest, delete the line
importing com.omatheusmesmo.shoppmate.user.service.UserService and leave the
rest of the file unchanged so the class is referenced directly without an
unnecessary self-package import.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4a3f480d-a5ae-45ba-ae8d-342d87e43df1
📒 Files selected for processing (27)
backend/AGENTS.mdbackend/pom.xmlbackend/src/test/java/com/omatheusmesmo/shoppmate/auth/controller/AuthControllerTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/auth/service/CustomUserDetailsServiceTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/auth/service/JwtServiceTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/category/controller/CategoryControllerTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/category/controller/CategoryControllerWithIntegrationTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/category/service/CategoryServiceTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/item/controller/ItemControllerTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/item/controller/ItemControllerWithIntegrationTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/item/service/ItemServiceTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/list/controller/ListItemControllerTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/list/controller/ListPermissionControllerTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/list/controller/ShoppingListControllerTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/list/service/ListItemServiceTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/list/service/ListPermissionServiceTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/list/service/ShoppingListServiceTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/shared/service/AuditServiceTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/shared/testutils/CategoryTestFactory.javabackend/src/test/java/com/omatheusmesmo/shoppmate/shared/testutils/FakerUtil.javabackend/src/test/java/com/omatheusmesmo/shoppmate/shared/testutils/ItemTestFactory.javabackend/src/test/java/com/omatheusmesmo/shoppmate/shared/testutils/ListTestFactory.javabackend/src/test/java/com/omatheusmesmo/shoppmate/shared/testutils/UnitTestFactory.javabackend/src/test/java/com/omatheusmesmo/shoppmate/shared/testutils/UserTestFactory.javabackend/src/test/java/com/omatheusmesmo/shoppmate/unit/service/UnitServiceTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/user/controller/UserControllerTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/user/service/UserServiceTest.java
Hlib-Rachkovskyy
changed the title
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
backend/src/test/java/com/omatheusmesmo/shoppmate/list/service/ListItemServiceTest.java (1)
3-25:⚠️ Potential issue | 🟡 MinorClean up the import block and replace wildcard static imports.
This file now adds another project import, but the import block still has
java.*split across groups and static wildcard imports. Please align it with the repository import convention.♻️ Proposed import cleanup
-import com.omatheusmesmo.shoppmate.item.entity.Item; -import java.math.BigDecimal; -import com.omatheusmesmo.shoppmate.item.service.ItemService; -import com.omatheusmesmo.shoppmate.list.dtos.ListItemRequestDTO; -import com.omatheusmesmo.shoppmate.list.dtos.ListItemUpdateRequestDTO; -import com.omatheusmesmo.shoppmate.list.entity.ListItem; -import com.omatheusmesmo.shoppmate.list.entity.ShoppingList; -import com.omatheusmesmo.shoppmate.list.mapper.ListItemMapper; -import com.omatheusmesmo.shoppmate.list.repository.ListItemRepository; -import com.omatheusmesmo.shoppmate.shared.service.AuditService; -import com.omatheusmesmo.shoppmate.shared.testutils.ListTestFactory; +import java.util.List; +import java.util.NoSuchElementException; +import java.util.Optional; + import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.MockitoAnnotations; -import java.util.List; -import java.util.NoSuchElementException; -import java.util.Optional; +import com.omatheusmesmo.shoppmate.item.entity.Item; +import com.omatheusmesmo.shoppmate.item.service.ItemService; +import com.omatheusmesmo.shoppmate.list.dtos.ListItemRequestDTO; +import com.omatheusmesmo.shoppmate.list.dtos.ListItemUpdateRequestDTO; +import com.omatheusmesmo.shoppmate.list.entity.ListItem; +import com.omatheusmesmo.shoppmate.list.entity.ShoppingList; +import com.omatheusmesmo.shoppmate.list.mapper.ListItemMapper; +import com.omatheusmesmo.shoppmate.list.repository.ListItemRepository; +import com.omatheusmesmo.shoppmate.shared.service.AuditService; +import com.omatheusmesmo.shoppmate.shared.testutils.ListTestFactory; -import static org.junit.jupiter.api.Assertions.*; -import static org.mockito.Mockito.*; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.Mockito.any; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when;As per coding guidelines,
backend/**/*.java: “Order imports as: java., jakarta., org.springframework.*, third-party, project packages. Use specific imports, no wildcards.”🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/test/java/com/omatheusmesmo/shoppmate/list/service/ListItemServiceTest.java` around lines 3 - 25, Reorder and clean the import block in ListItemServiceTest: group imports as java.*, jakarta.* (if any), org.springframework.* (if any), third-party, then project packages, and consolidate java.* into a single block; also remove wildcard static imports (currently static org.junit.jupiter.api.Assertions.* and static org.mockito.Mockito.*) and replace them with explicit static imports for only the methods used in this test (for example assertEquals, assertNotNull, assertThrows, when, verify, etc.), so update the import list around the classes referenced (ListItemServiceTest, ListItemService methods under test, ListTestFactory, ListItemMapper, ListItemRepository, ItemService, AuditService) accordingly.backend/src/test/java/com/omatheusmesmo/shoppmate/list/controller/ListItemControllerTest.java (1)
3-24:⚠️ Potential issue | 🟡 MinorReorder imports to match the repository convention.
The new project imports keep this file in project-first order, but the guideline requires
java.*, thenjakarta.*,org.springframework.*, third-party, then project imports.♻️ Proposed import ordering
-import com.fasterxml.jackson.databind.ObjectMapper; -import com.omatheusmesmo.shoppmate.auth.service.JwtService; -import com.omatheusmesmo.shoppmate.list.dtos.ListItemRequestDTO; -import com.omatheusmesmo.shoppmate.list.dtos.ListItemResponseDTO; -import com.omatheusmesmo.shoppmate.list.dtos.ListItemSummaryDTO; -import com.omatheusmesmo.shoppmate.list.entity.ListItem; -import com.omatheusmesmo.shoppmate.list.entity.ShoppingList; -import com.omatheusmesmo.shoppmate.list.mapper.ListItemMapper; -import com.omatheusmesmo.shoppmate.list.service.ListItemService; -import com.omatheusmesmo.shoppmate.shared.testutils.ListTestFactory; +import java.util.List; + import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc; import org.springframework.boot.test.autoconfigure.web.servlet.WebMvcTest; @@ import org.springframework.security.test.context.support.WithMockUser; import org.springframework.test.web.servlet.MockMvc; -import java.util.List; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.omatheusmesmo.shoppmate.auth.service.JwtService; +import com.omatheusmesmo.shoppmate.list.dtos.ListItemRequestDTO; +import com.omatheusmesmo.shoppmate.list.dtos.ListItemResponseDTO; +import com.omatheusmesmo.shoppmate.list.dtos.ListItemSummaryDTO; +import com.omatheusmesmo.shoppmate.list.entity.ListItem; +import com.omatheusmesmo.shoppmate.list.entity.ShoppingList; +import com.omatheusmesmo.shoppmate.list.mapper.ListItemMapper; +import com.omatheusmesmo.shoppmate.list.service.ListItemService; +import com.omatheusmesmo.shoppmate.shared.testutils.ListTestFactory;As per coding guidelines,
backend/**/*.java: “Order imports as: java., jakarta., org.springframework.*, third-party, project packages. Use specific imports, no wildcards.”🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/test/java/com/omatheusmesmo/shoppmate/list/controller/ListItemControllerTest.java` around lines 3 - 24, Reorder the imports in ListItemControllerTest so they follow the repo convention: group and sort imports as java.* first (e.g., java.util.List), then jakarta.* (if any), then org.springframework.* (MockMvc, annotations), then third-party libraries (Jackson, JUnit, Mockito, etc.), and finally project packages (com.omatheusmesmo.*); ensure no wildcard imports and alphabetize within each group; leave the class name ListItemControllerTest and all referenced symbols (ObjectMapper, MockMvc, WithMockUser, ListItemService, ListTestFactory, etc.) unchanged while only adjusting import ordering and grouping.
🧹 Nitpick comments (8)
backend/src/test/java/com/omatheusmesmo/shoppmate/list/service/ListPermissionServiceTest.java (2)
135-147: Drop the redundantassertDoesNotThrowwrapper.Any thrown exception in the Act step already fails the test; wrapping the call adds noise without assertion value. A direct call keeps the Arrange/Act/Assert flow consistent with the other tests in this class.
♻️ Proposed change
- // Act - assertDoesNotThrow(() -> listPermissionService.removeList(listPermission.getId())); + // Act + listPermissionService.removeList(listPermission.getId());🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/test/java/com/omatheusmesmo/shoppmate/list/service/ListPermissionServiceTest.java` around lines 135 - 147, Remove the redundant assertDoesNotThrow wrapper in the test method removeList_ExistingId_SoftDeletesPermission: replace the invocation assertDoesNotThrow(() -> listPermissionService.removeList(listPermission.getId())) with a direct call to listPermissionService.removeList(listPermission.getId()) so the Act step matches other tests and the subsequent verifications (verify(auditService, times(1)).softDelete(listPermission); verify(listPermissionRepository, times(1)).save(listPermission);) remain unchanged.
80-96: Strengthen the edit assertion to prove the managed entity is persisted.This test passes whether the service mutates the managed
listPermissionin place or saves a caller-provided instance, becausesave(any())is stubbed to return the same fixture andupdateDTO.permission()is compared against the mutated fixture. Given the AI summary highlights that the service was changed to update and persist the managed existing entity (rather than the caller's instance), consider using anArgumentCaptor<ListPermission>and asserting the captured argument is the same reference as the managedlistPermissionreturned byfindByIdAndDeletedFalse. That way a regression back to saving a fresh/caller-built entity would be caught.♻️ Suggested tightening
- verify(auditService, times(1)).setAuditData(listPermission, false); - verify(listPermissionRepository, times(1)).save(listPermission); + ArgumentCaptor<ListPermission> captor = ArgumentCaptor.forClass(ListPermission.class); + verify(auditService, times(1)).setAuditData(listPermission, false); + verify(listPermissionRepository, times(1)).save(captor.capture()); + assertSame(listPermission, captor.getValue()); + assertEquals(updateDTO.permission(), captor.getValue().getPermission());🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/test/java/com/omatheusmesmo/shoppmate/list/service/ListPermissionServiceTest.java` around lines 80 - 96, The test currently only checks the returned fixture and not what was actually passed to the repository; change the test to capture the saved entity with an ArgumentCaptor<ListPermission> and assert that the captured instance is the exact same reference as the managed listPermission returned by listPermissionRepository.findByIdAndDeletedFalse, and that its permission equals updateDTO.permission(); keep verifying auditService.setAuditData(listPermission, false) and that listPermissionRepository.save(...) was called once to ensure the service mutates and persists the managed entity rather than saving a caller-provided instance.backend/src/test/java/com/omatheusmesmo/shoppmate/item/controller/ItemControllerTest.java (3)
64-69: Remove the redundant ID de-duplication guard.
ItemTestFactory.createValidItem()already assigns IDs from anAtomicLong, so consecutive calls should be unique. Keeping this mutation makes the test setup more complex and can mask a broken factory.♻️ Proposed cleanup
item1 = ItemTestFactory.createValidItem(); item2 = ItemTestFactory.createValidItem(); - // Ensure unique IDs - if (item1.getId().equals(item2.getId())) { - item2.setId(item1.getId() + 1); - } CategoryResponseDTO categoryDTO1 = new CategoryResponseDTO(item1.getCategory().getId(),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/test/java/com/omatheusmesmo/shoppmate/item/controller/ItemControllerTest.java` around lines 64 - 69, Remove the redundant ID de-duplication guard in the test setup: since ItemTestFactory.createValidItem() already produces unique IDs via its AtomicLong, delete the conditional block that compares item1.getId() and item2.getId() and the subsequent item2.setId(...) mutation so the test relies on the factory (references: item1, item2, ItemTestFactory.createValidItem()).
71-79: Usevarfor obvious local declarations.These changed locals are initialized from constructors or clearly typed accessors, so they should use
varto align with the project’s Java 17 style. As per coding guidelines,backend/**/*.java: “Usevarfor local variables in Java 17 when the type is obvious.”Also applies to: 106-107, 123-123, 135-136, 154-154, 165-166
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/test/java/com/omatheusmesmo/shoppmate/item/controller/ItemControllerTest.java` around lines 71 - 79, In ItemControllerTest replace explicit local type declarations with var where the type is obvious: change CategoryResponseDTO, UnitResponseDTO, and ItemResponseDTO local declarations (e.g., the variables created for itemResponseDTO1/itemResponseDTO2 and their categoryDTO*/unitDTO* assignments) to use var; apply the same var substitution in the other mentioned locations (lines creating the DTO locals around startLine 106-107, 123, 135-136, 154, and 165-166) so constructors like new CategoryResponseDTO(...), new UnitResponseDTO(...), and new ItemResponseDTO(...) are assigned to var local variables to follow the Java 17 style guide.
3-27: Reorder imports to match the backend import convention.The new
ItemTestFactoryimport is added to a file whose imports are still grouped as project → JUnit/Mockito/Spring → java. Please reorder this block tojava.*,jakarta.*,org.springframework.*, third-party, then project imports. As per coding guidelines,backend/**/*.java: “Order imports as: java., jakarta., org.springframework.*, third-party, project packages. Use specific imports, no wildcards.”🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/test/java/com/omatheusmesmo/shoppmate/item/controller/ItemControllerTest.java` around lines 3 - 27, Reorder the import block in ItemControllerTest to follow project conventions: place all java.* imports first (e.g., java.util.Arrays, List, NoSuchElementException), then jakarta.* if any, then org.springframework.* and related Spring/Security testing imports (MockMvc, AutoConfigureMockMvc, WebMvcTest, MockBean, MediaType, WithMockUser, etc.), then third-party (JUnit/Mockito like org.junit.jupiter.api.*, org.mockito.Mockito), and finally project imports (com.omatheusmesmo.* including ItemTestFactory, ItemService, ItemMapper, DTOs, JwtService); ensure no wildcard imports are introduced and keep specific imports like ItemTestFactory in the project group.backend/src/test/java/com/omatheusmesmo/shoppmate/shared/testutils/UserTestFactory.java (3)
13-20: Usevarfor the obvious local type and reuse one timestamp.
new User()makes the local type obvious, and using onenowvalue keeps newly created fixture timestamps consistent.♻️ Proposed fixture cleanup
- User user = new User(); + var user = new User(); + var now = LocalDateTime.now(); user.setId(ID_GENERATOR.getAndIncrement()); user.setFullName(FakerUtil.getFaker().name().fullName()); user.setEmail(FakerUtil.getFaker().internet().emailAddress()); user.setPassword("password"); user.setRole("USER"); - user.setCreatedAt(LocalDateTime.now()); - user.setUpdatedAt(LocalDateTime.now()); + user.setCreatedAt(now); + user.setUpdatedAt(now);As per coding guidelines, Use
varfor local variables in Java 17 when the type is obvious.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/test/java/com/omatheusmesmo/shoppmate/shared/testutils/UserTestFactory.java` around lines 13 - 20, In UserTestFactory replace the explicit local type for the fixture with var (e.g., var user = new User()) and compute a single LocalDateTime now = LocalDateTime.now() once, then reuse that same timestamp for both user.setCreatedAt(...) and user.setUpdatedAt(...); this keeps the local type concise and ensures consistent timestamps for the User fixture.
8-10: Make the static factory non-instantiable.This class only exposes static factory helpers; marking it
finalwith a private constructor makes that intent explicit.♻️ Proposed utility-class cleanup
-public class UserTestFactory { +public final class UserTestFactory { private static final AtomicLong ID_GENERATOR = new AtomicLong(1); + + private UserTestFactory() { + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/test/java/com/omatheusmesmo/shoppmate/shared/testutils/UserTestFactory.java` around lines 8 - 10, Mark the utility test factory class as non-instantiable by declaring UserTestFactory as final and adding a private constructor (e.g., private UserTestFactory() {}) to prevent instantiation; keep existing static members like ID_GENERATOR and any static factory helper methods unchanged so callers continue to use the static API.
3-6: Reorder imports to match the repository convention.Project imports currently come before
java.*imports.♻️ Proposed import-order fix
-import com.omatheusmesmo.shoppmate.user.dtos.RegisterUserDTO; -import com.omatheusmesmo.shoppmate.user.entity.User; import java.time.LocalDateTime; import java.util.concurrent.atomic.AtomicLong; + +import com.omatheusmesmo.shoppmate.user.dtos.RegisterUserDTO; +import com.omatheusmesmo.shoppmate.user.entity.User;As per coding guidelines, Order imports as: java., jakarta., org.springframework.*, third-party, project packages. Use specific imports, no wildcards.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/test/java/com/omatheusmesmo/shoppmate/shared/testutils/UserTestFactory.java` around lines 3 - 6, Reorder the imports in UserTestFactory.java so java.* imports come first (e.g., java.time.LocalDateTime, java.util.concurrent.atomic.AtomicLong), followed by jakarta.*/org.springframework.* (if any), then third‑party, and finally project packages (com.omatheusmesmo.shoppmate.user.dtos.RegisterUserDTO and com.omatheusmesmo.shoppmate.user.entity.User); ensure no wildcard imports are introduced and keep imports specific for the RegisterUserDTO and User symbols.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In
`@backend/src/test/java/com/omatheusmesmo/shoppmate/list/controller/ListItemControllerTest.java`:
- Around line 3-24: Reorder the imports in ListItemControllerTest so they follow
the repo convention: group and sort imports as java.* first (e.g.,
java.util.List), then jakarta.* (if any), then org.springframework.* (MockMvc,
annotations), then third-party libraries (Jackson, JUnit, Mockito, etc.), and
finally project packages (com.omatheusmesmo.*); ensure no wildcard imports and
alphabetize within each group; leave the class name ListItemControllerTest and
all referenced symbols (ObjectMapper, MockMvc, WithMockUser, ListItemService,
ListTestFactory, etc.) unchanged while only adjusting import ordering and
grouping.
In
`@backend/src/test/java/com/omatheusmesmo/shoppmate/list/service/ListItemServiceTest.java`:
- Around line 3-25: Reorder and clean the import block in ListItemServiceTest:
group imports as java.*, jakarta.* (if any), org.springframework.* (if any),
third-party, then project packages, and consolidate java.* into a single block;
also remove wildcard static imports (currently static
org.junit.jupiter.api.Assertions.* and static org.mockito.Mockito.*) and replace
them with explicit static imports for only the methods used in this test (for
example assertEquals, assertNotNull, assertThrows, when, verify, etc.), so
update the import list around the classes referenced (ListItemServiceTest,
ListItemService methods under test, ListTestFactory, ListItemMapper,
ListItemRepository, ItemService, AuditService) accordingly.
---
Nitpick comments:
In
`@backend/src/test/java/com/omatheusmesmo/shoppmate/item/controller/ItemControllerTest.java`:
- Around line 64-69: Remove the redundant ID de-duplication guard in the test
setup: since ItemTestFactory.createValidItem() already produces unique IDs via
its AtomicLong, delete the conditional block that compares item1.getId() and
item2.getId() and the subsequent item2.setId(...) mutation so the test relies on
the factory (references: item1, item2, ItemTestFactory.createValidItem()).
- Around line 71-79: In ItemControllerTest replace explicit local type
declarations with var where the type is obvious: change CategoryResponseDTO,
UnitResponseDTO, and ItemResponseDTO local declarations (e.g., the variables
created for itemResponseDTO1/itemResponseDTO2 and their categoryDTO*/unitDTO*
assignments) to use var; apply the same var substitution in the other mentioned
locations (lines creating the DTO locals around startLine 106-107, 123, 135-136,
154, and 165-166) so constructors like new CategoryResponseDTO(...), new
UnitResponseDTO(...), and new ItemResponseDTO(...) are assigned to var local
variables to follow the Java 17 style guide.
- Around line 3-27: Reorder the import block in ItemControllerTest to follow
project conventions: place all java.* imports first (e.g., java.util.Arrays,
List, NoSuchElementException), then jakarta.* if any, then org.springframework.*
and related Spring/Security testing imports (MockMvc, AutoConfigureMockMvc,
WebMvcTest, MockBean, MediaType, WithMockUser, etc.), then third-party
(JUnit/Mockito like org.junit.jupiter.api.*, org.mockito.Mockito), and finally
project imports (com.omatheusmesmo.* including ItemTestFactory, ItemService,
ItemMapper, DTOs, JwtService); ensure no wildcard imports are introduced and
keep specific imports like ItemTestFactory in the project group.
In
`@backend/src/test/java/com/omatheusmesmo/shoppmate/list/service/ListPermissionServiceTest.java`:
- Around line 135-147: Remove the redundant assertDoesNotThrow wrapper in the
test method removeList_ExistingId_SoftDeletesPermission: replace the invocation
assertDoesNotThrow(() ->
listPermissionService.removeList(listPermission.getId())) with a direct call to
listPermissionService.removeList(listPermission.getId()) so the Act step matches
other tests and the subsequent verifications (verify(auditService,
times(1)).softDelete(listPermission); verify(listPermissionRepository,
times(1)).save(listPermission);) remain unchanged.
- Around line 80-96: The test currently only checks the returned fixture and not
what was actually passed to the repository; change the test to capture the saved
entity with an ArgumentCaptor<ListPermission> and assert that the captured
instance is the exact same reference as the managed listPermission returned by
listPermissionRepository.findByIdAndDeletedFalse, and that its permission equals
updateDTO.permission(); keep verifying auditService.setAuditData(listPermission,
false) and that listPermissionRepository.save(...) was called once to ensure the
service mutates and persists the managed entity rather than saving a
caller-provided instance.
In
`@backend/src/test/java/com/omatheusmesmo/shoppmate/shared/testutils/UserTestFactory.java`:
- Around line 13-20: In UserTestFactory replace the explicit local type for the
fixture with var (e.g., var user = new User()) and compute a single
LocalDateTime now = LocalDateTime.now() once, then reuse that same timestamp for
both user.setCreatedAt(...) and user.setUpdatedAt(...); this keeps the local
type concise and ensures consistent timestamps for the User fixture.
- Around line 8-10: Mark the utility test factory class as non-instantiable by
declaring UserTestFactory as final and adding a private constructor (e.g.,
private UserTestFactory() {}) to prevent instantiation; keep existing static
members like ID_GENERATOR and any static factory helper methods unchanged so
callers continue to use the static API.
- Around line 3-6: Reorder the imports in UserTestFactory.java so java.* imports
come first (e.g., java.time.LocalDateTime,
java.util.concurrent.atomic.AtomicLong), followed by
jakarta.*/org.springframework.* (if any), then third‑party, and finally project
packages (com.omatheusmesmo.shoppmate.user.dtos.RegisterUserDTO and
com.omatheusmesmo.shoppmate.user.entity.User); ensure no wildcard imports are
introduced and keep imports specific for the RegisterUserDTO and User symbols.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4080e574-9f31-44a3-b742-fb13e0e4866a
📒 Files selected for processing (19)
backend/pom.xmlbackend/src/main/java/com/omatheusmesmo/shoppmate/list/service/ShoppingListService.javabackend/src/test/java/com/omatheusmesmo/shoppmate/auth/controller/AuthControllerTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/auth/service/CustomUserDetailsServiceTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/category/controller/CategoryControllerWithIntegrationTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/item/controller/ItemControllerTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/list/controller/ListItemControllerTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/list/controller/ListPermissionControllerTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/list/controller/ShoppingListControllerTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/list/service/ListItemServiceTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/list/service/ListPermissionServiceTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/shared/testutils/CategoryTestFactory.javabackend/src/test/java/com/omatheusmesmo/shoppmate/shared/testutils/FakerUtil.javabackend/src/test/java/com/omatheusmesmo/shoppmate/shared/testutils/ItemTestFactory.javabackend/src/test/java/com/omatheusmesmo/shoppmate/shared/testutils/ListTestFactory.javabackend/src/test/java/com/omatheusmesmo/shoppmate/shared/testutils/UnitTestFactory.javabackend/src/test/java/com/omatheusmesmo/shoppmate/shared/testutils/UserTestFactory.javabackend/src/test/java/com/omatheusmesmo/shoppmate/user/controller/UserControllerTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/user/service/UserServiceTest.java
✅ Files skipped from review due to trivial changes (8)
- backend/src/test/java/com/omatheusmesmo/shoppmate/shared/testutils/FakerUtil.java
- backend/src/test/java/com/omatheusmesmo/shoppmate/shared/testutils/ItemTestFactory.java
- backend/src/test/java/com/omatheusmesmo/shoppmate/shared/testutils/CategoryTestFactory.java
- backend/src/test/java/com/omatheusmesmo/shoppmate/auth/service/CustomUserDetailsServiceTest.java
- backend/src/test/java/com/omatheusmesmo/shoppmate/list/controller/ShoppingListControllerTest.java
- backend/src/main/java/com/omatheusmesmo/shoppmate/list/service/ShoppingListService.java
- backend/src/test/java/com/omatheusmesmo/shoppmate/shared/testutils/UnitTestFactory.java
- backend/src/test/java/com/omatheusmesmo/shoppmate/auth/controller/AuthControllerTest.java
🚧 Files skipped from review as they are similar to previous changes (6)
- backend/src/test/java/com/omatheusmesmo/shoppmate/user/controller/UserControllerTest.java
- backend/src/test/java/com/omatheusmesmo/shoppmate/list/controller/ListPermissionControllerTest.java
- backend/pom.xml
- backend/src/test/java/com/omatheusmesmo/shoppmate/user/service/UserServiceTest.java
- backend/src/test/java/com/omatheusmesmo/shoppmate/shared/testutils/ListTestFactory.java
- backend/src/test/java/com/omatheusmesmo/shoppmate/category/controller/CategoryControllerWithIntegrationTest.java
matheusandre1
approved these changes
Apr 18, 2026
Collaborator
|
Good , Thanks for pr, I need someone who can rebase, resolve conflicts, and squash commit into one. |
Hlib-Rachkovskyy
force-pushed
the
feature/agnostic-category-tests
branch
from
51b918d to
5ff99a4
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (6)
backend/src/test/java/com/omatheusmesmo/shoppmate/item/controller/ItemControllerWithIntegrationTest.java (1)
191-198: Minor inconsistency: discard vs. reassign saved entity.
createCategoryToTestandcreateUnitToTestreassign the result ofrepository.save(...), butcreateItemToTestignores the return value on Line 196. While Hibernate typically mutates the passed entity's id in place, using the returned managed instance is safer and consistent with the sibling helpers.♻️ Proposed refactor
Item createItemToTest() { Item itemEntity = ItemTestFactory.createValidItem(); itemEntity.setId(null); // Let DB generate ID itemEntity.setCategory(createCategoryToTest()); itemEntity.setUnit(createUnitToTest()); - itemRepository.save(itemEntity); - return itemEntity; + return itemRepository.save(itemEntity); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/test/java/com/omatheusmesmo/shoppmate/item/controller/ItemControllerWithIntegrationTest.java` around lines 191 - 198, The helper createItemToTest currently calls itemRepository.save(itemEntity) but ignores the returned managed instance; to match createCategoryToTest and createUnitToTest and be safer, assign the result of itemRepository.save(...) back to itemEntity (or a new variable) and return that saved instance so the managed entity (with generated id and persistence context attached) is returned from createItemToTest.backend/src/test/java/com/omatheusmesmo/shoppmate/category/controller/CategoryControllerTest.java (1)
62-67: Unreachable collision guard.
CategoryTestFactory.createValidCategory()generates IDs via a staticAtomicLong.getAndIncrement(), so two sequential calls in the same JVM will never produce the same ID. Theif (category1.getId().equals(category2.getId()))branch is dead code and the comment "Ensure unique IDs for test sorting logic" is misleading. Safe to remove.♻️ Proposed cleanup
category1 = CategoryTestFactory.createValidCategory(); category2 = CategoryTestFactory.createValidCategory(); - // Ensure unique IDs for test sorting logic if any - if (category1.getId().equals(category2.getId())) { - category2.setId(category1.getId() + 1); - } - category1ResponseDTO = new CategoryResponseDTO(category1.getId(), category1.getName()); category2ResponseDTO = new CategoryResponseDTO(category2.getId(), category2.getName());🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/test/java/com/omatheusmesmo/shoppmate/category/controller/CategoryControllerTest.java` around lines 62 - 67, The conditional guard and comment are dead code: since CategoryTestFactory.createValidCategory() uses a static AtomicLong to assign IDs, two sequential calls cannot collide, so remove the entire if block that checks category1.getId().equals(category2.getId()) and the "Ensure unique IDs..." comment; simply create category1 and category2 via CategoryTestFactory.createValidCategory() and rely on their unique IDs (references: CategoryTestFactory.createValidCategory(), category1, category2, getId()).backend/src/test/java/com/omatheusmesmo/shoppmate/category/service/CategoryServiceTest.java (1)
3-17: Split project imports across two groups.Project imports are interleaved:
category.repositoryandshared.testutilsappear before the third-party/Mockito block (lines 7-8), whilecategory.entityandshared.serviceappear after it (lines 16-17). Consolidate them into a single project group placed after third-party imports.As per coding guidelines: "Import package structure:
com.omatheusmesmo.shoppmate.<domain>. Order: java., jakarta., org.springframework.*, third-party, project."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/test/java/com/omatheusmesmo/shoppmate/category/service/CategoryServiceTest.java` around lines 3 - 17, The imports in CategoryServiceTest are split; consolidate all project imports (e.g., com.omatheusmesmo.shoppmate.category.repository.CategoryRepository, com.omatheusmesmo.shoppmate.shared.testutils.CategoryTestFactory, com.omatheusmesmo.shoppmate.category.entity.Category, com.omatheusmesmo.shoppmate.shared.service.AuditService) into a single project group and place that group after the third-party imports (Mockito/JUnit) following the project's import ordering rule (java.*, jakarta.*, org.springframework.*, third-party, project); update the import block in the CategoryServiceTest class so project imports are not interleaved with Mockito/JUnit imports.backend/src/test/java/com/omatheusmesmo/shoppmate/unit/service/UnitServiceTest.java (1)
3-16: Reorder imports to match the repository convention.Project imports currently appear before
java.*and third-party imports. Please order them asjava.*,jakarta.*,org.springframework.*, third-party, then project imports.♻️ Proposed import order
-import com.omatheusmesmo.shoppmate.shared.service.AuditService; -import com.omatheusmesmo.shoppmate.shared.testutils.UnitTestFactory; -import com.omatheusmesmo.shoppmate.unit.entity.Unit; -import com.omatheusmesmo.shoppmate.unit.repository.UnitRepository; -import com.omatheusmesmo.shoppmate.unit.service.UnitService; +import java.util.List; +import java.util.Optional; + import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; -import java.util.List; -import java.util.Optional; +import com.omatheusmesmo.shoppmate.shared.service.AuditService; +import com.omatheusmesmo.shoppmate.shared.testutils.UnitTestFactory; +import com.omatheusmesmo.shoppmate.unit.entity.Unit; +import com.omatheusmesmo.shoppmate.unit.repository.UnitRepository; +import com.omatheusmesmo.shoppmate.unit.service.UnitService;As per coding guidelines, imports must be ordered as:
java.*,jakarta.*,org.springframework.*, third-party, project packages.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/test/java/com/omatheusmesmo/shoppmate/unit/service/UnitServiceTest.java` around lines 3 - 16, The imports in UnitServiceTest are not ordered per project convention; reorder them so java.* imports (e.g., java.util.List, java.util.Optional) appear first, followed by org.* test frameworks (e.g., org.junit.jupiter.*, org.mockito.*, org.mockito.junit.jupiter.MockitoExtension), then any third-party packages if present, and finally the project imports (e.g., com.omatheusmesmo.shoppmate.unit.* and other com.omatheusmesmo.shoppmate.* classes such as Unit, UnitRepository, UnitService, AuditService, UnitTestFactory); update the import block in the UnitServiceTest class to follow that sequence.backend/src/test/java/com/omatheusmesmo/shoppmate/shared/testutils/ListTestFactory.java (1)
45-55: Consider randomizingPermissionfor consistency with DTO factories.
createValidListPermissionhardcodesPermission.READ, whilecreateValidListPermissionRequestDTO/...UpdateRequestDTOuseFakerUtil.getFaker().options().option(Permission.class). For a data-agnostic suite (the PR's stated goal), randomizing here too improves variability — unless tests rely on a specific initial permission, in which case a dedicatedcreateListPermissionWithRole(...)overload would be clearer.- permission.setPermission(Permission.READ); + permission.setPermission(FakerUtil.getFaker().options().option(Permission.class));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/test/java/com/omatheusmesmo/shoppmate/shared/testutils/ListTestFactory.java` around lines 45 - 55, The factory method createValidListPermission currently hardcodes Permission.READ; update it to choose a random Permission like the DTO factories do (use FakerUtil.getFaker().options().option(Permission.class)) so tests are data-agnostic and consistent with createValidListPermissionRequestDTO/createValidListPermissionUpdateRequestDTO, or alternatively add an overload createListPermissionWithRole(ShoppingList list, Permission permission) to allow explicit roles when tests require a specific permission.backend/src/test/java/com/omatheusmesmo/shoppmate/user/controller/UserControllerTest.java (1)
3-18: Move the new project import into the project-import group.The added
UserTestFactoryimport keeps this block out of the repository’s required import order.♻️ Suggested import ordering
+import java.util.List; + +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc; +import org.springframework.boot.test.autoconfigure.web.servlet.WebMvcTest; +import org.springframework.boot.test.mock.mockito.MockBean; +import org.springframework.security.core.userdetails.UserDetailsService; +import org.springframework.security.test.context.support.WithMockUser; +import org.springframework.test.web.servlet.MockMvc; + import com.fasterxml.jackson.databind.ObjectMapper; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + import com.omatheusmesmo.shoppmate.auth.service.JwtService; import com.omatheusmesmo.shoppmate.shared.testutils.UserTestFactory; import com.omatheusmesmo.shoppmate.user.entity.User; import com.omatheusmesmo.shoppmate.user.service.UserService; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; -import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc; -import org.springframework.boot.test.autoconfigure.web.servlet.WebMvcTest; -import org.springframework.boot.test.mock.mockito.MockBean; -import org.springframework.security.core.userdetails.UserDetailsService; -import org.springframework.security.test.context.support.WithMockUser; -import org.springframework.test.web.servlet.MockMvc; - -import java.util.List;As per coding guidelines,
backend/**/*.java: “Order imports as: java., jakarta., org.springframework.*, third-party, project packages.”🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/test/java/com/omatheusmesmo/shoppmate/user/controller/UserControllerTest.java` around lines 3 - 18, The import ordering is incorrect: move the project package import UserTestFactory into the project-import group so imports follow the rule (java.*, jakarta.*, org.springframework.*, third-party, then project packages); update the import block in UserControllerTest to place com.omatheusmesmo.shoppmate.shared.testutils.UserTestFactory after third-party imports and before other project imports so the file complies with the backend/**/*.java import ordering guideline.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@backend/src/test/java/com/omatheusmesmo/shoppmate/auth/service/JwtServiceTest.java`:
- Line 50: Rename the test methods in JwtServiceTest to follow the
methodName_Scenario_ExpectedBehavior convention (for example change
shouldGenerateAndValidateValidToken() to
generateToken_ValidClaims_ReturnsValidToken or similar); update each test method
in the class (including the other methods referenced at lines 61, 75, 88, 98,
106, 115, 125, 131) to use a clear methodName_Scenario_ExpectedBehavior name,
keeping the test logic and signatures intact and only changing the method names.
- Line 108: In JwtServiceTest replace the realistic JWT-shaped literal assigned
to malformedJwe with an obviously fake/non-JWT value (e.g., "malformed-token" or
"INVALID_TOKEN") so the test still exercises invalid-token behavior but no
longer resembles a real JWT; update the variable malformedJwe in the
JwtServiceTest class accordingly.
- Around line 34-39: Tests instantiate JwtService with (RSAPublicKey,
RSAPrivateKey, long) but the class only exposes a no-arg/Spring constructor; add
a public constructor JwtService(RSAPublicKey publicKey, RSAPrivateKey
privateKey, long tokenExpiration) to the JwtService class that assigns the
corresponding fields (publicKey, privateKey, tokenExpiration) and preserves any
existing initialization logic, and update/ensure both test sites (setup near
generateRSAKeys() and the other usage around lines referenced 144-146) will
compile against this new constructor; alternatively, if you prefer not to change
production code, modify the JwtServiceTest to obtain JwtService via the existing
no-arg/Spring configuration path instead.
- Line 17: Replace the wildcard static import in JwtServiceTest with explicit
static imports for only the assertion methods actually used in this test class
(e.g., assertEquals, assertNotNull, assertThrows, assertTrue, etc.); update the
import line to list those specific assertion symbols so the file follows the
repository policy and only imports the assertions referenced by JwtServiceTest.
In
`@backend/src/test/java/com/omatheusmesmo/shoppmate/shared/testutils/ListTestFactory.java`:
- Around line 57-59: The factory method
ListTestFactory.createValidShoppingListRequestDTO currently calls the
ShoppingListRequestDTO constructor with (String, Long) but
ShoppingListRequestDTO is a record that only accepts String name; remove the
Long userId parameter from the factory method signature and from the
ShoppingListRequestDTO constructor call so the method only constructs new
ShoppingListRequestDTO(name), and update the call site in
ShoppingListControllerTest (the invocation at line referenced) to call
createValidShoppingListRequestDTO() without passing userId.
---
Nitpick comments:
In
`@backend/src/test/java/com/omatheusmesmo/shoppmate/category/controller/CategoryControllerTest.java`:
- Around line 62-67: The conditional guard and comment are dead code: since
CategoryTestFactory.createValidCategory() uses a static AtomicLong to assign
IDs, two sequential calls cannot collide, so remove the entire if block that
checks category1.getId().equals(category2.getId()) and the "Ensure unique
IDs..." comment; simply create category1 and category2 via
CategoryTestFactory.createValidCategory() and rely on their unique IDs
(references: CategoryTestFactory.createValidCategory(), category1, category2,
getId()).
In
`@backend/src/test/java/com/omatheusmesmo/shoppmate/category/service/CategoryServiceTest.java`:
- Around line 3-17: The imports in CategoryServiceTest are split; consolidate
all project imports (e.g.,
com.omatheusmesmo.shoppmate.category.repository.CategoryRepository,
com.omatheusmesmo.shoppmate.shared.testutils.CategoryTestFactory,
com.omatheusmesmo.shoppmate.category.entity.Category,
com.omatheusmesmo.shoppmate.shared.service.AuditService) into a single project
group and place that group after the third-party imports (Mockito/JUnit)
following the project's import ordering rule (java.*, jakarta.*,
org.springframework.*, third-party, project); update the import block in the
CategoryServiceTest class so project imports are not interleaved with
Mockito/JUnit imports.
In
`@backend/src/test/java/com/omatheusmesmo/shoppmate/item/controller/ItemControllerWithIntegrationTest.java`:
- Around line 191-198: The helper createItemToTest currently calls
itemRepository.save(itemEntity) but ignores the returned managed instance; to
match createCategoryToTest and createUnitToTest and be safer, assign the result
of itemRepository.save(...) back to itemEntity (or a new variable) and return
that saved instance so the managed entity (with generated id and persistence
context attached) is returned from createItemToTest.
In
`@backend/src/test/java/com/omatheusmesmo/shoppmate/shared/testutils/ListTestFactory.java`:
- Around line 45-55: The factory method createValidListPermission currently
hardcodes Permission.READ; update it to choose a random Permission like the DTO
factories do (use FakerUtil.getFaker().options().option(Permission.class)) so
tests are data-agnostic and consistent with
createValidListPermissionRequestDTO/createValidListPermissionUpdateRequestDTO,
or alternatively add an overload createListPermissionWithRole(ShoppingList list,
Permission permission) to allow explicit roles when tests require a specific
permission.
In
`@backend/src/test/java/com/omatheusmesmo/shoppmate/unit/service/UnitServiceTest.java`:
- Around line 3-16: The imports in UnitServiceTest are not ordered per project
convention; reorder them so java.* imports (e.g., java.util.List,
java.util.Optional) appear first, followed by org.* test frameworks (e.g.,
org.junit.jupiter.*, org.mockito.*, org.mockito.junit.jupiter.MockitoExtension),
then any third-party packages if present, and finally the project imports (e.g.,
com.omatheusmesmo.shoppmate.unit.* and other com.omatheusmesmo.shoppmate.*
classes such as Unit, UnitRepository, UnitService, AuditService,
UnitTestFactory); update the import block in the UnitServiceTest class to follow
that sequence.
In
`@backend/src/test/java/com/omatheusmesmo/shoppmate/user/controller/UserControllerTest.java`:
- Around line 3-18: The import ordering is incorrect: move the project package
import UserTestFactory into the project-import group so imports follow the rule
(java.*, jakarta.*, org.springframework.*, third-party, then project packages);
update the import block in UserControllerTest to place
com.omatheusmesmo.shoppmate.shared.testutils.UserTestFactory after third-party
imports and before other project imports so the file complies with the
backend/**/*.java import ordering guideline.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 62315768-8085-4e28-8f6a-0d56b3cb28bf
📒 Files selected for processing (27)
backend/AGENTS.mdbackend/pom.xmlbackend/src/test/java/com/omatheusmesmo/shoppmate/auth/controller/AuthControllerTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/auth/service/CustomUserDetailsServiceTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/auth/service/JwtServiceTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/category/controller/CategoryControllerTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/category/controller/CategoryControllerWithIntegrationTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/category/service/CategoryServiceTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/item/controller/ItemControllerTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/item/controller/ItemControllerWithIntegrationTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/item/service/ItemServiceTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/list/controller/ListItemControllerTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/list/controller/ListPermissionControllerTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/list/controller/ShoppingListControllerTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/list/service/ListItemServiceTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/list/service/ListPermissionServiceTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/list/service/ShoppingListServiceTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/shared/service/AuditServiceTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/shared/testutils/CategoryTestFactory.javabackend/src/test/java/com/omatheusmesmo/shoppmate/shared/testutils/FakerUtil.javabackend/src/test/java/com/omatheusmesmo/shoppmate/shared/testutils/ItemTestFactory.javabackend/src/test/java/com/omatheusmesmo/shoppmate/shared/testutils/ListTestFactory.javabackend/src/test/java/com/omatheusmesmo/shoppmate/shared/testutils/UnitTestFactory.javabackend/src/test/java/com/omatheusmesmo/shoppmate/shared/testutils/UserTestFactory.javabackend/src/test/java/com/omatheusmesmo/shoppmate/unit/service/UnitServiceTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/user/controller/UserControllerTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/user/service/UserServiceTest.java
✅ Files skipped from review due to trivial changes (10)
- backend/AGENTS.md
- backend/src/test/java/com/omatheusmesmo/shoppmate/item/service/ItemServiceTest.java
- backend/src/test/java/com/omatheusmesmo/shoppmate/shared/testutils/FakerUtil.java
- backend/src/test/java/com/omatheusmesmo/shoppmate/shared/service/AuditServiceTest.java
- backend/src/test/java/com/omatheusmesmo/shoppmate/shared/testutils/UnitTestFactory.java
- backend/src/test/java/com/omatheusmesmo/shoppmate/list/service/ShoppingListServiceTest.java
- backend/src/test/java/com/omatheusmesmo/shoppmate/shared/testutils/CategoryTestFactory.java
- backend/src/test/java/com/omatheusmesmo/shoppmate/list/controller/ListPermissionControllerTest.java
- backend/src/test/java/com/omatheusmesmo/shoppmate/shared/testutils/ItemTestFactory.java
- backend/src/test/java/com/omatheusmesmo/shoppmate/list/service/ListPermissionServiceTest.java
🚧 Files skipped from review as they are similar to previous changes (9)
- backend/src/test/java/com/omatheusmesmo/shoppmate/list/controller/ListItemControllerTest.java
- backend/src/test/java/com/omatheusmesmo/shoppmate/item/controller/ItemControllerTest.java
- backend/src/test/java/com/omatheusmesmo/shoppmate/auth/service/CustomUserDetailsServiceTest.java
- backend/src/test/java/com/omatheusmesmo/shoppmate/shared/testutils/UserTestFactory.java
- backend/src/test/java/com/omatheusmesmo/shoppmate/list/service/ListItemServiceTest.java
- backend/src/test/java/com/omatheusmesmo/shoppmate/auth/controller/AuthControllerTest.java
- backend/pom.xml
- backend/src/test/java/com/omatheusmesmo/shoppmate/list/controller/ShoppingListControllerTest.java
- backend/src/test/java/com/omatheusmesmo/shoppmate/user/service/UserServiceTest.java
| @Test | ||
| void shouldNotValidateMalformedJweToken() { | ||
| // Arrange | ||
| String malformedJwe = "eyJhbGciOiJIUzI1NiJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c"; |
Contributor
There was a problem hiding this comment.
Avoid realistic JWT-shaped literals in invalid-token tests.
This value triggered the secret scanner. The test only needs malformed input, so use an obviously fake value that cannot be mistaken for a real token.
Proposed test-data fix
- String malformedJwe = "eyJhbGciOiJIUzI1NiJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c";
+ String malformedJwe = String.join(".", "not", "a", "valid", "jwe", "token");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| String malformedJwe = "eyJhbGciOiJIUzI1NiJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c"; | |
| String malformedJwe = String.join(".", "not", "a", "valid", "jwe", "token"); |
🧰 Tools
🪛 Betterleaks (1.1.2)
[high] 108-108: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@backend/src/test/java/com/omatheusmesmo/shoppmate/auth/service/JwtServiceTest.java`
at line 108, In JwtServiceTest replace the realistic JWT-shaped literal assigned
to malformedJwe with an obviously fake/non-JWT value (e.g., "malformed-token" or
"INVALID_TOKEN") so the test still exercises invalid-token behavior but no
longer resembles a real JWT; update the variable malformedJwe in the
JwtServiceTest class accordingly.
193a2ef to
10c2802
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
backend/src/test/java/com/omatheusmesmo/shoppmate/user/service/UserServiceTest.java (1)
170-180:⚠️ Potential issue | 🟠 MajorCall
editUser(...)or remove the unused stubs.These tests still stub
findById(...)but only callvalidateIfDataIsNullOrEmpty(...)through the helper, so the stubs are unused and the scenario does not exerciseeditUser.Proposed fix
void editUser_EmailNull_ThrowsIllegalArgumentException() { when(userRepository.findById(userMock.getId())).thenReturn(Optional.of(userMock)); userMock.setEmail(null); - assertValidationException(userMock, "E-mail is required!"); + IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, + () -> userService.editUser(userMock)); + assertEquals("E-mail is required!", exception.getMessage()); } `@Test` void editUser_PasswordNull_ThrowsIllegalArgumentException() { when(userRepository.findById(userMock.getId())).thenReturn(Optional.of(userMock)); userMock.setPassword(null); - assertValidationException(userMock, "Password is required!"); + IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, + () -> userService.editUser(userMock)); + assertEquals("Password is required!", exception.getMessage()); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/test/java/com/omatheusmesmo/shoppmate/user/service/UserServiceTest.java` around lines 170 - 180, The tests stub userRepository.findById(...) but never call editUser(...) (they only exercise validateIfDataIsNullOrEmpty via assertValidationException), so remove the unused stubbing or change the test to call the real method under test; update each test (e.g., editUser_EmailNull_ThrowsIllegalArgumentException and editUser_PasswordNull_ThrowsIllegalArgumentException) to either (A) remove the when(userRepository.findById(userMock.getId()))... line if you intend to unit-test validateIfDataIsNullOrEmpty in isolation, or (B) actually invoke userService.editUser(userMock.getId(), userMock) so the stubbed findById(...) is used and the exception path is exercised; ensure you reference userMock, userRepository.findById, editUser, and assertValidationException appropriately.
🧹 Nitpick comments (1)
backend/src/test/java/com/omatheusmesmo/shoppmate/list/controller/ShoppingListControllerTest.java (1)
87-91: Keep the POST request fixture aligned with the mocked response.
requestDTOnow has an independently generated name, but the mocked mapper/service still returns theshoppingListfromsetUp(). The test can pass even if the controller ignores or misroutes the request body. Tie the created entity/response torequestDTO.name()or stub the mapper with the exact request.🔧 Proposed test tightening
// Arrange ShoppingListRequestDTO requestDTO = ListTestFactory.createValidShoppingListRequestDTO(); - when(listMapper.toEntity(any(ShoppingListRequestDTO.class), any(User.class))).thenReturn(shoppingList); - when(shoppingListService.saveList(any(ShoppingList.class))).thenReturn(shoppingList); - when(listMapper.toResponseDTO(any(ShoppingList.class))).thenReturn(responseDTO); + shoppingList.setName(requestDTO.name()); + ShoppingListResponseDTO expectedResponseDTO = new ShoppingListResponseDTO( + shoppingList.getId(), + requestDTO.name(), + responseDTO.owner(), + responseDTO.totalValue()); + + when(listMapper.toEntity(any(ShoppingListRequestDTO.class), any(User.class))).thenReturn(shoppingList); + when(shoppingListService.saveList(any(ShoppingList.class))).thenReturn(shoppingList); + when(listMapper.toResponseDTO(any(ShoppingList.class))).thenReturn(expectedResponseDTO);Then assert
expectedResponseDTOinstead ofresponseDTO.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/test/java/com/omatheusmesmo/shoppmate/list/controller/ShoppingListControllerTest.java` around lines 87 - 91, The test creates a requestDTO with a generated name but still stubs listMapper.toEntity / shoppingListService.saveList / listMapper.toResponseDTO to return the fixed shoppingList/responseDTO from setUp(), so the test can pass even if the controller ignores the request body; change the stubs to be tied to requestDTO (e.g., stub listMapper.toEntity(requestDTO, any(User.class)) to return an entity whose name is requestDTO.name(), and stub listMapper.toResponseDTO to return an expectedResponseDTO built from that entity), then assert against expectedResponseDTO (or at least assert that the returned response has name == requestDTO.name()) to ensure the POST payload is actually used by the controller.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@backend/src/test/java/com/omatheusmesmo/shoppmate/user/service/UserServiceTest.java`:
- Around line 72-82: The test
UserServiceTest.addUser_EmailAlreadyUsed_ThrowsIllegalArgumentException
incorrectly stubs userRepository.save(...) to throw, so it doesn't verify that
UserService.addUser(...) checks userRepository.findByEmail(...) first; update
the test to stub userRepository.findByEmail(...) to return an
Optional.of(existingUser) (or appropriate non-empty result) and remove or stop
stubbing save() to force the duplicate-email branch in addUser to throw the
IllegalArgumentException, then assert the exception message as before.
---
Duplicate comments:
In
`@backend/src/test/java/com/omatheusmesmo/shoppmate/user/service/UserServiceTest.java`:
- Around line 170-180: The tests stub userRepository.findById(...) but never
call editUser(...) (they only exercise validateIfDataIsNullOrEmpty via
assertValidationException), so remove the unused stubbing or change the test to
call the real method under test; update each test (e.g.,
editUser_EmailNull_ThrowsIllegalArgumentException and
editUser_PasswordNull_ThrowsIllegalArgumentException) to either (A) remove the
when(userRepository.findById(userMock.getId()))... line if you intend to
unit-test validateIfDataIsNullOrEmpty in isolation, or (B) actually invoke
userService.editUser(userMock.getId(), userMock) so the stubbed findById(...) is
used and the exception path is exercised; ensure you reference userMock,
userRepository.findById, editUser, and assertValidationException appropriately.
---
Nitpick comments:
In
`@backend/src/test/java/com/omatheusmesmo/shoppmate/list/controller/ShoppingListControllerTest.java`:
- Around line 87-91: The test creates a requestDTO with a generated name but
still stubs listMapper.toEntity / shoppingListService.saveList /
listMapper.toResponseDTO to return the fixed shoppingList/responseDTO from
setUp(), so the test can pass even if the controller ignores the request body;
change the stubs to be tied to requestDTO (e.g., stub
listMapper.toEntity(requestDTO, any(User.class)) to return an entity whose name
is requestDTO.name(), and stub listMapper.toResponseDTO to return an
expectedResponseDTO built from that entity), then assert against
expectedResponseDTO (or at least assert that the returned response has name ==
requestDTO.name()) to ensure the POST payload is actually used by the
controller.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 08013d93-0574-485f-aceb-7ff8dafe7ac0
📒 Files selected for processing (27)
backend/AGENTS.mdbackend/pom.xmlbackend/src/test/java/com/omatheusmesmo/shoppmate/auth/controller/AuthControllerTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/auth/service/CustomUserDetailsServiceTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/auth/service/JwtServiceTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/category/controller/CategoryControllerTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/category/controller/CategoryControllerWithIntegrationTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/category/service/CategoryServiceTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/item/controller/ItemControllerTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/item/controller/ItemControllerWithIntegrationTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/item/service/ItemServiceTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/list/controller/ListItemControllerTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/list/controller/ListPermissionControllerTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/list/controller/ShoppingListControllerTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/list/service/ListItemServiceTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/list/service/ListPermissionServiceTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/list/service/ShoppingListServiceTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/shared/service/AuditServiceTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/shared/testutils/CategoryTestFactory.javabackend/src/test/java/com/omatheusmesmo/shoppmate/shared/testutils/FakerUtil.javabackend/src/test/java/com/omatheusmesmo/shoppmate/shared/testutils/ItemTestFactory.javabackend/src/test/java/com/omatheusmesmo/shoppmate/shared/testutils/ListTestFactory.javabackend/src/test/java/com/omatheusmesmo/shoppmate/shared/testutils/UnitTestFactory.javabackend/src/test/java/com/omatheusmesmo/shoppmate/shared/testutils/UserTestFactory.javabackend/src/test/java/com/omatheusmesmo/shoppmate/unit/service/UnitServiceTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/user/controller/UserControllerTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/user/service/UserServiceTest.java
✅ Files skipped from review due to trivial changes (10)
- backend/AGENTS.md
- backend/src/test/java/com/omatheusmesmo/shoppmate/shared/testutils/FakerUtil.java
- backend/src/test/java/com/omatheusmesmo/shoppmate/auth/controller/AuthControllerTest.java
- backend/src/test/java/com/omatheusmesmo/shoppmate/shared/testutils/UnitTestFactory.java
- backend/src/test/java/com/omatheusmesmo/shoppmate/shared/testutils/CategoryTestFactory.java
- backend/src/test/java/com/omatheusmesmo/shoppmate/list/service/ShoppingListServiceTest.java
- backend/src/test/java/com/omatheusmesmo/shoppmate/shared/testutils/ItemTestFactory.java
- backend/src/test/java/com/omatheusmesmo/shoppmate/list/controller/ListItemControllerTest.java
- backend/src/test/java/com/omatheusmesmo/shoppmate/shared/service/AuditServiceTest.java
- backend/src/test/java/com/omatheusmesmo/shoppmate/list/service/ListPermissionServiceTest.java
🚧 Files skipped from review as they are similar to previous changes (8)
- backend/src/test/java/com/omatheusmesmo/shoppmate/item/service/ItemServiceTest.java
- backend/src/test/java/com/omatheusmesmo/shoppmate/unit/service/UnitServiceTest.java
- backend/src/test/java/com/omatheusmesmo/shoppmate/item/controller/ItemControllerTest.java
- backend/src/test/java/com/omatheusmesmo/shoppmate/auth/service/JwtServiceTest.java
- backend/src/test/java/com/omatheusmesmo/shoppmate/list/service/ListItemServiceTest.java
- backend/src/test/java/com/omatheusmesmo/shoppmate/shared/testutils/UserTestFactory.java
- backend/src/test/java/com/omatheusmesmo/shoppmate/list/controller/ListPermissionControllerTest.java
- backend/src/test/java/com/omatheusmesmo/shoppmate/category/controller/CategoryControllerWithIntegrationTest.java
Hlib-Rachkovskyy
force-pushed
the
feature/agnostic-category-tests
branch
from
10c2802 to
41cef84
Compare
Hlib-Rachkovskyy
force-pushed
the
feature/agnostic-category-tests
branch
from
41cef84 to
2eee7cc
Compare
matheusandre1
approved these changes
Apr 19, 2026
omatheusmesmo
approved these changes
Apr 20, 2026
Owner
omatheusmesmo
left a comment
omatheusmesmo
left a comment
There was a problem hiding this comment.
LGTM @Hlib-Rachkovskyy , thanks! ☕
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
3 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.
Description
This PR performs a comprehensive refactor of the backend test suite to improve maintainability, reduce hardcoded data dependency, and align the test structure with the project's feature-based organization.
Key Changes
1. Architectural Restructuring
com.omatheusmesmo.shoppmate.servicepackage to their respective feature packages (e.g.,user/service,item/service,category/service).shared.testutilspackage containing domain-specific factories to manage object instantiation:UserTestFactory: Agnostic generation of User entities andRegisterUserDTO.ListTestFactory: HandlesShoppingList,ListItem, andListPermission.ItemTestFactory,CategoryTestFactory, andUnitTestFactory: Standardized data for core domains.FakerUtilto provide a single, consistent instance of Datafaker across the suite.2. Standardized Data Generation
generateValidPassword(), to ensure generated data satisfies strict business constraints (e.g., regex-based password complexity).3. Test Suite Refinement
4. Documentation Updates
ItemTestFactory) with Datafaker for generating dynamic test data.Technical Details
How to Verify
Run the full test suite from the
backenddirectory:mvn testCloses #115
Summary by CodeRabbit
Tests
Chores