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

Scc 4327 #97

Merged
merged 4 commits into from
Oct 30, 2024
Merged

Scc 4327 #97

merged 4 commits into from
Oct 30, 2024

Conversation

nonword
Copy link
Member

@nonword nonword commented Oct 24, 2024

  • Improve handling fetch errors
  • Improve test coverage around error conditions
  • Replace axios with fetch
  • Generally tighten mocking of nypl-source-mapper requests

https://newyorkpubliclibrary.atlassian.net/browse/SCC-4327

 - Improve handling fetch errors
 - Improve test coverage around error conditions
 - Replace axios with fetch
 - Generally tighten mocking of nypl-source-mapper requests

https://newyorkpubliclibrary.atlassian.net/browse/SCC-4327
expect(callCount).to.equal(1)
expect(mapping.nyplSourceMap).to.nested.include({ 'sierra-nypl.organization': 'nyplOrg:0001' })

// Trigger another instance creation, which will break if another `fetch`
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd love to see this as an explicit assertion rather than implicit in the setup of the nock. Can you replace the callback counter from the original testing setup?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure thing. Added a counter and updated test to make explicit assertions about counts throughout this test.

@nonword nonword merged commit c97d1cd into main Oct 30, 2024
3 checks passed
@nonword nonword deleted the scc-4327 branch October 30, 2024 14:32
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