Skip to content

LG-10983: use mock client to generate http status error response#9244

Merged
dawei-nava merged 4 commits intomainfrom
dwang/LG-10983-mock-client-http-status
Sep 22, 2023
Merged

LG-10983: use mock client to generate http status error response#9244
dawei-nava merged 4 commits intomainfrom
dwang/LG-10983-mock-client-http-status

Conversation

@dawei-nava
Copy link
Contributor

🎫 Ticket

LG-10983

🛠 Summary of changes

The original mock client always generate error from the last http call( of the 3 http call conversation with Acuant AssureID), it's expecting a http response of 2xx in most cases.

This change allows to return non 2xx response at any step of the 3 http calls. Namely 440, 438, 439 for some image metric issue during post front/back image phase, in addition to 5xx for these phases.

Also can trigger 5xx at the last step(get result) as needed, though 4xx codes seems not used according to API documentation.

📜 Testing Plan

Provide a checklist of steps to confirm the changes.

  • Step 1: Enter doc auth flow
  • Step 2: Stop at the image upload step
  • Step 3: Test with example yaml file like following

Front side 440 error

http_status:
  front: 440

Back side 438 error

http_status:
  back: 438

Get result 500 error

http_status:
  result: 500

@dawei-nava dawei-nava requested review from a team and amirbey and removed request for a team September 20, 2023 14:45
@dawei-nava dawei-nava force-pushed the dwang/LG-10983-mock-client-http-status branch from a64f90d to 26c47db Compare September 20, 2023 15:28
@dawei-nava dawei-nava marked this pull request as ready for review September 20, 2023 18:00
Copy link
Contributor

@charleyf charleyf left a comment

Choose a reason for hiding this comment

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

I don't fully understand the why of this PR, but approving b/c it's entirely mocks and I think I'm hitting the limits of my own knowledge.

@@ -7,7 +7,8 @@
expect(
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure about this, the test name doesn't seem to match what you're testing anymore.

end

describe 'generate response for failure indicating http status' do
it 'generate network error response for status 500 when post image' do
Copy link
Contributor

Choose a reason for hiding this comment

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

When check out this branch and try the yml files as described in the PR testing plan, I don't see the errors I'd expect. Instead of the technical difficulties message, I see:

440: Your image size is too large or too small. Please add images of your ID that are about 2025 x 1275 pixels.
438: The image file that you added is not supported. Please take new photos of your ID and try again.
500: Try taking new pictures.

Maybe I'm misunderstanding and this is expected, but I thought I'd see the network_error of We are having technical difficulties on our end. Please try to submit your images again later.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm looking again at your test though, maybe I'm wrong and this is expected, as in line 221 of this file.

@dawei-nava dawei-nava merged commit e9ee332 into main Sep 22, 2023
@dawei-nava dawei-nava deleted the dwang/LG-10983-mock-client-http-status branch September 22, 2023 12:30
@solipet solipet mentioned this pull request Sep 26, 2023
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.

3 participants