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

Conditional test case #54

Merged
merged 5 commits into from
Jul 19, 2024
Merged

Conversation

FabrizioMoggio
Copy link
Collaborator

@FabrizioMoggio FabrizioMoggio commented Jul 10, 2024

  • test case for the conditional endpoint and unconditional endpoint with phone number from access token.
  • a very minor update on the unconditional .feature file

What type of PR is this?

  • documentation

What this PR does / why we need it:

to be compliant with commonalities

Which issue(s) this PR fixes:

#35 (comment)
#51
compatible with: #48

- test case for the conditional endpoint.
- a very minor update on the unconditional .feature file
@bigludo7
Copy link
Collaborator

Hello Fabrizio,

Few comments ... most of them my fault as came from the first test definition PR.

  • We should add a Test Case for 403 PERMISSION_DENIED as it is defined in the yaml but not test case. --> my proposal: we can do it in this PR

  • We have test cases for 409, 415 & 429 but these errors are not defined in the yaml - either we add them in the yaml or we remove the tests --> my proposal: Add these errors in our yaml are defined in CAMARA_common.yaml - this should be another PR for the yamls.

  • I've doubt about keeping 503 & 504 error. It means that as implementer you have to shutdown your implem for testing --> my proposal: I'm in favor to remove them in the test definition

@FabrizioMoggio
Copy link
Collaborator Author

Hello Fabrizio,

Few comments ... most of them my fault as came from the first test definition PR.

  • We should add a Test Case for 403 PERMISSION_DENIED as it is defined in the yaml but not test case. --> my proposal: we can do it in this PR
  • We have test cases for 409, 415 & 429 but these errors are not defined in the yaml - either we add them in the yaml or we remove the tests --> my proposal: Add these errors in our yaml are defined in CAMARA_common.yaml - this should be another PR for the yamls.
  • I've doubt about keeping 503 & 504 error. It means that as implementer you have to shutdown your implem for testing --> my proposal: I'm in favor to remove them in the test definition

About 403, you are right. I'm going to add it.

409, 415, 429 are in the current PR rel 0.2.0, that we should approve today :-) (hopefully)

503, 504 I had the same doubt, ok to remove them.

403-PERMISSION_DENIED added

503 and 504 removed

According to: camaraproject#54 (comment)
according to: camaraproject#51

and supporting last commit on : camaraproject#48
@FabrizioMoggio
Copy link
Collaborator Author

please consider: camaraproject/Commonalities#248

Copy link
Collaborator

@bigludo7 bigludo7 left a comment

Choose a reason for hiding this comment

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

LGTM

@FabrizioMoggio FabrizioMoggio merged commit 144f375 into camaraproject:main Jul 19, 2024
1 check passed
@FabrizioMoggio FabrizioMoggio deleted the TestCase branch July 29, 2024 09:44
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.

2 participants