ut: login service ut,jsonUtil ut#317
Merged
Merged
Conversation
Walkthrough
ChangesLoginServiceImpl visibility fix and tests
JsonUtils test suite
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
base/src/main/java/com/tinyengine/it/login/service/impl/LoginServiceImpl.java (1)
127-127: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid widening production visibility just to satisfy the test.
validatorPublicKey(...)is still an internal helper offorgotPassword(...). Making it package-private couples the test suite to an implementation detail and expands the service surface in production. Prefer keeping thisprivateand exercising it throughforgotPassword(...), or extract the crypto check into a separate collaborator that can be tested directly.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@base/src/main/java/com/tinyengine/it/login/service/impl/LoginServiceImpl.java` at line 127, The helper validatorPublicKey in LoginServiceImpl should not be made package-private just for test access; keep it private and verify the behavior through forgotPassword instead, or move the crypto verification into a separate collaborator that can be tested directly. Update the implementation so the test suite targets the public forgotPassword flow while preserving the internal helper as an implementation detail.base/src/test/java/com/tinyengine/it/common/utils/JsonUtilsTest.java (2)
26-90: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCollapse the repeated string-encoding cases into one table-driven test.
These methods all exercise the same path with different literals, so they add a lot of noise without adding distinct coverage. A single loop/helper-based test would keep the suite much easier to maintain and debug.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@base/src/test/java/com/tinyengine/it/common/utils/JsonUtilsTest.java` around lines 26 - 90, The JsonUtilsTest class has several nearly identical string-encoding tests that only differ by the literal value, so replace them with a single table-driven test or parameterized-style loop covering all cases. Keep the same assertions around JsonUtils.encode and use the shared helper/test method to verify each input value, so the repeated test methods are removed while preserving coverage.
250-266: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDon’t lock the pretty-print test to one formatter layout.
Checking for line breaks is enough to prove prettification here. Matching exact spacing around
:makes the test brittle to serializer formatting changes that do not change behavior. Prefer parsing the output back and asserting structural equality.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@base/src/test/java/com/tinyengine/it/common/utils/JsonUtilsTest.java` around lines 250 - 266, The JsonUtilsTest.testEncodePrettily_ComplexObject assertion is too tightly coupled to a specific pretty-print spacing layout. Update this test to focus on behavior by keeping the line-break check and replacing the exact string matches for key/value spacing with parsing the output back through the JSON utility or an equivalent JSON parser and asserting the resulting structure matches the original data. Keep the test anchored around JsonUtils.encodePrettily so it remains resilient to formatter changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@base/src/test/java/com/tinyengine/it/common/utils/JsonUtilsTest.java`:
- Around line 120-127: The invalid JSON tests in JsonUtilsTest are asserting
parser-specific exception message text, which is brittle across parser versions.
Update testDecode_InvalidJson and the other affected invalid-JSON cases to
assert only the thrown exception type or JsonUtils-specific wrapping behavior
from JsonUtils.decode, and remove any checks against message contents like
parser token text. Use the existing JsonUtils.decode calls and
assertThrows-based tests to keep the contract stable.
In
`@base/src/test/java/com/tinyengine/it/login/service/impl/LoginServiceImplTest.java`:
- Around line 172-180: The exception tests in LoginServiceImplTest are currently
hitting an unstubbed query path, so createUser() fails on users.isEmpty() before
the empty-username/null-salt branch is exercised. In the test methods around
testCreateUser_EmptyUsername and the matching null-salt case, stub
queryUserByCondition(...) to return Collections.emptyList() so
loginService.createUser(user) reaches the intended validation branch, or add
explicit input validation in LoginServiceImpl.createUser and assert that
behavior instead. Use the createUser() and queryUserByCondition(...) symbols to
update the tests so they verify the named input case rather than Mockito’s
default null.
---
Nitpick comments:
In
`@base/src/main/java/com/tinyengine/it/login/service/impl/LoginServiceImpl.java`:
- Line 127: The helper validatorPublicKey in LoginServiceImpl should not be made
package-private just for test access; keep it private and verify the behavior
through forgotPassword instead, or move the crypto verification into a separate
collaborator that can be tested directly. Update the implementation so the test
suite targets the public forgotPassword flow while preserving the internal
helper as an implementation detail.
In `@base/src/test/java/com/tinyengine/it/common/utils/JsonUtilsTest.java`:
- Around line 26-90: The JsonUtilsTest class has several nearly identical
string-encoding tests that only differ by the literal value, so replace them
with a single table-driven test or parameterized-style loop covering all cases.
Keep the same assertions around JsonUtils.encode and use the shared helper/test
method to verify each input value, so the repeated test methods are removed
while preserving coverage.
- Around line 250-266: The JsonUtilsTest.testEncodePrettily_ComplexObject
assertion is too tightly coupled to a specific pretty-print spacing layout.
Update this test to focus on behavior by keeping the line-break check and
replacing the exact string matches for key/value spacing with parsing the output
back through the JSON utility or an equivalent JSON parser and asserting the
resulting structure matches the original data. Keep the test anchored around
JsonUtils.encodePrettily so it remains resilient to formatter changes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f0dec557-dc12-4fae-a66f-5c4996896abd
📒 Files selected for processing (3)
base/src/main/java/com/tinyengine/it/login/service/impl/LoginServiceImpl.javabase/src/test/java/com/tinyengine/it/common/utils/JsonUtilsTest.javabase/src/test/java/com/tinyengine/it/login/service/impl/LoginServiceImplTest.java
hexqi
approved these changes
Jun 30, 2026
hexqi
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
login service ut,jsonUtil ut
Summary by CodeRabbit
Tests
Refactor