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

🌱 (catalogd) Validate catalogd api returns jsonl format, cleanup #1720

Merged
merged 1 commit into from
Feb 10, 2025

Conversation

azych
Copy link
Contributor

@azych azych commented Feb 6, 2025

This PR adds additional unit test cases to make sure catalogd web api returns data in valid jsonl format when serving via both /all and /metas endpoints.
Additionally, it:

  • removes unused suite_test.go and
  • fixes a typo in storage/http_preconditions_check.go

Closes #1709

Description

Reviewer Checklist

  • API Go Documentation
  • Tests: Unit Tests (and E2E Tests, if appropriate)
  • Comprehensive Commit Messages
  • Links to related GitHub Issue(s)

@azych azych requested a review from a team as a code owner February 6, 2025 12:51
Copy link

netlify bot commented Feb 6, 2025

Deploy Preview for olmv1 ready!

Name Link
🔨 Latest commit 0f231f5
🔍 Latest deploy log https://app.netlify.com/sites/olmv1/deploys/67a9de0815ea960008a848ff
😎 Deploy Preview https://deploy-preview-1720--olmv1.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

codecov bot commented Feb 6, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 68.24%. Comparing base (d72e551) to head (0f231f5).
Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1720   +/-   ##
=======================================
  Coverage   68.24%   68.24%           
=======================================
  Files          58       58           
  Lines        4988     4988           
=======================================
  Hits         3404     3404           
  Misses       1342     1342           
  Partials      242      242           
Flag Coverage Δ
e2e 52.92% <ø> (ø)
unit 55.45% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@azych azych changed the title 🌱 Validate catalogd api returns jsonl format, cleanup 🌱 (catalogd) Validate catalogd api returns jsonl format, cleanup Feb 6, 2025
perdasilva
perdasilva previously approved these changes Feb 6, 2025
Copy link
Contributor

@perdasilva perdasilva left a comment

Choose a reason for hiding this comment

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

Nice! Thank you!!

grokspawn
grokspawn previously approved these changes Feb 6, 2025
@azych azych dismissed stale reviews from grokspawn and perdasilva via 6889bed February 6, 2025 15:25
@azych azych force-pushed the catalogd-api-misc-test branch from aef513b to 6889bed Compare February 6, 2025 15:25
anik120
anik120 previously requested changes Feb 6, 2025
catalogd/internal/storage/localdir_test.go Outdated Show resolved Hide resolved
catalogd/internal/storage/localdir_test.go Outdated Show resolved Hide resolved
catalogd/internal/storage/localdir_test.go Outdated Show resolved Hide resolved
@azych
Copy link
Contributor Author

azych commented Feb 6, 2025

@anik120 I asked you and @grokspawn if you were going to work on this yesterday before picking it up to avoid situations where we duplicate work, but I guess it couldn't be helped ;)

Feel free to include the rest of testing/rest of the changes from this PR in yours - I'll be (at least) removing the testing layer from here - no need to needlessly multiply and split work if it's already being done elsewhere.

@anik120
Copy link
Contributor

anik120 commented Feb 7, 2025

@azych apologies for the confusion. While that test is already in main at this point, the rest of the changes are still relevant though. I don't have any open PR at the moment, but I'm happy to help with this PR if needed.

This adds additional unit test cases to make sure
catalogd web api returns data in valid jsonl format
both when serving via /all and /metas endpoints.
Additionally, it:
- removes unused suite_test.go and
- fixes a typo in storage/http_preconditions_check.go
@azych azych force-pushed the catalogd-api-misc-test branch from 6889bed to 0f231f5 Compare February 10, 2025 11:07
@grokspawn grokspawn dismissed anik120’s stale review February 10, 2025 14:51

requested changes were made but user is on PTO for a week

@grokspawn grokspawn added this pull request to the merge queue Feb 10, 2025
Merged via the queue into operator-framework:main with commit a9f402b Feb 10, 2025
22 checks passed
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.

catalogd web api unit test misc fixes
4 participants