test: add unit tests for VCProcessor SD-JWT reconstruction#2408
test: add unit tests for VCProcessor SD-JWT reconstruction#2408dhruv1955 wants to merge 2 commits into
Conversation
Signed-off-by: Chandra Keshav Mishra <chandrakeshavmishra@gmail.com>
Signed-off-by: dhruv1955 <dhruvyadav042905@gmail.com>
WalkthroughA new test file for VCProcessor is added with comprehensive unit tests covering reconstructSdJwtFromCompact and processForRendering methods across multiple VC formats (vc_sd_jwt, dc_sd_jwt, ldp_vc). The test file is registered in the Talisman ignore configuration. Changes
Estimated Code Review Effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly Related PRs
Suggested Reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 (2)
components/VC/common/VCProcessor.test.ts (2)
32-32: Nit:createDisclosureis a thin alias.
createDisclosurejust forwards toencodeBase64Urlwithout adding behavior. Either inline the call sites or have it perform the SD-JWT-specific tuple shape check ([salt, key, value]vs[salt, value]) so the helper carries semantic value.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/VC/common/VCProcessor.test.ts` at line 32, createDisclosure is just a thin alias to encodeBase64Url; either inline encodeBase64Url at the test call sites or enhance createDisclosure to validate and normalize SD-JWT disclosure tuples before encoding. Specifically, update the helper createDisclosure to accept a value: unknown[], assert it matches SD-JWT disclosure shapes (length 2 or 3), verify the first element is a salt string and the optional second element (for 3-tuple) is a key string, then construct the canonical tuple ([salt, key, value] or [salt, value]) and pass that to encodeBase64Url; alternatively remove createDisclosure and replace its usages with direct calls to encodeBase64Url with properly-formed tuples.
172-253: Consider addingmso_mdoccoverage.The
RNPixelpassModule.decodeBase64UrlEncodedCBORDatamock is set up at lines 10-12 but never asserted against.VCProcessor.processForRenderinghas twomso_mdocbranches (returningvcData.processedCredentialearly, and decoding viaRNPixelpassModule) that are currently uncovered. Adding two small tests there would close the remaining gap in this method.Want me to draft those two
mso_mdoctest cases?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/VC/common/VCProcessor.test.ts` around lines 172 - 253, Add two unit tests to cover VCProcessor.processForRendering's VCFormat.mso_mdoc branches: 1) a test where vcData contains processedCredential and calling VCProcessor.processForRendering with VCFormat.mso_mdoc returns vcData.processedCredential directly (assert no call to RNPixelpassModule.decodeBase64UrlEncodedCBORData and that the returned value equals processedCredential); 2) a test where vcData contains a base64url CBOR payload and stub RNPixelpassModule.decodeBase64UrlEncodedCBORData to return a decoded object, then call VCProcessor.processForRendering with VCFormat.mso_mdoc and assert RNPixelpassModule.decodeBase64UrlEncodedCBORData was called with the encoded payload and that the returned value matches the decoded object; reference VCProcessor.processForRendering, VCFormat.mso_mdoc, vcData.processedCredential and RNPixelpassModule.decodeBase64UrlEncodedCBORData to locate the logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@components/VC/common/VCProcessor.test.ts`:
- Line 32: createDisclosure is just a thin alias to encodeBase64Url; either
inline encodeBase64Url at the test call sites or enhance createDisclosure to
validate and normalize SD-JWT disclosure tuples before encoding. Specifically,
update the helper createDisclosure to accept a value: unknown[], assert it
matches SD-JWT disclosure shapes (length 2 or 3), verify the first element is a
salt string and the optional second element (for 3-tuple) is a key string, then
construct the canonical tuple ([salt, key, value] or [salt, value]) and pass
that to encodeBase64Url; alternatively remove createDisclosure and replace its
usages with direct calls to encodeBase64Url with properly-formed tuples.
- Around line 172-253: Add two unit tests to cover
VCProcessor.processForRendering's VCFormat.mso_mdoc branches: 1) a test where
vcData contains processedCredential and calling VCProcessor.processForRendering
with VCFormat.mso_mdoc returns vcData.processedCredential directly (assert no
call to RNPixelpassModule.decodeBase64UrlEncodedCBORData and that the returned
value equals processedCredential); 2) a test where vcData contains a base64url
CBOR payload and stub RNPixelpassModule.decodeBase64UrlEncodedCBORData to return
a decoded object, then call VCProcessor.processForRendering with
VCFormat.mso_mdoc and assert RNPixelpassModule.decodeBase64UrlEncodedCBORData
was called with the encoded payload and that the returned value matches the
decoded object; reference VCProcessor.processForRendering, VCFormat.mso_mdoc,
vcData.processedCredential and RNPixelpassModule.decodeBase64UrlEncodedCBORData
to locate the logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f012732d-cb34-4009-9865-ba49f7486a34
📒 Files selected for processing (2)
.talismanrccomponents/VC/common/VCProcessor.test.ts
What and Why
VCProcessor.tsis the core SD-JWT parsing layer but had zero test coverage. This PR adds 8 unit tests covering the full reconstruction and rendering pipeline.Changes
components/VC/common/VCProcessor.test.ts- new test file.talismanrc- added checksum ignore for JWT claim names (iss,iat) that were flagged as false positivesTests Added
reconstructSdJwtFromCompact()
processForRendering()
Summary by CodeRabbit