-
Notifications
You must be signed in to change notification settings - Fork 31
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
Test definition location retrieval #229
Conversation
In order to prepare a release candidate added Test Definition for location Retrieval
Update following camaraproject/Commonalities#117 proposal.
Fixed after Cetin review + added a TC to differentiate unauthenticated from unauthorized + added error code for rainy scenario.
Co-authored-by: Jose Luis Urien <[email protected]>
Aligned with change on error code
For expired access token changed code to UNAUTHENTICATED
🦙 MegaLinter status: ✅ SUCCESS
See detailed report in MegaLinter reports |
And the response property "$.code" is "UNSUPPORTED_DEVICE_IDENTIFIERS" | ||
And the response property "$.message" contains a user friendly text | ||
|
||
@location_retrieval_13_device_not_found |
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.
Will this 404 case be possible in three-legged token use cases? The access token would be tied to a valid device already?
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.
As for 400.4 TC this is a borderline TC. See my comment below and I guess we could apply same decision.
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.
There may be situations as @bigludo7 suggests when the access token is valid but the device included in the request does not point to any device.
- Device can NOT be deducted from the access token:
device
NOT included in the request body-> 422 UNIDENTIFIABLE_DEVICEdevice
included in the request body but not pointing to a known device -> 404 DEVICE_NOT_FOUND
|
||
# Other specific 400 errors | ||
|
||
@location_retrieval_400.3_max_age_schema_compliant |
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.
What do you think about one more test case to validate the maxAge minimum value compliance (minimum=60)?
It could either be combined with this test case (@location_retrieval_400.3_max_age_schema_compliant) or it could be a new test case. Examples below.
Combining into one:
@location_retrieval_400.3_max_age_schema_compliant
Scenario: maxAge value does not comply with the schema
Given the request body property "$.device" is set to a valid testing device
And the request body property "maxAge" does not comply with the OAS schema at "/components/schemas/RetrievalLocationRequest"
When the HTTP "POST" request is sent
Then the response status code is 400
And the response property "$.status" is 400
And the response property "$.code" is "INVALID_ARGUMENT"
And the response property "$.message" contains a user friendly text
Examples:
| maxAge |
| 6a0 |
| 59 |
Creating a separate use case:
@location_retrieval_400.4_max_age_min_schema_compliant
Scenario: maxAge value is below the minimum (60) allowed
Given the request body property "$.device" is set to a valid testing device
And the "maxAge" is set to 59
When the HTTP "POST" request is sent
Then the response status code is 400
And the response property "$.status" is 400
And the response property "$.code" is "INVALID_ARGUMENT"
And the response property "$.message" contains a user friendly text
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.
@trehman-gsma we have removed the maxAge limitation. You can omit it or pass it with 0 value as well.
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.
oh! I must have been looking at an older spec then, apologies!
And the response property "$.message" contains a user friendly text | ||
|
||
|
||
@location_retrieval_400.4_required_device_identifier_missing |
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.
Will device remain a mandatory field in the next release?
(Referring to three legged use case where access token is tied to a device)
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.
This is a fair question.
By the generic rule this API will require 3-legs but what if this API is used to track device (not linked to people) for a B2B customer? I tend to think we can have some borderline UC in 2-legs. I prefer to keep it as optional but if we prefer to remove it I understand.
@jlurien WDYT?
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.
If we make device optional this test case would need to be adapted. For 2-legged access, or even for 3-legged when a single device cannot be deducted from the access token, it would be an error, but not 400 INVALID_ARGUMENT (not compliant with schema) but 422 UNIDENTIFIABLE_DEVICE
Thank you @trehman-gsma for the review. |
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.
OK with the clarifications, We may need to adapt some scenarios if we make device opcional and add support for 422 UNIDENTIFIABLE_DEVICE, but we make that changes along with the adaptations in the API specs
And the response property "$.message" contains a user friendly text | ||
|
||
|
||
@location_retrieval_400.4_required_device_identifier_missing |
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.
If we make device optional this test case would need to be adapted. For 2-legged access, or even for 3-legged when a single device cannot be deducted from the access token, it would be an error, but not 400 INVALID_ARGUMENT (not compliant with schema) but 422 UNIDENTIFIABLE_DEVICE
And the response property "$.code" is "UNSUPPORTED_DEVICE_IDENTIFIERS" | ||
And the response property "$.message" contains a user friendly text | ||
|
||
@location_retrieval_13_device_not_found |
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.
There may be situations as @bigludo7 suggests when the access token is valid but the device included in the request does not point to any device.
- Device can NOT be deducted from the access token:
device
NOT included in the request body-> 422 UNIDENTIFIABLE_DEVICEdevice
included in the request body but not pointing to a known device -> 404 DEVICE_NOT_FOUND
What type of PR is this?
Add one of the following kinds:
What this PR does / why we need it:
Provide test definition for Location-retrieval
Which issue(s) this PR fixes:
Fixes #196
Special notes for reviewers:
I got an issue with my previous PR #119 so I have to create a new one :(
Changelog input
Additional documentation
This section can be blank.