fix: implement BasicAuthHandler#getAuthenticationInfo in tests#26895
fix: implement BasicAuthHandler#getAuthenticationInfo in tests#26895Mihix wants to merge 1 commit intowso2:masterfrom
Conversation
Avoid returning null AuthenticationInfo in test utilities by creating a BasicAuthInfo populated with default credentials and a Basic auth header. Made-with: Cursor
|
|
WalkthroughTwo test utility classes modified to implement Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ 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 |
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/utils/BasicAuthHandler.java (1)
27-27: Redundant import from the same package.
BasicAuthInfois in the same package (org.wso2.identity.integration.test.utils) as this class, so the explicit import is unnecessary. Java automatically resolves classes within the same package.♻️ Proposed fix
-import org.wso2.identity.integration.test.utils.BasicAuthInfo;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/utils/BasicAuthHandler.java` at line 27, Remove the redundant import of BasicAuthInfo from BasicAuthHandler: since BasicAuthInfo is in the same package org.wso2.identity.integration.test.utils, delete the import statement referencing BasicAuthInfo and rely on the package-local resolution; update the import list in BasicAuthHandler to eliminate that unused line and run a quick build to ensure no remaining import warnings.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/utils/BasicAuthHandler.java`:
- Line 27: Remove the redundant import of BasicAuthInfo from BasicAuthHandler:
since BasicAuthInfo is in the same package
org.wso2.identity.integration.test.utils, delete the import statement
referencing BasicAuthInfo and rely on the package-local resolution; update the
import list in BasicAuthHandler to eliminate that unused line and run a quick
build to ensure no remaining import warnings.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3ca7b258-1db9-4778-99e2-44c9aa696dd4
📒 Files selected for processing (2)
modules/integration/tests-integration/tests-backend/src/test/java/org/wso2/identity/integration/test/utils/BasicAuthHandler.javamodules/integration/tests-ui-integration/src/test/java/org/wso2/identity/ui/integration/test/utils/BasicAuthHandler.java



I noticed that getAuthenticationInfo() was essentially a dead end—it always returned null, even though the class had all the credentials it needed. This was a bit of a landmine for anyone calling the method, as it could lead to unexpected crashes or authentication silent-fails. I’ve updated it to actually 'speak up' and return a fully formed BasicAuthInfo object. Now, the method does exactly what it says on the tin, making it much safer for the rest of the team to use.