-
Notifications
You must be signed in to change notification settings - Fork 171
MOSIP-42531 : PR from develop branch for Automated kyc delegate and kyc exchange scenarios for BIO,OTP and DEMO #1611
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
Signed-off-by: Anuranjan14 <anuranjan.kumar@technoforte.co.in>
Signed-off-by: Anuranjan14 <anuranjan.kumar@technoforte.co.in>
Signed-off-by: Anuranjan14 <anuranjan.kumar@technoforte.co.in>
Signed-off-by: Anuranjan14 <anuranjan.kumar@technoforte.co.in>
Signed-off-by: Anuranjan14 <anuranjan.kumar@technoforte.co.in>
Signed-off-by: Anuranjan14 <anuranjan.kumar@technoforte.co.in>
Signed-off-by: Anuranjan14 <anuranjan.kumar@technoforte.co.in>
Signed-off-by: Anuranjan14 <anuranjan.kumar@technoforte.co.in>
…IO,OTP and DEMO (mosip#1567) * MOSIP-42531 : Automated bio and demo kyc delegate and kyc exchange scenarios Signed-off-by: Anuranjan14 <anuranjan.kumar@technoforte.co.in> * Added ekyc delegated scenarios Signed-off-by: Anuranjan14 <anuranjan.kumar@technoforte.co.in> * MOSIP- 42531 : Added demo auth delegated xml to run demo auth scenarios Signed-off-by: Anuranjan14 <anuranjan.kumar@technoforte.co.in> * MOSIP-42531: Updated demo auth delegated scenarios Signed-off-by: Anuranjan14 <anuranjan.kumar@technoforte.co.in> * MOSIP-42531: Updated transactionId for demo auth delegated Signed-off-by: Anuranjan14 <anuranjan.kumar@technoforte.co.in> * MOSIP-42531: Fixed issues in demo auth Signed-off-by: Anuranjan14 <anuranjan.kumar@technoforte.co.in> * MOSIP-42531: Fixed issues from yaml Signed-off-by: Anuranjan14 <anuranjan.kumar@technoforte.co.in> * MOSIP-42531: Updated xml to run full auth suite Signed-off-by: Anuranjan14 <anuranjan.kumar@technoforte.co.in> * MOSIP-42531: Resolved the reviewed comments Signed-off-by: Anuranjan14 <anuranjan.kumar@technoforte.co.in> * MOSIP-42531: Added common DelegateError hbs Signed-off-by: Anuranjan14 <anuranjan.kumar@technoforte.co.in> --------- Signed-off-by: Anuranjan14 <anuranjan.kumar@technoforte.co.in>
…IO,OTP and DEMO Signed-off-by: Anuranjan14 <anuranjan.kumar@technoforte.co.in>
WalkthroughAdds delegated authentication test suites and templates (Bio/Demo/OTP) and updates two Java test scripts to support a new Changes
Sequence Diagram(s)sequenceDiagram
participant TestNG as TestNG Suite
participant Script as DemoAuth/OtpAuthNew
participant Template as Endpoint Template Engine
participant Partner as Partner URL Resolver
participant HTTP as HTTP Client
participant Store as AutoID Store
Note over TestNG,Script: Test starts with test context containing idKeyName (optional)
TestNG->>Script: startTest(testContext)
Script->>Template: load endpoint (may include $partialPartnerKeyUrl$)
Template->>Partner: compute partialPartnerKeyUrl (if placeholder)
Partner-->>Template: resolved endpoint
Script->>Script: replaceIdWithAutogeneratedId(endpoint, "$ID:")
Script->>HTTP: POST resolved endpoint with payload
HTTP-->>Script: response
alt test name contains "_sid"
Script->>Store: writeAutoGeneratedId(response, idKeyName, testName)
end
Script-->>TestNG: testResult
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 20
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
api-test/src/main/resources/ida/GenerateVID/createGenerateVID.yml (1)
478-511: Test data identifier mismatch in rightIris scenario.The scenario
auth_GenerateVID_for_kycDelegated_bio_rightIris_sid(line 478) uses test data identifiers containing "leftIris" (lines 489, 495, 499):
AddIdentity_For_Kyc_Delegated_leftIris_Auth_VID_smoke_Pos_UINAddIdentity_For_Kyc_Delegated_leftIris_Auth_VID_smoke_Pos_EMAILThis appears to be a copy-paste error. The scenario should reference corresponding rightIris test data identifiers if it's intended to test the rightIris authentication path.
Verify whether this scenario should use rightIris test data identifiers or if the scenario name should be corrected to reference leftIris instead.
♻️ Duplicate comments (2)
api-test/src/main/resources/ida/OtpAuthDelegated/Sendotperror.hbs (1)
1-17: Duplicate template detected.This template is identical to
api-test/src/main/resources/ida/OtpAuthDelegatedNeg/Sendotperror.hbs. The duplication was already noted in that file's review.api-test/src/main/resources/ida/OtpAuthDelegated/OtpAuthDelegated.hbs (1)
1-23: Duplicate template structure detected.This template is structurally identical to
api-test/src/main/resources/ida/DemoAuthDelegatedNeg/DemoAuthDelegated.hbs. The duplication was already noted in that file's review.
🧹 Nitpick comments (11)
api-test/src/main/resources/ida/DelegateError.hbs (1)
6-8: Improve JSON formatting by placing comma immediately after closing brace.The current placement of the comma separator on line 7 will generate JSON with the comma on a separate line with leading whitespace, making the output less readable.
Apply this diff to improve the formatting:
- } - {{#unless @last}},{{/unless}} - {{/each}} + }{{#unless @last}},{{/unless}} + {{/each}}This will generate cleaner JSON output:
}, {instead of:
} , {api-test/src/main/resources/ida/DemoAuthDelegated/DemoIdentityEncryptName.hbs (1)
1-14: Consider standardizing indentation.The template mixes tabs and spaces for indentation. While this doesn't affect functionality, consistent indentation (preferably tabs or spaces throughout) improves maintainability.
api-test/src/main/resources/ida/OtpAuthDelegated/sendOtp.hbs (1)
1-12: LGTM, but note code duplication.This template is identical to
api-test/src/main/resources/ida/OtpAuthDelegatedNeg/sendOtp.hbs. Consider whether these can be consolidated into a shared template to reduce maintenance overhead.api-test/src/main/resources/ida/DemoAuthDelegated/DemoIdentityEncryptGenderRS.hbs (1)
1-23: Consider standardizing indentation.The template mixes tabs and spaces for indentation, similar to other templates in this PR. Consistent indentation improves maintainability.
api-test/src/main/resources/ida/OtpAuthDelegatedNeg/sendOtp.hbs (1)
1-12: Code duplication detected.This template is identical to
api-test/src/main/resources/ida/OtpAuthDelegated/sendOtp.hbs. Consider consolidating these into a single shared template to avoid maintenance issues when updates are needed.If the templates need to diverge for positive vs. negative test scenarios in the future, document why they're kept separate.
api-test/src/main/java/io/mosip/testrig/apirig/auth/testscripts/OtpAuthNew.java (1)
42-42: Consider encapsulation for the new field.The new
idKeyNamefield is publicly exposed. While acceptable for test classes, prefer private fields with getters/setters for better encapsulation.api-test/src/main/resources/ida/OtpAuthDelegatedNeg/Sendotperror.hbs (1)
1-17: Template is correct but consider consolidation.This error template is duplicated at
api-test/src/main/resources/ida/OtpAuthDelegated/Sendotperror.hbs. Consider consolidating identical templates into a shared location to reduce maintenance overhead.api-test/src/main/resources/ida/DemoAuthDelegatedNeg/DemoAuthDelegated.hbs (1)
1-23: Template is valid; consider consolidation with OtpAuthDelegated.hbs.This template is structurally identical to
api-test/src/main/resources/ida/OtpAuthDelegated/OtpAuthDelegated.hbs. Consider consolidating identical templates for better maintainability.api-test/src/main/java/io/mosip/testrig/apirig/auth/testscripts/DemoAuth.java (1)
44-44: Consider encapsulation for the new field.Similar to
OtpAuthNew.java, theidKeyNamefield is publicly exposed. Consider using private fields with getters/setters for better encapsulation.api-test/src/main/resources/ida/OtpAuthDelegatedNeg/OtpAuthDelegatedNeg.hbs (1)
6-7: Consider parameterizing hardcoded values for improved test flexibility.Several fields contain hardcoded string values (
"data","hmac","sessionkey","baseurl","Staging","string","partnerORinternalthumbprint"). While this may be intentional for negative testing scenarios, parameterizing these values would improve test reusability and make it easier to test different environments or configurations.Also applies to: 15-15, 17-21
api-test/src/main/resources/ida/BioAuthDelegated/BioAuthDelegated.yml (1)
18-37: Consider extracting duplicated device metadata.The device metadata block (deviceCode, dateTime, deviceProviderID, deviceServiceID, deviceServiceVersion, deviceProvider, deviceProviderId, deviceSubType, make, model, serialNo, type) is duplicated across all 25 test scenarios with identical values.
This creates maintenance burden—updating device information requires changing 25+ locations. Consider extracting this to a shared template, constant, or test data fixture that can be referenced by all scenarios.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (48)
api-test/src/main/java/io/mosip/testrig/apirig/auth/testscripts/DemoAuth.java(4 hunks)api-test/src/main/java/io/mosip/testrig/apirig/auth/testscripts/OtpAuthNew.java(5 hunks)api-test/src/main/resources/ida/AddIdentity/AddIdentity.yml(10 hunks)api-test/src/main/resources/ida/AuthInternalLock/AuthInternalLock.yml(1 hunks)api-test/src/main/resources/ida/BioAuthDelegated/BioAuthDelegated.yml(1 hunks)api-test/src/main/resources/ida/BioAuthDelegatedNeg/BioAuthDelegated.hbs(1 hunks)api-test/src/main/resources/ida/BioAuthDelegatedNeg/BioAuthDelegatedNeg.yml(1 hunks)api-test/src/main/resources/ida/BioAuthDelegatedNeg/BioAuthIdentityEncrypt.hbs(1 hunks)api-test/src/main/resources/ida/BioAuthDelegatedNeg/BioKYCAuthDelegatedResult.hbs(1 hunks)api-test/src/main/resources/ida/BioAuthKycExchange/BioAuthKYCExchange.yml(1 hunks)api-test/src/main/resources/ida/DelegateError.hbs(1 hunks)api-test/src/main/resources/ida/DemoAuthDelegated/DemoAuthDelegated.hbs(1 hunks)api-test/src/main/resources/ida/DemoAuthDelegated/DemoAuthDelegated.yml(1 hunks)api-test/src/main/resources/ida/DemoAuthDelegated/DemoAuthDelegatedResult.hbs(1 hunks)api-test/src/main/resources/ida/DemoAuthDelegated/DemoIdentityEncrypt.hbs(1 hunks)api-test/src/main/resources/ida/DemoAuthDelegated/DemoIdentityEncryptGender.hbs(1 hunks)api-test/src/main/resources/ida/DemoAuthDelegated/DemoIdentityEncryptGenderRS.hbs(1 hunks)api-test/src/main/resources/ida/DemoAuthDelegated/DemoIdentityEncryptInvalid.hbs(1 hunks)api-test/src/main/resources/ida/DemoAuthDelegated/DemoIdentityEncryptName.hbs(1 hunks)api-test/src/main/resources/ida/DemoAuthDelegatedNeg/DemoAuthDelegated.hbs(1 hunks)api-test/src/main/resources/ida/DemoAuthDelegatedNeg/DemoAuthDelegatedNeg.yml(1 hunks)api-test/src/main/resources/ida/DemoAuthDelegatedNeg/DemoAuthDelegatedResult.hbs(1 hunks)api-test/src/main/resources/ida/DemoAuthDelegatedNeg/DemoIdentityEncrypt.hbs(1 hunks)api-test/src/main/resources/ida/DemoAuthDelegatedNeg/DemoIdentityEncryptGender.hbs(1 hunks)api-test/src/main/resources/ida/DemoAuthDelegatedNeg/DemoIdentityEncryptGenderRS.hbs(1 hunks)api-test/src/main/resources/ida/DemoAuthDelegatedNeg/DemoIdentityEncryptInvalid.hbs(1 hunks)api-test/src/main/resources/ida/DemoAuthDelegatedNeg/DemoIdentityEncryptName.hbs(1 hunks)api-test/src/main/resources/ida/DemoAuthKycExchange/DemoAuthKycExchange.hbs(1 hunks)api-test/src/main/resources/ida/DemoAuthKycExchange/DemoAuthKycExchange.yml(1 hunks)api-test/src/main/resources/ida/DemoAuthKycExchange/DemoAuthKycExchangeResult.hbs(1 hunks)api-test/src/main/resources/ida/GenerateVID/createGenerateVID.yml(2 hunks)api-test/src/main/resources/ida/OtpAuthDelegated/OtpAuthDelegated.hbs(1 hunks)api-test/src/main/resources/ida/OtpAuthDelegated/OtpAuthDelegated.yml(1 hunks)api-test/src/main/resources/ida/OtpAuthDelegated/OtpAuthDelegatedResult.hbs(1 hunks)api-test/src/main/resources/ida/OtpAuthDelegated/OtpIdentityEncrypt.json(1 hunks)api-test/src/main/resources/ida/OtpAuthDelegated/Sendotperror.hbs(1 hunks)api-test/src/main/resources/ida/OtpAuthDelegated/sendOtp.hbs(1 hunks)api-test/src/main/resources/ida/OtpAuthDelegated/sendOtpRes.hbs(1 hunks)api-test/src/main/resources/ida/OtpAuthDelegatedNeg/OtpAuthDelegatedNeg.hbs(1 hunks)api-test/src/main/resources/ida/OtpAuthDelegatedNeg/OtpAuthDelegatedNeg.yml(1 hunks)api-test/src/main/resources/ida/OtpAuthDelegatedNeg/OtpIdentityEncrypt.json(1 hunks)api-test/src/main/resources/ida/OtpAuthDelegatedNeg/Sendotperror.hbs(1 hunks)api-test/src/main/resources/ida/OtpAuthDelegatedNeg/sendOtp.hbs(1 hunks)api-test/src/main/resources/ida/OtpAuthDelegatedNeg/sendOtpRes.hbs(1 hunks)api-test/src/main/resources/ida/OtpAuthKycExchange/OtpAuthKycExchange.hbs(1 hunks)api-test/src/main/resources/ida/OtpAuthKycExchange/OtpAuthKycExchange.yml(1 hunks)api-test/src/main/resources/ida/OtpAuthKycExchange/OtpAuthKycExchangeResult.hbs(1 hunks)api-test/testNgXmlFiles/authSuite.xml(3 hunks)
🔇 Additional comments (32)
api-test/src/main/resources/ida/DelegateError.hbs (1)
5-5: Verify if error objects should include additional fields.The error object currently contains only
errorCode. Standard error responses typically include anerrorMessagefield as well to provide human-readable descriptions.Please confirm whether the minimal structure is intentional or if additional fields should be included:
{ "errorCode": "{{errorCode}}", "errorMessage": "{{errorMessage}}" }api-test/src/main/resources/ida/BioAuthDelegatedNeg/BioKYCAuthDelegatedResult.hbs (1)
1-5: LGTM!The result template structure is correct and follows the expected pattern for KYC status responses.
api-test/src/main/resources/ida/OtpAuthDelegatedNeg/OtpIdentityEncrypt.json (1)
1-4: LGTM!The OTP identity encryption payload structure is minimal and appropriate for test scenarios.
api-test/src/main/resources/ida/OtpAuthKycExchange/OtpAuthKycExchangeResult.hbs (1)
1-1: No issues found—empty result template is intentional.The template is part of a consistent pattern across all KycExchange variants (Bio, Demo, Otp), and the configuration in OtpAuthKycExchange.yml specifies
checkErrorsOnlyInResponse: true, indicating the tests only validate error conditions, not the response structure. An empty object{}is the appropriate response format for this use case.api-test/src/main/resources/ida/DemoAuthDelegated/DemoIdentityEncryptGenderRS.hbs (1)
22-22: No changes required—field naming difference is intentional.The grep results confirm that all
*GenderRS.hbstemplates consistently use{{requestTime}}, while otherDemoIdentity*.hbstemplates use{{timestamp}}. This pattern is deliberate: the "RS" (Response Schema) templates reference the preservedrequestTimefield, while regular templates reference the generatedtimestampfield.The calling code in
DemoAuth.javasupports this design by separately managing both fields—injecting atimestampvalue (lines 139, 145) and explicitly preserving the originalrequestTimeacross request modifications (lines 149–150, 158). The codebase provides both fields correctly.api-test/src/main/resources/ida/DemoAuthDelegated/DemoAuthDelegated.hbs (1)
6-7: These placeholder values are intentional for test schema validation—no changes required.The codebase consistently uses these placeholder strings across 40+ Handlebars templates. The pattern mixing static placeholders (
"request": "data","requestHMAC": "hmac") with dynamic Handlebars variables ({{requestTime}},{{transactionId}}) is systematic throughout the test framework. The templates are used inOutputValidationUtil.doJsonOutputValidation()for request-response schema validation rather than generating live API calls, where placeholder values serve to maintain structural integrity during test execution. This pattern is found identically in OtpAuth, BioAuth, MultiFactorAuth, DemoAuth, and related test templates, confirming it's an intentional design choice.api-test/src/main/resources/ida/DemoAuthDelegatedNeg/DemoIdentityEncryptGenderRS.hbs (1)
1-23: LGTM!The Handlebars template correctly iterates over collections with proper comma handling using
{{#unless @last}}, preventing trailing commas in the generated JSON.api-test/src/main/java/io/mosip/testrig/apirig/auth/testscripts/OtpAuthNew.java (2)
70-70: Verify idKeyName parameter is provided when test names contain "_sid".The
idKeyNameparameter is retrieved from the test context without null checking. Since it's used at line 188 whentestCaseName.toLowerCase().contains("_sid"), ensure all relevant test configurations provide this parameter.
186-189: LGTM!The conditional logic correctly writes auto-generated IDs for test cases containing "_sid" in their names, supporting the new delegated authentication scenarios.
api-test/src/main/resources/ida/BioAuthDelegatedNeg/BioAuthIdentityEncrypt.hbs (1)
1-37: LGTM!The biometric identity encryption template correctly structures the payload with proper placeholders for dynamic values and follows the expected schema for biometric authentication data.
api-test/src/main/java/io/mosip/testrig/apirig/auth/testscripts/DemoAuth.java (2)
72-72: Verify idKeyName parameter is provided when test names contain "_sid".The
idKeyNameparameter is retrieved without null checking. Ensure all test configurations for "_sid" test cases provide this parameter, as it's used at line 171.
170-173: LGTM!The conditional logic for writing auto-generated IDs is correctly implemented and consistent with the parallel changes in
OtpAuthNew.java.api-test/src/main/resources/ida/AddIdentity/AddIdentity.yml (1)
1926-2255: LGTM!The test configuration changes are well-structured:
- Unique identifier renaming follows a consistent pattern (TC_DependentModule_IdRepo_Delegated_XX)
- Test names appropriately updated with "smoke" suffix
- Two new test blocks for OTP and locked UIN demo delegation scenarios mirror the structure of existing entries
api-test/src/main/resources/ida/DemoAuthDelegatedNeg/DemoIdentityEncryptInvalid.hbs (1)
1-6: LGTM!The template structure is valid and uses proper Handlebars placeholder syntax for dynamic key-value pairs in demographics.
api-test/src/main/resources/ida/DemoAuthDelegatedNeg/DemoIdentityEncrypt.hbs (1)
1-6: LGTM!The template provides a clean structure for delegated demo authentication identity encryption payloads.
api-test/src/main/resources/ida/DemoAuthDelegated/DemoIdentityEncrypt.hbs (1)
1-6: LGTM!The template is consistent with the related negative scenario templates and provides proper structure for delegated demo authentication flows.
api-test/src/main/resources/ida/OtpAuthDelegated/OtpIdentityEncrypt.json (1)
1-4: LGTM!The template provides a minimal, well-structured payload for OTP delegated authentication identity encryption.
api-test/src/main/resources/ida/OtpAuthDelegatedNeg/sendOtpRes.hbs (1)
1-11: LGTM!The template appropriately uses
$IGNORE$placeholders for flexible response validation in delegated OTP authentication tests.api-test/src/main/resources/ida/DemoAuthDelegated/DemoIdentityEncryptInvalid.hbs (1)
1-6: LGTM!The template maintains consistency with other identity encryption templates across delegated authentication scenarios.
api-test/src/main/resources/ida/DemoAuthDelegated/DemoAuthDelegatedResult.hbs (1)
1-5: AI summary inconsistency detected.The AI summary mentions
response.authStatusbut the actual field in the template isresponse.kycStatus. The code is correct for a KYC delegated result template.api-test/src/main/resources/ida/OtpAuthDelegated/OtpAuthDelegatedResult.hbs (1)
1-5: LGTM!The template structure is appropriate for an OTP delegated KYC result response.
api-test/src/main/resources/ida/DemoAuthKycExchange/DemoAuthKycExchangeResult.hbs (1)
1-1: Verify that empty result template is intentional.The template contains only an empty object
{}. If the test scenarios using this template need to validate response structure or fields, the template may need to be populated with expected response fields and placeholders.api-test/src/main/resources/ida/OtpAuthDelegated/sendOtpRes.hbs (1)
1-11: LGTM!The template correctly uses
$IGNORE$placeholders for all fields, which is appropriate for test scenarios where specific response values don't need validation. The response structure properly includes all standard OTP response fields.api-test/src/main/resources/ida/DemoAuthDelegatedNeg/DemoIdentityEncryptGender.hbs (1)
1-14: LGTM!The Handlebars template correctly iterates over the gender collection with proper comma separation handling. The use of
{{timestamp}}placeholder is appropriate for test flexibility.api-test/src/main/resources/ida/OtpAuthKycExchange/OtpAuthKycExchange.hbs (1)
9-14: Verify Handlebars variable names match data structure.The template references
{{consentObtainedValue}}(line 11) and{{localesValue}}(line 17) within the iteration blocks. Ensure these property names match the actual data structure being passed to the template. If the arrays contain simple string values, consider using{{this}}instead for clarity.Also applies to: 15-20
api-test/src/main/resources/ida/DemoAuthKycExchange/DemoAuthKycExchange.hbs (1)
1-23: LGTM! Template structure is correct.The Handlebars template properly handles array iteration with correct comma placement using
{{#unless @last}}for bothconsentObtainedandlocalesarrays.api-test/src/main/resources/ida/DemoAuthDelegated/DemoIdentityEncryptGender.hbs (1)
1-14: LGTM! Gender demographics template is correctly structured.The template properly iterates over the gender array with appropriate comma handling between items.
api-test/src/main/resources/ida/DemoAuthDelegatedNeg/DemoAuthDelegatedResult.hbs (1)
1-5: LGTM! Result template is straightforward and correct.api-test/src/main/resources/ida/DemoAuthDelegated/DemoAuthDelegated.yml (1)
1-561: Good test coverage for demographic authentication scenarios.The test scenarios comprehensively cover various demographic attributes (DOB, email, gender, name with titles, age, phone) across different identifier types (UIN/VID) and special cases (infant UIN, drafted UIN). The consistent structure makes the test suite maintainable.
api-test/testNgXmlFiles/authSuite.xml (1)
43-58: The review comment is based on incorrect assumptions about the PR's objectives.The premise that V2 tests are being "replaced" contradicts the actual codebase structure. Evidence shows:
- New KYC tests (lines 18-42:
BioAuthKYCDelegated,BioAuthKYCDelegatedNeg,BioAuthKYCExchange) are separate additions, not replacements for V2 variants- V2 tests (lines 43-58:
BioAuthDelegatedV2,BioAuthKycExchangeV2) coexist with new KYC tests intentionally- No deprecated or old "BioAuthDelegated"/"BioAuthKycExchange" test blocks exist for V2 to replace
- Both V2 and new KYC variants maintain supported YAML data files
The coexistence of both variants is intentional feature expansion, not a deprecation scenario. The V2 tests should remain.
Likely an incorrect or invalid review comment.
api-test/src/main/resources/ida/DemoAuthDelegatedNeg/DemoAuthDelegatedNeg.yml (1)
12-12: Verify template resource references exist.This file references templates like
ida/DemoAuthDelegatedNeg/DemoIdentityEncrypt(line 21),ida/DemoAuthDelegatedNeg/DemoIdentityEncryptInvalid(line 79),ida/DemoAuthDelegatedNeg/DemoIdentityEncryptGenderRS(line 350), etc.Confirm all referenced Handlebars template files exist in the repository under
api-test/src/main/resources/ida/DemoAuthDelegatedNeg/directory.Also applies to: 41-41
api-test/src/main/resources/ida/BioAuthDelegatedNeg/BioAuthDelegatedNeg.yml (1)
1115-1115: Template path inconsistency.Late test scenarios (lines 1115, 1163, 1211, 1306) reference template
ida/BioAuth/BioAuthIdentityEncrypt2, while earlier tests referenceida/BioAuthDelegatedNeg/BioAuthIdentityEncrypt.Verify that:
- Both template files exist.
- Path difference is intentional (if not, standardize to one consistent path).
- These are correct qualified paths relative to
api-test/src/main/resources/.Also applies to: 1163-1163, 1211-1211, 1306-1306
api-test/src/main/java/io/mosip/testrig/apirig/auth/testscripts/DemoAuth.java
Show resolved
Hide resolved
api-test/src/main/java/io/mosip/testrig/apirig/auth/testscripts/OtpAuthNew.java
Show resolved
Hide resolved
api-test/src/main/resources/ida/BioAuthDelegatedNeg/BioAuthDelegated.hbs
Show resolved
Hide resolved
api-test/src/main/resources/ida/DemoAuthKycExchange/DemoAuthKycExchange.yml
Show resolved
Hide resolved
api-test/src/main/resources/ida/OtpAuthDelegatedNeg/OtpAuthDelegatedNeg.yml
Show resolved
Hide resolved
api-test/src/main/resources/ida/OtpAuthDelegatedNeg/OtpAuthDelegatedNeg.yml
Show resolved
Hide resolved
api-test/src/main/resources/ida/OtpAuthKycExchange/OtpAuthKycExchange.yml
Show resolved
Hide resolved
Signed-off-by: Anuranjan14 <anuranjan.kumar@technoforte.co.in>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
api-test/src/main/java/io/mosip/testrig/apirig/auth/testscripts/DemoAuth.java (1)
44-44: Consider encapsulation, though acceptable for test classes.The public field
idKeyNamebreaks encapsulation. While this is generally acceptable in test classes, consider making it private if it doesn't need external access.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
api-test/src/main/java/io/mosip/testrig/apirig/auth/testscripts/DemoAuth.java(4 hunks)api-test/src/main/java/io/mosip/testrig/apirig/auth/testscripts/OtpAuthNew.java(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- api-test/src/main/java/io/mosip/testrig/apirig/auth/testscripts/OtpAuthNew.java
🔇 Additional comments (4)
api-test/src/main/java/io/mosip/testrig/apirig/auth/testscripts/DemoAuth.java (4)
83-83: LGTM!The
partialPartnerKeyUrlconstruction is correct and follows the same pattern as other URL variables in the method.
91-96: LGTM! Previous threading issue has been resolved.The endpoint replacement logic correctly uses
.set()method at line 95, maintaining thread-local semantics. The additionalreplaceIdWithAutogeneratedIdcall at line 93 is appropriate for this endpoint pattern.
72-72: Unable to fully verify the review comment due to incomplete information.The script execution revealed many test cases with "_sid" suffixes across the test suite, confirming that such tests exist. However, the search for "idKeyName" parameter definitions in XML configurations was cut off before completion, preventing verification of whether all "_sid" tests provide this parameter.
More critically, I lack access to the complete DemoAuth.java code to determine:
- Whether a null check exists for
idKeyNameafter line 72- How
idKeyNameis actually used at line 171- Whether the code safely handles null values in the conditional logic
The review comment's concern assumes the code could fail if
idKeyNameis null when a "_sid" test runs, but without seeing the full implementation (including any null-checking or try-catch logic), this risk cannot be definitively confirmed or dismissed.
170-172: Verify null-safety of idKeyName before the writeAutoGeneratedId call.The method
writeAutoGeneratedIdis inherited from an external dependency (not found in repository source), so its null-handling behavior cannot be verified. However, the concern is valid:idKeyNameis initialized to null if "idKeyName" is not provided in the test context (line 72), and line 171 callswriteAutoGeneratedIdwithout checking ifidKeyNameis null.Consider adding a null check to be defensive:
- if (testCaseName.toLowerCase().contains("_sid")) { - writeAutoGeneratedId(response, idKeyName, testCaseName); - } + if (testCaseName.toLowerCase().contains("_sid") && idKeyName != null) { + writeAutoGeneratedId(response, idKeyName, testCaseName); + }
Automated bio auth kyc delegated and kyc exchange
Automated demo auth kyc delegated and kyc exchange
Automated ekyc otp scenarios
Summary by CodeRabbit
New Features
Tests