Skip to content

Conversation

piyush-kukadiya
Copy link
Contributor

@piyush-kukadiya piyush-kukadiya commented Sep 18, 2025

Depends on PR #11

Adds unit tests for Vault SDK

Summary by CodeRabbit

  • Tests
    • Added extensive unit tests across the SDK: initialization, single/batch tokenization and detokenization, caching behavior, networking configuration, authentication flows, retry/backoff logic, encryption/decryption (with fallbacks), response processing, and type conversion utilities. Includes broad edge-case and error-path coverage to enhance reliability and stability.
  • Chores
    • Removed an obsolete sample unit test.

Note: This release improves test coverage and robustness without altering public APIs or introducing user-facing changes.

@francispereira
Copy link

🎉 Snyk checks have passed. No issues have been found so far.

code/snyk check is complete. No issues have been found. (View Details)

@darshanclevertap
Copy link

@coderabbitai review

Copy link

coderabbitai bot commented Sep 19, 2025

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link

coderabbitai bot commented Sep 19, 2025

Walkthrough

Removes a sample unit test and adds extensive new test suites across SDK layers: VaultSDK, repositories (single/batch tokenize/detokenize, auth, retry), cache, network provider, encryption strategies, response processing, state modeling, and type conversion utilities. No production code or public API changes.

Changes

Cohort / File(s) Summary
Housekeeping
clevertap-android-vault-sdk/src/test/java/com/clevertap/android/vault/sdk/ExampleUnitTest.kt
Removed sample unit test asserting 2 + 2 equals 4.
SDK Facade Tests
clevertap-android-vault-sdk/src/test/java/com/clevertap/android/vault/sdk/VaultSDKTest.kt
Added comprehensive tests for initialization, singleton behavior, tokenization/detokenization (single/batch), and error handling with coroutine test setup and MockK.
Cache Layer Tests
.../cache/TokenCacheTest.kt, .../repository/CacheManagerTest.kt
Introduced parameterized tests for TokenCache and CacheManager covering enabled/disabled modes, single/batch get/put, bidirectional mappings, edge cases, and logging interactions.
Network Layer Tests
.../network/NetworkProviderTest.kt
Added tests for lazy initialization, configuration (timeouts, base URLs), API creation over varied URLs, and integration ensuring shared OkHttpClient.
Auth and Retry Tests
.../repository/AuthRepositoryImplTest.kt, .../repository/RetryHandlerTest.kt
Added dense suites for auth token lifecycle, time-based validity, error handling, response parsing; and retry logic covering 401 flows, backoff, network/server errors, and edge cases.
Repository Operation Tests
.../repository/SingleTokenizeOperationTest.kt, .../repository/SingleDetokenizeOperationTest.kt, .../repository/BatchTokenizeOperationTest.kt, .../repository/BatchDetokenizeOperationTest.kt
Added extensive tests for cache checks, response processing (regular/encrypted), API delegation, cache updates, validation, and error/result creation for single and batch operations.
Encryption Strategy Tests
.../repository/EncryptionSrategyTest.kt
Added tests for NoEncryption and WithEncryption strategies, multi-level fallbacks (including 419 handling), decryption flow, and factory selection.
Response Processing and State Tests
.../repository/ResponseProcessorTest.kt, .../repository/StatesTest.kt
Added parameterized tests validating tokenize/detokenize and batch response handling, error paths, large inputs, Unicode; and state construction/results for cache-check and batch states.
Conversion Utilities Tests
.../util/ResultConversionUtilsTest.kt, .../util/TypeConverterTest.kt
Added broad tests for converting repo results to public results across types, batch summaries, errors; and registry plus individual converters including edge and invalid cases.

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes

Poem

Thump-thump go my code-reviewing feet,
New tests sprout like clover, dense and neat.
Cache, auth, and nets all vetted through,
Encryption bunnies guard the burrow, too.
With whiskered grit I nibble each line—
Green meadows of passing checks, divine! 🐇✅

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.41% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The title "Add unit tests for SDK code SDK-5216" is concise and accurately describes the primary change (adding unit tests) and includes the related ticket ID; this matches the changeset which predominantly introduces many new test files and contains no unrelated content.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/vault/unit_test/SDK-5216

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.


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: 3

🧹 Nitpick comments (27)
clevertap-android-vault-sdk/src/test/java/com/clevertap/android/vault/sdk/repository/StatesTest.kt (3)

281-309: Confirm summary semantics for fully-cached tokenize results (existingCount/newlyCreatedCount).

You assert existingCount == processedCount and newlyCreatedCount == 0 even when some cached items have newlyCreated = true. If “complete from cache” is intended to force existingCount = processedCount regardless of flags, keep as-is; otherwise, adjust assertions or test data to reflect actual flag handling.


456-480: Potential mismatch: detokenize “found” summary ignores item flags.

Here foundCount is asserted as 4 while two cached items have exists = false. Verify whether createCompleteResult() for a complete-from-cache state should count all as found. If not, fix expectations or sample data.


482-505: Null-value handling in detokenize summary.

You assert foundCount == processedCount even when exists = false and/or value == null is present. Please confirm the intended invariant for complete-from-cache summaries; if summaries should reflect actual item flags, update the assertions.

clevertap-android-vault-sdk/src/test/java/com/clevertap/android/vault/sdk/repository/CacheManagerTest.kt (4)

127-128: Avoid brittle log message matching in verifications.

Verifying exact log strings is fragile. Prefer verifying a log call occurred without asserting the literal message.

-            if (result != null) {
-                verify { mockLogger.d("Token found in cache") }
-            }
+            if (result != null) {
+                verify { mockLogger.d(any()) }
+            }

241-243: Same for value retrieval log verification.

Reduce coupling to log text.

-            if (result != null) {
-                verify { mockLogger.d("Value found in cache") }
-            }
+            if (result != null) {
+                verify { mockLogger.d(any()) }
+            }

971-974: Use matchers in “not-called” verification to avoid item.value ?: any() pitfalls.

When value is null, item.value ?: any() mixes a runtime value with a matcher via Elvis. Use matchers consistently.

-                    verify(exactly = 0) {
-                        mockTokenCache.putValue(item.token, item.value ?: any(), item.dataType)
-                    }
+                    verify(exactly = 0) {
+                        mockTokenCache.putValue(eq(item.token), any(), eq(item.dataType))
+                    }

852-869: Redundant global count + per-item verifications.

You both verify the total number of calls and per-item expectations. Consider keeping per‑item checks and dropping the global expectedStoreCalls assertion to reduce brittleness, or vice‑versa.

clevertap-android-vault-sdk/src/test/java/com/clevertap/android/vault/sdk/cache/TokenCacheTest.kt (2)

152-166: Also cover dataType defaulting on value-side storage.

You validate defaulting to "string" via putToken/getToken. Mirror a small test for putValue/getValue to ensure symmetry.

Happy to draft the extra test case if you want it in this PR.


325-334: Empty-key behavior is intentionally permissive—document intent.

Since empty keys are accepted without assertions, consider a brief comment noting that empty keys are allowed by design to avoid future “tightening” breaking tests.

clevertap-android-vault-sdk/src/test/java/com/clevertap/android/vault/sdk/VaultSDKTest.kt (3)

485-491: Strengthen null-detokenize assertions across all types.

This test only asserts for String. For other types, no type-specific assertion runs, weakening the check. Either parametrize expectations per type (e.g., numeric -> null success, boolean -> error or null, per design) or split into dedicated tests.


44-61: Unify coroutine test scheduler to avoid mixing runTest with UnconfinedTestDispatcher.

Mixing UnconfinedTestDispatcher, TestScope, setMain, and runTest can mask scheduling issues. Prefer a single StandardTestDispatcher shared between SDK scope and runTest.

-@OptIn(ExperimentalCoroutinesApi::class)
-class VaultSDKInitializationTest {
-    private val testDispatcher: TestDispatcher = UnconfinedTestDispatcher()
+@OptIn(ExperimentalCoroutinesApi::class)
+class VaultSDKInitializationTest {
+    private val testDispatcher = kotlinx.coroutines.test.StandardTestDispatcher()
...
-    @Before
-    fun setUp() {
-        Dispatchers.setMain(testDispatcher)
+    @Before
+    fun setUp() {
+        Dispatchers.setMain(testDispatcher)
         // Reset singleton instance before each test
         resetVaultSDKInstance()
     }

And in parameterized suites where you call runTest, pass the same dispatcher:

-@Test
-fun shouldTokenizeValueCorrectly() = runTest {
+@Test
+fun shouldTokenizeValueCorrectly() = runTest(testDispatcher) {

Also set sdk.sdkScope = TestScope(testDispatcher) so both use the same scheduler. Apply similarly across classes in this file.


127-135: DRY: duplicate resetVaultSDKInstance() helpers in multiple classes.

Extract a single top‑level private helper in this file (or a small test util) and reuse to reduce duplication.

Also applies to: 273-281, 495-503, 737-745, 1376-1385, 1701-1710, 2234-2243

clevertap-android-vault-sdk/src/test/java/com/clevertap/android/vault/sdk/repository/BatchTokenizeOperationTest.kt (3)

122-127: Should “no values cached” be NothingFromCache instead of PartialFromCache?

For the scenario with an empty cache hit set, the expected state is set to "PartialFromCache". Verify the implementation of checkCacheWithState—semantically this case often maps to "NothingFromCache".


528-546: Prefer non-deprecated ResponseBody creation in tests.

If using OkHttp 4.x, ResponseBody.create(null, ...) is deprecated. Use ResponseBody.create("Server Error".toByteArray()) or ResponseBody.create(MediaType.parse("text/plain"), "Server Error") depending on your OkHttp version.


488-505: Strategy mismatch test: assert exact error contract.

You check the error message equals "Encryption strategy mismatch". If this message is not part of the public contract, consider asserting it contains a stable substring to reduce brittleness.

clevertap-android-vault-sdk/src/test/java/com/clevertap/android/vault/sdk/repository/ResponseProcessorTest.kt (2)

110-110: Fix misleading test description.

Description says “null error body” but an empty string is passed. Rename to avoid confusion.

-                arrayOf(422, "", "422 with null error body")
+                arrayOf(422, "", "422 with empty error body")

286-289: Correct copy-paste in assertion message.

This is a detokenize test; update the message accordingly.

-            "Error message should indicate tokenization failure",
-            errorResult.message.contains("Detokenization failed")
+            "Error message should indicate detokenization failure",
+            errorResult.message.contains("Detokenization failed")
clevertap-android-vault-sdk/src/test/java/com/clevertap/android/vault/sdk/repository/BatchDetokenizeOperationTest.kt (1)

529-531: Use modern toResponseBody API instead of deprecated ResponseBody.create.

OkHttp 4 deprecates ResponseBody.create; prefer extension API with explicit media type.

-        val errorBody = ResponseBody.create(null, "Tokens not found")
-        val response = Response.error<BatchDetokenizeResponse>(404, errorBody)
+        val errorBody = "Tokens not found".toResponseBody("text/plain".toMediaTypeOrNull())
+        val response = Response.error<BatchDetokenizeResponse>(404, errorBody)
clevertap-android-vault-sdk/src/test/java/com/clevertap/android/vault/sdk/repository/AuthRepositoryImplTest.kt (2)

134-145: Time control via spyk on getNowInMillis may be brittle. Consider injecting a clock.

If getNowInMillis isn’t open, these tests will fail or require making production code open. Prefer constructor-injected Clock/TimeSource to avoid spying.

Would you like a small patch to introduce a Clock interface and wire it into AuthRepositoryImpl?


608-611: Use toResponseBody instead of deprecated ResponseBody.create.

Modernize to avoid deprecation warnings and keep consistency with other tests.

-        val errorResponse = Response.error<AuthTokenResponse>(
-            400,
-            ResponseBody.create(null, "")
-        )
+        val errorResponse = Response.error<AuthTokenResponse>(
+            400,
+            "".toResponseBody("text/plain".toMediaTypeOrNull())
+        )
clevertap-android-vault-sdk/src/test/java/com/clevertap/android/vault/sdk/util/TypeConverterTest.kt (1)

391-396: Add delta when asserting floats.

Assert.equals(Object) works but is brittle for floats. Use the float overload with a delta.

-        assertEquals(123.45f, converter.fromString("123.45"))
-        assertEquals(0.0f, converter.fromString("0.0"))
-        assertEquals(-456.789f, converter.fromString("-456.789"))
+        assertEquals(123.45f, converter.fromString("123.45"), 0.0001f)
+        assertEquals(0.0f, converter.fromString("0.0"), 0.0001f)
+        assertEquals(-456.789f, converter.fromString("-456.789"), 0.0001f)
clevertap-android-vault-sdk/src/test/java/com/clevertap/android/vault/sdk/repository/RetryHandlerTest.kt (1)

450-456: Don’t swallow the exception; assert its type/message.

This addresses detekt’s SwallowedException warning and makes the test stronger.

-        // Act & Assert
-        try {
-            retryHandler.executeWithRetry(apiCall)
-            fail("Expected exception was not thrown")
-        } catch (e: Exception) {
-            // Expected
-        }
+        // Act & Assert
+        try {
+            retryHandler.executeWithRetry(apiCall)
+            fail("Expected IOException but none was thrown")
+        } catch (e: IOException) {
+            assertEquals("Network connection failed", e.message)
+        }
clevertap-android-vault-sdk/src/test/java/com/clevertap/android/vault/sdk/repository/SingleDetokenizeOperationTest.kt (1)

267-293: Consider testing for specific error codes in null value scenarios.

When a token doesn't exist (exists = false with value = null), the test correctly validates the response. However, in production environments, you might want to distinguish between "token not found" vs. "token exists but has null value" for better debugging.

Consider adding a test case that validates error messages or codes when tokens are not found:

 @Test
 fun shouldProcessDetokenizeResponseWithNullValue() {
     // Arrange
     val detokenizeResponse = DetokenizeResponse(
         value = null,
         exists = false,
         dataType = null
     )
     val response = Response.success(detokenizeResponse)
     val cacheResult = CacheCheckResult.NothingFromCache<String, DetokenizeRepoResult>(
         originalRequest = testToken,
         uncachedRequest = testToken
     )
     val expectedResult = DetokenizeRepoResult.Success(null, false, null)

     every { mockResponseProcessor.processDetokenizeResponse(response) } returns expectedResult

     // Act
     val result = operation.processResponseWithCacheData(response, cacheResult)

     // Assert
     assertTrue("Should return success result", result is DetokenizeRepoResult.Success)
     val successResult = result as DetokenizeRepoResult.Success
     assertEquals("Value should be null", null, successResult.value)
     assertFalse("Should not exist", successResult.exists)
     assertEquals("Data type should be null", null, successResult.dataType)
+    // Consider adding semantic validation for "not found" scenarios

     verify(exactly = 1) { mockResponseProcessor.processDetokenizeResponse(response) }
 }
clevertap-android-vault-sdk/src/test/java/com/clevertap/android/vault/sdk/network/NetworkProviderTest.kt (1)

95-103: Consider adding tests for network error scenarios.

While the timeout configuration is properly tested, consider adding tests for network failure scenarios such as connection timeouts, read timeouts, and network unavailability to ensure the SDK handles these gracefully.

Would you like me to generate test cases for network error scenarios including timeout handling, retry behavior, and error propagation?

clevertap-android-vault-sdk/src/test/java/com/clevertap/android/vault/sdk/repository/EncryptionSrategyTest.kt (1)

1-1: Fix typo in filename: EncryptionSrategyTest.kt should be EncryptionStrategyTest.kt.

The filename has a typo - "Srategy" should be "Strategy". This could cause confusion and make the file harder to find when searching.

clevertap-android-vault-sdk/src/test/java/com/clevertap/android/vault/sdk/repository/SingleTokenizeOperationTest.kt (1)

526-576: Test class placement inconsistency detected.

The SingleTokenizeOperationErrorResultTest class starts at line 527, immediately after the closing brace of SingleTokenizeOperationCacheUpdateTest without any blank line separation. This is inconsistent with the spacing between other test classes in the file.

Add a blank line for consistency:

 }
+
 @RunWith(Parameterized::class)
 class SingleTokenizeOperationErrorResultTest(
clevertap-android-vault-sdk/src/test/java/com/clevertap/android/vault/sdk/util/ResultConversionUtilsTest.kt (1)

416-432: Consider testing edge cases for boolean conversion.

The boolean converter tests cover standard true/false cases well. Consider adding tests for edge cases that might occur in production data.

Consider adding these edge cases:

arrayOf("t", null, false, "single char t"),
arrayOf("f", null, false, "single char f"),
arrayOf(" true ", null, false, "whitespace around true"),
arrayOf("TRUE ", null, false, "trailing whitespace"),
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 71cdc89 and 069a44e.

📒 Files selected for processing (16)
  • clevertap-android-vault-sdk/src/test/java/com/clevertap/android/vault/sdk/ExampleUnitTest.kt (0 hunks)
  • clevertap-android-vault-sdk/src/test/java/com/clevertap/android/vault/sdk/VaultSDKTest.kt (1 hunks)
  • clevertap-android-vault-sdk/src/test/java/com/clevertap/android/vault/sdk/cache/TokenCacheTest.kt (1 hunks)
  • clevertap-android-vault-sdk/src/test/java/com/clevertap/android/vault/sdk/network/NetworkProviderTest.kt (1 hunks)
  • clevertap-android-vault-sdk/src/test/java/com/clevertap/android/vault/sdk/repository/AuthRepositoryImplTest.kt (1 hunks)
  • clevertap-android-vault-sdk/src/test/java/com/clevertap/android/vault/sdk/repository/BatchDetokenizeOperationTest.kt (1 hunks)
  • clevertap-android-vault-sdk/src/test/java/com/clevertap/android/vault/sdk/repository/BatchTokenizeOperationTest.kt (1 hunks)
  • clevertap-android-vault-sdk/src/test/java/com/clevertap/android/vault/sdk/repository/CacheManagerTest.kt (1 hunks)
  • clevertap-android-vault-sdk/src/test/java/com/clevertap/android/vault/sdk/repository/EncryptionSrategyTest.kt (1 hunks)
  • clevertap-android-vault-sdk/src/test/java/com/clevertap/android/vault/sdk/repository/ResponseProcessorTest.kt (1 hunks)
  • clevertap-android-vault-sdk/src/test/java/com/clevertap/android/vault/sdk/repository/RetryHandlerTest.kt (1 hunks)
  • clevertap-android-vault-sdk/src/test/java/com/clevertap/android/vault/sdk/repository/SingleDetokenizeOperationTest.kt (1 hunks)
  • clevertap-android-vault-sdk/src/test/java/com/clevertap/android/vault/sdk/repository/SingleTokenizeOperationTest.kt (1 hunks)
  • clevertap-android-vault-sdk/src/test/java/com/clevertap/android/vault/sdk/repository/StatesTest.kt (1 hunks)
  • clevertap-android-vault-sdk/src/test/java/com/clevertap/android/vault/sdk/util/ResultConversionUtilsTest.kt (1 hunks)
  • clevertap-android-vault-sdk/src/test/java/com/clevertap/android/vault/sdk/util/TypeConverterTest.kt (1 hunks)
💤 Files with no reviewable changes (1)
  • clevertap-android-vault-sdk/src/test/java/com/clevertap/android/vault/sdk/ExampleUnitTest.kt
🧰 Additional context used
🪛 detekt (1.23.8)
clevertap-android-vault-sdk/src/test/java/com/clevertap/android/vault/sdk/repository/RetryHandlerTest.kt

[warning] 453-453: The caught exception is swallowed. The original exception could be lost.

(detekt.exceptions.SwallowedException)

🔇 Additional comments (9)
clevertap-android-vault-sdk/src/test/java/com/clevertap/android/vault/sdk/repository/BatchTokenizeOperationTest.kt (2)

719-742: Good delegation verification.

Clear assertion that operation delegates to strategy with the API and token once, and network API is fetched once.


767-789: Helper factory keeps tests clean.

Centralizing construction with overridable dependencies is solid.

clevertap-android-vault-sdk/src/test/java/com/clevertap/android/vault/sdk/repository/SingleDetokenizeOperationTest.kt (1)

1-706: Good test coverage structure and organization!

The file is well-organized into focused test classes covering cache checks, response processing, API delegation, cache updates, and error handling. The use of parameterized tests enhances coverage with diverse input scenarios.

clevertap-android-vault-sdk/src/test/java/com/clevertap/android/vault/sdk/network/NetworkProviderTest.kt (1)

1-267: Excellent network layer test coverage!

The tests thoroughly validate lazy initialization, configuration, and the network stack setup. The timeout configurations are properly tested at 15 seconds for all timeout types.

clevertap-android-vault-sdk/src/test/java/com/clevertap/android/vault/sdk/repository/EncryptionSrategyTest.kt (2)

1-1159: Comprehensive encryption strategy testing!

Excellent coverage of encryption scenarios including fallback mechanisms, 419 error handling, and various failure modes. The parameterized tests effectively cover multiple operation types.


786-853: Good handling of 419 errors and non-fallback scenarios.

The test correctly validates that 419 errors trigger fallback to non-encrypted operations, while other HTTP errors (like 500) are propagated without fallback. This is crucial for maintaining proper error semantics.

clevertap-android-vault-sdk/src/test/java/com/clevertap/android/vault/sdk/repository/SingleTokenizeOperationTest.kt (1)

1-576: Well-structured test suite with comprehensive coverage!

The tests effectively cover cache operations, API delegation, response processing, and error handling for single tokenization operations. The use of parameterized tests with diverse input scenarios including Unicode characters and edge cases is commendable.

clevertap-android-vault-sdk/src/test/java/com/clevertap/android/vault/sdk/util/ResultConversionUtilsTest.kt (2)

1-732: Thorough conversion test coverage across all data types!

Excellent test coverage for type conversions including edge cases like NaN, Infinity, null values, and invalid inputs. The parameterized tests effectively validate conversion behavior across all supported types.


284-286: Good handling of special float/double values (NaN, Infinity).

The tests properly handle special floating-point values including positive/negative infinity and NaN, with appropriate assertions that account for NaN's special equality semantics.

Also applies to: 318-320, 359-361

Comment on lines +4 to +19
import com.clevertap.android.vault.sdk.api.TokenizationApi
import com.clevertap.android.vault.sdk.cache.TokenCache
import com.clevertap.android.vault.sdk.encryption.EncryptionManager
import com.clevertap.android.vault.sdk.model.BatchDetokenItemResponse
import com.clevertap.android.vault.sdk.model.BatchDetokenizeRepoResult
import com.clevertap.android.vault.sdk.model.BatchDetokenizeResponse
import com.clevertap.android.vault.sdk.model.BatchDetokenizeSummary
import com.clevertap.android.vault.sdk.model.EncryptedResponse
import com.clevertap.android.vault.sdk.network.NetworkProvider
import com.clevertap.android.vault.sdk.util.VaultLogger
import io.mockk.coEvery
import io.mockk.coVerify
import io.mockk.every
import io.mockk.mockk
import io.mockk.verify
import kotlinx.coroutines.test.runTest
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Missing imports for encryption strategy types will break compilation.

Add explicit imports for the strategy interfaces used below.

 import com.clevertap.android.vault.sdk.encryption.EncryptionManager
+import com.clevertap.android.vault.sdk.encryption.EncryptionStrategy
+import com.clevertap.android.vault.sdk.encryption.WithEncryptionStrategy
+import com.clevertap.android.vault.sdk.encryption.NoEncryptionStrategy
📝 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
import com.clevertap.android.vault.sdk.api.TokenizationApi
import com.clevertap.android.vault.sdk.cache.TokenCache
import com.clevertap.android.vault.sdk.encryption.EncryptionManager
import com.clevertap.android.vault.sdk.model.BatchDetokenItemResponse
import com.clevertap.android.vault.sdk.model.BatchDetokenizeRepoResult
import com.clevertap.android.vault.sdk.model.BatchDetokenizeResponse
import com.clevertap.android.vault.sdk.model.BatchDetokenizeSummary
import com.clevertap.android.vault.sdk.model.EncryptedResponse
import com.clevertap.android.vault.sdk.network.NetworkProvider
import com.clevertap.android.vault.sdk.util.VaultLogger
import io.mockk.coEvery
import io.mockk.coVerify
import io.mockk.every
import io.mockk.mockk
import io.mockk.verify
import kotlinx.coroutines.test.runTest
import com.clevertap.android.vault.sdk.api.TokenizationApi
import com.clevertap.android.vault.sdk.cache.TokenCache
import com.clevertap.android.vault.sdk.encryption.EncryptionManager
import com.clevertap.android.vault.sdk.encryption.EncryptionStrategy
import com.clevertap.android.vault.sdk.encryption.WithEncryptionStrategy
import com.clevertap.android.vault.sdk.encryption.NoEncryptionStrategy
import com.clevertap.android.vault.sdk.model.BatchDetokenItemResponse
import com.clevertap.android.vault.sdk.model.BatchDetokenizeRepoResult
import com.clevertap.android.vault.sdk.model.BatchDetokenizeResponse
import com.clevertap.android.vault.sdk.model.BatchDetokenizeSummary
import com.clevertap.android.vault.sdk.model.EncryptedResponse
import com.clevertap.android.vault.sdk.network.NetworkProvider
import com.clevertap.android.vault.sdk.util.VaultLogger
import io.mockk.coEvery
import io.mockk.coVerify
import io.mockk.every
import io.mockk.mockk
import io.mockk.verify
import kotlinx.coroutines.test.runTest
🤖 Prompt for AI Agents
In
clevertap-android-vault-sdk/src/test/java/com/clevertap/android/vault/sdk/repository/BatchDetokenizeOperationTest.kt
around lines 4 to 19, the file is missing imports for the encryption strategy
interfaces referenced later in the test, which will break compilation; add
explicit imports for the strategy types used (for example
com.clevertap.android.vault.sdk.encryption.EncryptionStrategy and
com.clevertap.android.vault.sdk.encryption.DecryptionStrategy or the exact
strategy interface class names present in the production code) so the test
compiles and the mocked strategy types resolve.

Comment on lines +62 to +63
val expectedResponse = Response.success(httpStatus, responseBody)
val apiCall: suspend () -> Response<String> = mockk()
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Incorrect Retrofit Response.success overload.

Response.success(httpStatus, body) doesn’t exist. Build a raw OkHttp response to set status, or just use Response.success(body). Here’s a safe helper approach.

-        val expectedResponse = Response.success(httpStatus, responseBody)
+        val expectedResponse = successResponse(httpStatus, if (httpStatus == 204) null else responseBody)

Add this helper inside the test class:

@@
     fun setUp() {
         mockAuthRepository = mockk(relaxed = true)
         mockLogger = mockk(relaxed = true)
         retryHandler = RetryHandler(mockAuthRepository, mockLogger, maxRetries = 2)
     }
+
+    private fun successResponse(code: Int, body: String?): Response<String> {
+        val raw = okhttp3.Response.Builder()
+            .request(okhttp3.Request.Builder().url("https://example.test/").build())
+            .protocol(okhttp3.Protocol.HTTP_1_1)
+            .code(code)
+            .message("OK")
+            .build()
+        return if (code == 204) Response.success(null, raw) else Response.success(body, raw)
+    }

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In
clevertap-android-vault-sdk/src/test/java/com/clevertap/android/vault/sdk/repository/RetryHandlerTest.kt
around lines 62-63, the call Response.success(httpStatus, responseBody) is
invalid; replace it by either using Response.success(responseBody) or by adding
a small helper in the test class that builds an OkHttp raw Response with the
desired status code and returns Retrofit's Response.success(body, raw). Add that
helper method to construct an okhttp3.Response (set protocol, request, code,
message, and body), then call Response.success(responseBody, raw) where the test
previously used the bad overload.

Comment on lines +4 to +12
import BooleanConverter
import DoubleConverter
import FloatConverter
import IntConverter
import LongConverter
import StringConverter
import TypeConverter
import TypeConverterRegistry
import org.junit.Assert.assertEquals
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Invalid/unnamespaced imports will not compile.

These classes are in the same package; drop the imports or fully qualify them. Prefer removing them.

-import BooleanConverter
-import DoubleConverter
-import FloatConverter
-import IntConverter
-import LongConverter
-import StringConverter
-import TypeConverter
-import TypeConverterRegistry
📝 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
import BooleanConverter
import DoubleConverter
import FloatConverter
import IntConverter
import LongConverter
import StringConverter
import TypeConverter
import TypeConverterRegistry
import org.junit.Assert.assertEquals
🤖 Prompt for AI Agents
In
clevertap-android-vault-sdk/src/test/java/com/clevertap/android/vault/sdk/util/TypeConverterTest.kt
around lines 4 to 12, there are unnamespaced imports (BooleanConverter,
DoubleConverter, FloatConverter, IntConverter, LongConverter, StringConverter,
TypeConverter, TypeConverterRegistry) that will not compile because they lack a
package; since these classes are in the same package, remove these import lines
(or alternatively replace them with fully-qualified package imports) so the file
compiles; ensure the file has the correct package declaration at the top and
then delete the listed import statements.

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.

4 participants