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.

Sign up for GitHub

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

Jump to bottom

Unit tests for registry clients #32

Merged
merged 20 commits into from
Aug 7, 2024
Merged

Unit tests for registry clients #32

merged 20 commits into from
Aug 7, 2024

Conversation

KaelynJefferson
Copy link
Collaborator

This PR includes unit tests for clients in the Registry folder and some modifications to their corresponding classes:

  • FHIRRegistryClient
  • NPMRegistryClient
  • DefaultRegistryClient
  • RedundantRegistryClient

To test: jest/npm

Related Issue: n/a

cmoesel
cmoesel requested changes Jul 19, 2024
View reviewed changes
Copy link
Member

@cmoesel cmoesel left a comment

Choose a reason for hiding this comment

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

Nice job, Kaelyn. There's some really good stuff in here! I've made some comments and suggestions to help tighten it up and make it even better! Let me know if you have any questions...

test/cache/DiskBasedPackageCache.test.ts Show resolved Hide resolved
test/cache/DiskBasedPackageCache.test.ts Show resolved Hide resolved
test/registry/DefaultRegistryClient.test.ts Outdated Show resolved Hide resolved
test/registry/RedundantRegistryClient.test.ts Outdated Show resolved Hide resolved
test/registry/FHIRRegistryClient.test.ts Outdated Show resolved Hide resolved
test/registry/FHIRRegistryClient.test.ts Outdated Show resolved Hide resolved
test/registry/FHIRRegistryClient.test.ts Outdated Show resolved Hide resolved
test/registry/FHIRRegistryClient.test.ts Outdated Show resolved Hide resolved
test/registry/FHIRRegistryClient.test.ts Outdated Show resolved Hide resolved
test/registry/NPMRegistryClient.test.ts Show resolved Hide resolved
cmoesel
cmoesel requested changes Jul 26, 2024
View reviewed changes
Copy link
Member

@cmoesel cmoesel left a comment

Choose a reason for hiding this comment

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

Thank you, Kaelyn. This is coming along nicely! I just have a few more small suggestions!

src/registry/registryClientHelper.ts Outdated Show resolved Hide resolved
test/registry/FHIRRegistryClient.test.ts Outdated Show resolved Hide resolved
test/registry/FHIRRegistryClient.test.ts Outdated Show resolved Hide resolved
test/registry/NPMRegistryClient.test.ts Outdated Show resolved Hide resolved
cmoesel
cmoesel requested changes Aug 7, 2024
View reviewed changes
Copy link
Member

@cmoesel cmoesel left a comment

Choose a reason for hiding this comment

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

I'm sure this feels like the never-ending PR. I'm sorry! I left one last comment that should be easy to address! Once you've done that, we're golden!

test/registry/FHIRRegistryClient.test.ts Outdated Show resolved Hide resolved
cmoesel
cmoesel approved these changes Aug 7, 2024
View reviewed changes
Copy link
Member

@cmoesel cmoesel left a comment

Choose a reason for hiding this comment

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

This is a really comprehensive set of tests that will give us confidence as we continue to develop FPL. Thank you for covering all the edge cases and thinking through this so thoroughly!

@cmoesel cmoesel merged commit 993e036 into new-fpl Aug 7, 2024
14 checks passed
@cmoesel cmoesel deleted the new-fpl-tests branch August 7, 2024 17:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cmoesel cmoesel cmoesel approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@KaelynJefferson @cmoesel