Skip to content

LG-7675: test for Faraday errors that have a nil response#7057

Merged
eileen-nava merged 10 commits intomainfrom
em/7675-test-for-errors-in-int
Oct 4, 2022
Merged

LG-7675: test for Faraday errors that have a nil response#7057
eileen-nava merged 10 commits intomainfrom
em/7675-test-for-errors-in-int

Conversation

@eileen-nava
Copy link
Contributor

@eileen-nava eileen-nava commented Sep 29, 2022

🎫 Ticket

LG-7675

🛠 Summary of changes

(I merged Tomas's work from PR #7049 into my branch. Please let me know if I need to rebase or make other changes for the git history.)

I wrote tests to catch NewRelic errors, which Tomas had already fixed in PR #7049.

📜 Testing Plan

Provide a checklist of steps to confirm the changes.

  • Confirm that automated tests pass

@eileen-nava eileen-nava changed the title test for Faraday errors that have a nil response LG-7675: test for Faraday errors that have a nil response Sep 29, 2022
@eileen-nava eileen-nava marked this pull request as ready for review September 29, 2022 21:36
@eileen-nava eileen-nava requested review from a team and allthesignals September 29, 2022 21:37
}
end

def stub_request_with_timeout_error(*_response)
Copy link
Contributor

Choose a reason for hiding this comment

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

Couple thoughts:

  • The other methods in this file are named specific to the endpoint they're stubbing, and I think we should follow that pattern here
  • If we're not using the argument, do we need it? Many of the other methods here omit it, so I also don't see it being necessary purely out of consistency
Suggested change
def stub_request_with_timeout_error(*_response)
def stub_request_proofing_results_with_timeout_error

@aduth
Copy link
Contributor

aduth commented Sep 30, 2022

While we're here, should we fix up the additional changes recommended in the initial pull request? (e.g. https://github.com/18F/identity-idp/pull/7049/files#r983552375)

@eileen-nava
Copy link
Contributor Author

While we're here, should we fix up the additional changes recommended in the initial pull request? (e.g. https://github.com/18F/identity-idp/pull/7049/files#r983552375)

I think the plan is to do that follow-up in ticket 7706.

@eileen-nava eileen-nava merged commit f239598 into main Oct 4, 2022
@eileen-nava eileen-nava deleted the em/7675-test-for-errors-in-int branch October 4, 2022 13:44
jskinne3 pushed a commit that referenced this pull request Oct 12, 2022
* handle StandardError without response

changelog: Bug Fixes, In-person proofing, Improve job error handling

* write repetitive tests for faraday errors

* dry up tests with shared examples

* change expected message for parsing error

* changelog: Internal, In-person proofing, add tests for faraday errors

* rewrite parsing error message

* refactor tests

* revert changes to usps proofing results job

Co-authored-by: Tomas Apodaca <thomas.apodaca@gsa.gov>
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.

4 participants