[Test Improver] Add unit tests for connectId response parsers (LinkHqWorker and RetrieveChannelEncryptionKey)#3626
Conversation
…yptionKeyResponseParser Tests cover: - LinkHqWorkerResponseParser: workerLinked flag mutation, return value, storeApp call - RetrieveChannelEncryptionKeyResponseParser: key parsing, empty response guard, storeMessagingChannel call, invalid JSON and missing key error paths Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
| @Test | ||
| fun testParse_setsWorkerLinkedTrue_andReturnsTrue() { | ||
| // Arrange | ||
| val appRecord = ConnectLinkedAppRecord() |
There was a problem hiding this comment.
/test-assist Create a class level member declaration something like
private lateinit var appRecord: ConnectLinkedAppRecord, intialize it in setUp() and use it in each test.
|
|
||
| @Before | ||
| fun setUp() { | ||
| val context = ApplicationProvider.getApplicationContext<android.content.Context>() |
There was a problem hiding this comment.
/test-assist Create a class-level member declaration something like
private lateinit var context: android.content.Context, initialise in setUp() and use it in each test.
|
|
||
| @Before | ||
| fun setUp() { | ||
| val context = ApplicationProvider.getApplicationContext<android.content.Context>() |
There was a problem hiding this comment.
/test-assist Simply initialise it by context = ApplicationProvider.getApplicationContext()
| val context = ApplicationProvider.getApplicationContext<android.content.Context>() | ||
| val channel = ConnectMessagingChannelRecord() |
There was a problem hiding this comment.
/test-assist Create a class-level member declaration of context and channel variable, initialise in setUp() and use it in each test.
|
|
||
| @Config(application = CommCareTestApplication::class) | ||
| @RunWith(AndroidJUnit4::class) | ||
| class LinkHqWorkerResponseParserTest { |
There was a problem hiding this comment.
/test-assist Pass null to anyInputObject and add another test with @Test(expected = NullPointerException::class)
|
|
||
| @Config(application = CommCareTestApplication::class) | ||
| @RunWith(AndroidJUnit4::class) | ||
| class RetrieveChannelEncryptionKeyResponseParserTest { |
There was a problem hiding this comment.
/test-assist Pass null to anyInputObject and add another test with @Test(expected = NullPointerException::class)
There was a problem hiding this comment.
/test-assist Pass null to anyInputObject and add another test with @test(expected = NullPointerException::class)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Commit pushed:
|
…onnectId parser tests - Promote context and appRecord/channel to class-level members initialized in setUp() to avoid repetition across tests - Import Context directly instead of using fully qualified name - Replace assertTrue(!x) with assertFalse(x) for clarity 🤖 PR Comment Handler
|
Commit pushed:
|
|
🤖 PR Comment Handler here — I've processed the latest review feedback. Implemented:
Already present (no change needed):
Please re-review the latest commit. Happy to make further adjustments.
|
|
🤖 PR Comment Handler here — I've reviewed the feedback and confirmed it has been addressed in prior commits. Previously addressed (no further action needed): From
From
All 8 review threads appear to have been addressed. Please re-review and resolve the conversation threads when you get a chance.
|
…60318-bc79730ac1f7b737
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3626 +/- ##
============================================
+ Coverage 23.31% 23.36% +0.04%
- Complexity 3929 3938 +9
============================================
Files 924 924
Lines 56190 56194 +4
Branches 6654 6654
============================================
+ Hits 13098 13127 +29
+ Misses 41394 41369 -25
Partials 1698 1698 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
conroy-ricketts
left a comment
conroy-ricketts
left a comment
There was a problem hiding this comment.
All new unit tests written in Kotlin should utilize backticks to write test names as natural-language sentences.
…age [AI] 🤖 PR Comment Handler
|
Commit pushed:
|
|
🤖 PR Comment Handler Implemented:
Verified the two files have no new ktlint violations vs. master (a handful of pre-existing auto-fixable violations in unrelated lines were left in place — out of scope for this feedback). Please re-review.
|
…-assist/connectid-response-parsers-20260318-bc79730ac1f7b737
|
🤖 PR Comment Handler I checked the open review threads against the current head ( Already addressed (these threads can likely be resolved):
Merge status: No conflicts; the branch is up to date with If any of the above doesn't match what you intended, leave a follow-up comment and I'll pick it up on the next run.
|
|
🤖 PR Comment Handler I reviewed the open (unresolved) review threads against the current head commit. Each requested change is already present in the code, so no new commits were pushed. Already addressed (open threads):
No action taken:
If these resolve the feedback, the open threads can be marked resolved.
|
2 participants
🤖 Test Improver — automated AI assistant
Goal and Rationale
LinkHqWorkerResponseParserandRetrieveChannelEncryptionKeyResponseParserare connectId network parsers with zero test coverage. Both handle mutations to persistent model objects and trigger database writes — silent bugs here would cause worker linking state or channel encryption keys to be incorrectly stored.Why it matters: Incorrect worker-linked state would break job eligibility logic; a wrong or unset channel encryption key would prevent secure messaging.
Approach
Two new test classes in
app/unit-tests/src/org/commcare/connect/network/connectId/parser/:LinkHqWorkerResponseParserTesttestParse_setsWorkerLinkedTrue_andReturnsTrueparse()setsappRecord.workerLinked = trueand returnstruetestParse_workerLinkedFalseBeforeParse_trueAfterfalse, becomestrueafter parse (no toggle, only set)testParse_callsStoreApp_onceConnectAppDatabaseUtil.storeApp(context, appRecord)is called exactly onceRetrieveChannelEncryptionKeyResponseParserTesttestParse_validJson_setsKeyOnChannel_andReturnsTrue{"key": "abc123-encryption-key"}→channel.getKey() == "abc123-encryption-key", returnstruetestParse_emptyResponse_channelKeyUnchanged_andReturnsTruechannel.getKey()remainsnull, returnstrue, no exceptiontestParse_emptyResponse_doesNotCallStoreMessagingChannelstoreMessagingChannelis never called (guard path verified)testParse_callsStoreMessagingChannel_onceConnectMessagingDatabaseHelper.storeMessagingChannel(context, channel)called oncetestParse_invalidJson_throwsRuntimeExceptionRuntimeException(wrappingJSONException)testParse_missingKeyField_throwsRuntimeExceptionkeyfield →RuntimeException(wrappingJSONException)Static database methods are isolated using
mockStatic(Mockito 5.5.0 — already a test dependency). Tests follow the@Config(application = CommCareTestApplication::class)/@RunWith(AndroidJUnit4::class)pattern used across the suite.Coverage Impact
Both
LinkHqWorkerResponseParser.ktandRetrieveChannelEncryptionKeyResponseParser.ktpreviously had 0% coverage. This PR covers all branches ofparse()in both classes:LinkHqWorkerResponseParser.parse(): single path (no branching)RetrieveChannelEncryptionKeyResponseParser.parse(): theisNotEmpty()guard, the happy path, and theJSONExceptioncatchTrade-offs
storeAppandstoreMessagingChannelare static void methods — mocked withmockStaticto prevent database access in unit tests. This is the same pattern used inReportIntegrityResponseParserTest.LinkHqWorkerResponseParserignoresresponseCodeandresponseDataentirely; tests pass an emptyByteArrayInputStreamfor correctness without additional noise.Test Status
Build not runnable locally (requires
../commcare-core/sibling directory, checked out only in CI).ktlintFileGradle task unavailable in this environment due to a Gradle wrapper lock file issue (infrastructure constraint). Files follow the style of existing tests in the same package.To run: