MOSIP-44096 Added additional dependancies for single test case execution #996
Conversation
Signed-off-by: Nitin Hegde <[email protected]>
WalkthroughAdds a runtime flag to control generation/loading of test-case inter-dependencies. MosipTestRunner reads Changes
Sequence Diagram(s)sequenceDiagram
participant Runner as "MosipTestRunner"
participant Admin as "AdminTestUtil"
participant Util as "MimotoUtil"
participant Config as "InterDependency JSON / YAMLs"
Runner->>Runner: read mimoto.properties (generateDependencyJson)
Runner->>Admin: init()
alt generateDependency != "yes"
Runner->>Config: load testCaseInterDependency.json
Config-->>Util: populate testCasesInRunScope
else generateDependency == "yes"
Runner->>Runner: skip inter-dependency load
end
loop per test case
Runner->>Util: isTestCaseValidForTheExecution(testCaseDTO)
Util->>Admin: check AdminTestUtil.generateDependency flag
alt flag == true and testCaseDTO.additionalDependencies != null
Util->>Util: addAdditionalDependencies(testCaseDTO)
end
Util-->>Runner: validation result
end
alt generateDependency == "yes"
Runner->>Admin: on shutdown -> generateTestCaseInterDependencies(path)
Admin->>Config: write/update testCaseInterDependency.json
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
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/mimoto/LoginFlow/DownloadMosipIssuerCredential/ViewCredential/ViewCredential.yml (1)
301-320: DuplicateuniqueIdentifierwill break single test case execution.
TC_Mimoto_ViewCredential_14is used by bothMimoto_ViewCredential_SpaceIn_AcceptLanguage_Neg(Line 283) andMimoto_ViewCredential_Invalid_AcceptHeader_Neg(Line 304). Since the PR objective is to enable single test case execution, this duplicate identifier will cause ambiguity when targeting a specific test.Suggested fix: Renumber the uniqueIdentifiers
Mimoto_ViewCredential_Invalid_AcceptHeader_Neg: endPoint: /v1/mimoto/wallets/{walletId}/credentials/{credentialId}?action=inline description: View the credential saved for the wallet with invalid accept header and expects it to fail - uniqueIdentifier: TC_Mimoto_ViewCredential_14 + uniqueIdentifier: TC_Mimoto_ViewCredential_14b role: userDefinedCookieAlternatively, renumber all subsequent identifiers (15 → 16, 16 → 17, 17 → 18) and use
TC_Mimoto_ViewCredential_15here.
🧹 Nitpick comments (3)
api-test/src/main/java/io/mosip/testrig/apirig/mimoto/utils/MimotoUtil.java (1)
99-104: Minor cleanup suggestions for this conditional block.
- Typo in comment: "additionalDependancies" should be "additionalDependencies"
- Redundant null check:
testCaseDTO != nullis unnecessary sincetestCaseDTO.getTestCaseName()is already called at line 92 without a null check—if it were null, the method would have already thrown an NPE.- Non-idiomatic boolean comparison: Use
AdminTestUtil.generateDependencyinstead ofAdminTestUtil.generateDependency == true.♻️ Suggested cleanup
- - // enable the condition only for capturing the additionalDependancies into json file - if (testCaseDTO != null && testCaseDTO.getAdditionalDependencies() != null - && AdminTestUtil.generateDependency == true) { + // Enable the condition only for capturing the additionalDependencies into JSON file + if (testCaseDTO.getAdditionalDependencies() != null && AdminTestUtil.generateDependency) { addAdditionalDependencies(testCaseDTO); }api-test/src/main/java/io/mosip/testrig/apirig/mimoto/testrunner/MosipTestRunner.java (2)
83-86: Consider making dependency generation configurable rather than always enabled.Unconditionally setting
generateDependency = truemeans dependency collection will run for every test execution, which may have performance implications or generate files that aren't needed in CI/production runs. TheFIX:comment also suggests this might be a temporary workaround.Consider controlling this via a configuration property (e.g.,
MimotoConfigManager.getproperty("generateDependency")) to allow enabling it only when needed (e.g., during development or when updating dependency metadata).♻️ Suggested approach
AdminTestUtil.init(); MimotoConfigManager.init(); - - // FIX: Enable dependency generation so MimotoUtil collects dependencies from YAML - AdminTestUtil.generateDependency = true; + + // Enable dependency generation if configured (useful for updating dependency metadata) + String generateDep = MimotoConfigManager.getproperty("generateDependency"); + AdminTestUtil.generateDependency = "true".equalsIgnoreCase(generateDep);
132-133: Remove or properly control commented-out code.Commented-out code should typically be removed rather than left in the codebase. If this functionality needs to be toggled, use a configuration flag instead. The comment above it indicates its purpose, so it can be re-enabled conditionally:
♻️ Suggested approach
- // Used for generating the test case interdependency JSON file -// AdminTestUtil.generateTestCaseInterDependencies(getGlobalResourcePath() + "/config/testCaseInterDependency.json"); + // Generate test case interdependency JSON file if dependency generation is enabled + if (AdminTestUtil.generateDependency) { + AdminTestUtil.generateTestCaseInterDependencies(getGlobalResourcePath() + "/config/testCaseInterDependency.json"); + }Alternatively, if this code is no longer needed, remove it entirely along with the comment.
...urces/mimoto/LoginFlow/DownloadMosipIssuerCredential/AuthorizationCode/AuthorizationCode.yml
Outdated
Show resolved
Hide resolved
...oginFlow/DownloadMosipIssuerCredential/DownloadIssuerCredential/DownloadIssuerCredential.yml
Outdated
Show resolved
Hide resolved
...ectedIssuerCredentialWithGoogleLogin/AuthorizationCodeSunBirdC/AuthorizationCodeSunBirdC.yml
Outdated
Show resolved
Hide resolved
...otectedIssuerCredentialWithGoogleLogin/DownloadIssuerCredential/DownloadIssuerCredential.yml
Outdated
Show resolved
Hide resolved
|
...otectedIssuerCredentialWithGoogleLogin/DownloadIssuerCredential/DownloadIssuerCredential.yml
Outdated
Show resolved
Hide resolved
Removed an additional dependency from the authorization code configuration. Signed-off-by: Nitin Hegde <[email protected]>
Signed-off-by: Nitin Hegde <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@api-test/src/main/java/io/mosip/testrig/apirig/mimoto/testrunner/MosipTestRunner.java`:
- Around line 109-116: After retrieving the config into the local String
generateDependency in MosipTestRunner, assign the boolean feature flag on
AdminTestUtil so MimotoUtil.isTestCaseValidForTheExecution() sees the correct
value; set AdminTestUtil.generateDependency to true when generateDependency
equals "yes" (case-insensitive) and false otherwise (i.e., map the
"generateDependency" String to the boolean AdminTestUtil.generateDependency
immediately after the config read).
🧹 Nitpick comments (1)
api-test/src/main/java/io/mosip/testrig/apirig/mimoto/utils/MimotoUtil.java (1)
105-109: Redundant null check and boolean comparison style.The
testCaseDTO != nullcheck is unnecessary sincetestCaseDTOis already dereferenced on lines 92-98 without null checks. Also, preferAdminTestUtil.generateDependencyoverAdminTestUtil.generateDependency == truefor boolean comparisons.♻️ Suggested simplification
- // Handle extra workflow dependencies - if (testCaseDTO != null && testCaseDTO.getAdditionalDependencies() != null - && AdminTestUtil.generateDependency == true) { + // Handle extra workflow dependencies + if (testCaseDTO.getAdditionalDependencies() != null + && AdminTestUtil.generateDependency) { addAdditionalDependencies(testCaseDTO); }
api-test/src/main/java/io/mosip/testrig/apirig/mimoto/testrunner/MosipTestRunner.java
Show resolved
Hide resolved
api-test/src/main/java/io/mosip/testrig/apirig/mimoto/testrunner/MosipTestRunner.java
Outdated
Show resolved
Hide resolved
...otectedIssuerCredentialWithGoogleLogin/DownloadIssuerCredential/DownloadIssuerCredential.yml
Outdated
Show resolved
Hide resolved
Signed-off-by: Nitin Hegde <[email protected]>
Signed-off-by: Nitin Hegde <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@api-test/src/main/resources/config/testCaseInterDependency.json`:
- Around line 256-260: The test dependency for TC_Mimoto_AddTrustedVerifier_05
mixes TC_Mimoto_GoogleLoginToken_06 and TC_Mimoto_CreateWallet_23 with
VerifiablePresentationAuthorization_01 (which pulls token_01/wallet_01 flows)
causing mismatched setups; update the dependency list for
TC_Mimoto_AddTrustedVerifier_05 to use the flow-specific VP authorization that
matches token_06/wallet_23 (e.g., replace VerifiablePresentationAuthorization_01
with VerifiablePresentationAuthorization_04) or swap to the correct
token_06-compatible VP variant, and apply the same alignment to the other
occurrences noted (the blocks around the other indices).
- Around line 501-523: Several test case dependency arrays redundantly include
"TC_Mimoto_CreateWallet_01" alongside "TC_Mimoto_UnlockWallet_01" (which already
depends on CreateWallet), so remove the direct "TC_Mimoto_CreateWallet_01"
entries wherever "TC_Mimoto_UnlockWallet_01" appears to avoid duplicate
dependency declarations; update each affected JSON array (e.g., entries like
"TC_Mimoto_AddTrustedVerifier_01",
"TC_Mimoto_DownloadStayProtectedIssuerCredentialWithGoogleLogin_03",
"TC_Mimoto_DownloadStayProtectedIssuerCredentialWithGoogleLogin_01",
"TC_Mimoto_FetchAllCredentials_*",
"TC_Mimoto_VerifiablePresentationAuthorization_*", "TC_Mimoto_ViewCredential_*")
by deleting "TC_Mimoto_CreateWallet_01" while keeping
"TC_Mimoto_UnlockWallet_01" and other dependencies intact.
🧹 Nitpick comments (1)
api-test/src/main/java/io/mosip/testrig/apirig/mimoto/testrunner/MosipTestRunner.java (1)
133-136: Consider adding error handling for dependency generation.The dependency generation call is outside the try-catch block. If
generateTestCaseInterDependencies()throws an exception, it would be unhandled. Since this is at shutdown after tests complete, the impact is minimal, but wrapping in a try-catch would ensure cleaner error reporting.🛡️ Optional: Add defensive error handling
if ("yes".equalsIgnoreCase(generateDependency)) { + try { LOGGER.info("Generating test case inter-dependencies"); AdminTestUtil.generateTestCaseInterDependencies(BaseTestCase.testCaseInterDependencyPath); + } catch (Exception e) { + LOGGER.error("Failed to generate test case inter-dependencies: " + e.getMessage()); + } }
Signed-off-by: Nitin Hegde <[email protected]>
Signed-off-by: Nitin Hegde <[email protected]>
|
2 participants
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.