Skip to content

Conversation

@srikanth716
Copy link
Contributor

@srikanth716 srikanth716 commented Jan 22, 2026

Summary by CodeRabbit

  • Bug Fixes

    • Safer cleanup during binary decoding with guarded deletion and a warning if removal fails.
  • Tests

    • Large expansion of test coverage for QR generation/decoding, data mapping, conversions, and integrations.
    • Introduced a cross-platform test annotation to skip tests on Android.
    • Minor test message/formatting tweaks and new JVM/common test suites for mapping and utility conversions.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Jan 22, 2026

Walkthrough

Adds multiplatform test annotation support, large new JVM/common test suites for QR/CBOR/JSON mapping and utilities, a small Android unit-test message tweak, and a guarded temp-file deletion with error logging in PixelPass.decodeBinary.

Changes

Cohort / File(s) Summary
Test annotation implementations
kotlin/PixelPass/src/commonTest/kotlin/io/mosip/pixelpass/IgnoreOnAndroid.kt, kotlin/PixelPass/src/jvmTest/kotlin/io/mosip/pixelpass/IgnoreOnAndroid.kt, kotlin/PixelPass/src/androidUnitTest/kotlin/io/mosip/pixelpass/IgnoreOnAndroid.kt
Add multiplatform test annotation: common expect annotation class IgnoreOnAndroid, JVM actual annotation class IgnoreOnAndroid, and Android typealias to org.junit.Ignore.
Core library file deletion change
kotlin/PixelPass/src/commonMain/kotlin/io/mosip/pixelpass/PixelPass.kt
In decodeBinary, guard temp-file deletion with a null check and log a warning if deletion fails (avoid unconditional delete).
Android unit test tweak
kotlin/PixelPass/src/androidUnitTest/kotlin/io/mosip/pixelpass/QrDataConvertorAndroidTest.kt
Adjusted test assertion message/formatting around exception case; no behavior or control-flow change in test logic.
Expanded common test suite
kotlin/PixelPass/src/commonTest/kotlin/io/mosip/pixelpass/PixelPassGenerateQRCodeTest.kt, kotlin/PixelPass/src/commonTest/kotlin/io/mosip/pixelpass/PixelPassTest.kt
Add @IgnoreOnAndroid to generate-QRCODE tests and replace/expand PixelPassTest with extensive QR generation/decoding, mapping, CBOR/JSON, headers, ECC variants, edge cases, and integration scenarios (large addition of tests).
JVM utility tests
kotlin/PixelPass/src/jvmTest/kotlin/io/mosip/pixelpass/MapperUtilTest.kt, kotlin/PixelPass/src/jvmTest/kotlin/io/mosip/pixelpass/UtilsTest.kt
New comprehensive JVM tests covering Mapper utilities and bidirectional JSON↔CBOR conversions, nested structures, key/value remapping, and edge cases.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested reviewers

  • swatigoel

Poem

🐰 I nibble bugs and hop through lanes,
With CBOR crumbs and QR-code grains,
Tests stretch wide where carrots hide,
Annotations snug on every side,
Temp files cleared with cautious pride.

🚥 Pre-merge checks | ✅ 1 | ❌ 2
❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 21.70% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title '[INJIVER-1534] fix: sonar reliability and coverage' is vague and does not clearly convey what specific changes were made. 'Sonar reliability and coverage' is too generic and doesn't describe the actual modifications like annotation alias creation, test additions, or file deletion handling. Provide a more specific title that describes the main changes, such as 'Add cross-platform IgnoreOnAndroid annotation and expand test coverage' or 'Improve QR code error handling and add comprehensive test suite'.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

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

✨ Finishing touches
  • 📝 Generate docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Fix all issues with AI agents
In
`@kotlin/PixelPass/src/androidUnitTest/kotlin/io/mosip/pixelpass/QrDataConvertorAndroidTest.kt`:
- Around line 55-58: The test expects convertQRDataIntoBase64 to let the
RuntimeException from QrCode.encodeText propagate, but the implementation in
QrDataConvertorAndroid.kt currently swallows all exceptions in the catch (e:
Exception) block and returns ""; change the implementation in
convertQRDataIntoBase64 to stop suppressing errors — either remove the broad
try/catch so exceptions bubble up, or rethrow the caught exception (e.g., throw
e) instead of returning an empty string; keep logging (logger.severe) if desired
but ensure the exception is propagated to satisfy
assertFailsWith<RuntimeException>.

In `@kotlin/PixelPass/src/commonTest/kotlin/io/mosip/pixelpass/PixelPassTest.kt`:
- Around line 726-745: The test testComplexMappingRoundTrip prepares a
decodeKeyMapper but never uses it; either rename the test to reflect it only
encodes (e.g., testComplexMappingEncode) or complete the round-trip by calling
the decoding method (use pixelPass.getMappedData or the appropriate decode
function with the encoded map and decodeKeyMapper) and add assertions that the
decoded JSONObject contains "userStatus"="active" and "userName"="TestUser".
Ensure you reference the existing local variables encodeKeyMapper,
encodeValueMapper, decodeKeyMapper and the encoded result when adding the decode
call and assertions.
🧹 Nitpick comments (3)
kotlin/PixelPass/src/jvmTest/kotlin/io/mosip/pixelpass/UtilsTest.kt (1)

292-343: Tighten Claim169 tests with deterministic expectations.

A few tests only assert non-null or rely on implicit mapper configuration, which can pass even if mappings are incorrect. Consider asserting specific transformed values using known mapper entries or a test-local mapper to improve reliability.

kotlin/PixelPass/src/commonTest/kotlin/io/mosip/pixelpass/PixelPassTest.kt (2)

277-279: Consider simplifying redundant type cast.

After the is String check on line 278, the explicit cast on line 279 is safe but could be simplified. A similar pattern appears at lines 344-345.

♻️ Suggested simplification
         assertNotNull(result)
-        assertTrue(result is String)
-        assertTrue((result as String).isNotEmpty())
+        assertTrue(result is String && result.isNotEmpty())

Or using smart cast:

assertNotNull(result)
assertTrue(result is String)
assertTrue(result.isNotEmpty()) // Smart cast applies after type check

663-672: Consider adding positive test cases for decodeBinary.

Currently only the error path is tested. If valid binary decoding is a core feature, consider adding test cases with valid image byte arrays (e.g., minimal valid PNG or JPEG headers) to verify successful decoding behavior.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@kotlin/PixelPass/src/jvmTest/kotlin/io/mosip/pixelpass/IgnoreOnAndroid.kt`:
- Line 3: The JVM actual annotation class IgnoreOnAndroid is missing the `@Target`
and `@Retention` metadata present on the expect declaration; update the actual
declaration of IgnoreOnAndroid to include the exact same `@Target`(...) and
`@Retention`(...) annotations as the expect declaration (copy the AnnotationTarget
entries and AnnotationRetention value), and add the necessary imports
(kotlin.annotation.Target, kotlin.annotation.Retention,
kotlin.annotation.AnnotationTarget, kotlin.annotation.AnnotationRetention) so
the JVM annotation metadata matches the expect metadata exactly.
♻️ Duplicate comments (1)
kotlin/PixelPass/src/commonTest/kotlin/io/mosip/pixelpass/PixelPassTest.kt (1)

759-767: Rename test or add a decode step.

The test name says “GenerateQRCodeAndDecode” but only generates and asserts non-empty output. Either rename to match intent or add a decode step if the API supports it.

✅ Suggested rename
-    fun testGenerateQRCodeAndDecodeIntegration() {
+    fun testGenerateQRCodeIntegration() {

@sonarqubecloud
Copy link

@mayuradesh mayuradesh merged commit fb96871 into inji:develop Jan 28, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants