Skip to content
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

Include test plan for kyc-match operation #140

Conversation

fernandopradocabrillo
Copy link
Collaborator

What type of PR is this?

Add one of the following kinds:

  • tests

What this PR does / why we need it:

Include kyc-match test plan to be added to the Release r1.2.
This test plan covers all functionality not only the base cases

Which issue(s) this PR fixes:

Fixes #139

Changelog input

 Include KYC-match test plan

@fernandopradocabrillo
Copy link
Collaborator Author

@ToshiWakayama-KDDI @GillesInnov35
I'll be on vacation next week, please take a look and feel free to include or improve whatever you see not correct.
This PR is allowed to be updated by maintainers so there shouldn't be any problem.
You can merge it too and include it as part of r1.2 public release

@GillesInnov35
Copy link
Collaborator

hi Fernando, thanks a lot. This test file proposal covers a a large amount of UC.
I'll take a look next week.
BR Gilles

@GillesInnov35
Copy link
Collaborator

@fernandopradocabrillo , some comments

@KYC_Match_6_success_multiple_optional_parameter_combinations
the 2 fields are nameKanaHankak, nameKanaZenkaku should be added in the list

       And the request body property "$.nameKanaHankak" is set to a valid KanaHankak name
       And the request body property "$.nameKanaZenkaku" is set to a valid KanaZenkaku name

@KYC_Match_6_success_multiple_optional_parameter_combinations
the gender's condition might be completed

And the request body property "$.gender" is set to a valid gender value that belongs to the enumeration (MALE, FEMALE)

@GillesInnov35
Copy link
Collaborator

@KYC_Match_10_invalid_param_combination

       And the response property "$.code" is "KNOW_YOUR_CUSTOMER.INVALID_PARAM_COMBINATION"

I thought that a minimum number of parametrers (2) has been added. In that case the request may be rejected as a invalide request.
If not, does the commonalities section recommends the use of KNOW_YOUR_CUSTOMER.INVALID_PARAM_COMBINATION error code ?
it might be perhaps, a INVALID_REQUEST error code. What do you think about that ?
Thanks
Gilles

@ToshiWakayama-KDDI
Copy link
Collaborator

Hi @GillesInnov35 ,
I am still checking it with our internal team. Sorry for taking time.

- KYC_Match_6: added 'nameKanaHankaku' and 'nameKanaZenkaku'
- KYC_Match_6: completed the sentence of 'gender'
- KYC_Match_7: added note this test is optional
- KYC_Match_12: added note this test is optional
- KYC_Match_13: added note this test is optional
@ToshiWakayama-KDDI
Copy link
Collaborator

Hi @GillesInnov35 , cc @fernandopradocabrillo ,

I have just made some modification to the KYC Match feature file, for the folllowing points, includding Gilles's comments:

  • KYC_Match_6: added 'nameKanaHankaku' and 'nameKanaZenkaku', and completed the 'gender' sentence.
  • KYC_Match_7: added note "#This test scenario is optional, as implementation of 'score' feature is optional to network operators/ API providers."
  • KYC_Match_12: added note "# This test scenario is optional, as idDocument parameter/ Second Level Validation is optional to network operators/ API providers."
  • KYC_Match_13: same as above

Please review the latest feature file, and approve, if it is ok with you.

BR
Toshi

GillesInnov35
GillesInnov35 previously approved these changes Sep 2, 2024
Copy link
Collaborator

@GillesInnov35 GillesInnov35 left a comment

Choose a reason for hiding this comment

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

OK, Thanks

@fernandopradocabrillo
Copy link
Collaborator Author

Hi @ToshiWakayama-KDDI @GillesInnov35 !
Thanks for your reviews! Please don't merge yet, we've been checking this internally and have found a couple of things to update/improve :) I'll leave it updated today

Copy link
Collaborator

@GillesInnov35 GillesInnov35 left a comment

Choose a reason for hiding this comment

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

ok thanks

@ToshiWakayama-KDDI
Copy link
Collaborator

Hi @fernandopradocabrillo ,

Thanks for the revision. Two quick questions/confirmation, please.

  1. You have changed the order of "success multiple opitional parameter combinations" and "success specific property score". Any reasons?
  2. For 400.1 Invalid_phoneNumber, (new) Line 178 reads "Given the request header "Authorisation" is set to a valid acces token from which a valid testing phone number can be deducted". Is it ok? Because, forr other error 400 cases, you have changed to "Given a valid testing phone number supported by the service, identifed by the access token or provided in the request body".

Thanks,
Toshi

@fernandopradocabrillo fernandopradocabrillo merged commit 067bace into camaraproject:main Sep 3, 2024
1 check passed
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.

Include test plan for kyc-match operation
3 participants