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

(#3502) Return the PageSize if specified #3547

Merged
merged 3 commits into from
Nov 5, 2024

Conversation

corbob
Copy link
Member

@corbob corbob commented Oct 31, 2024

Description Of Changes

If a page size has been specified, use it. Also emit a warning if it's something other than 30 as there are known issues with some feeds.

Motivation and Context

Make a few things clearer and easier when working with Sleet and other feeds.

Testing

  1. Setup Sleet as per Use the user-provided page size when querying repositories #3502 (comment)
  2. Run choco search -s http://localhost/index.json.
  3. Ensure that the page size is being used in queries to Sleet.
  4. Run choco search -s http://localhost/index.json --page-size 5.
  5. Ensure only 5 results returned along with a warning about the page size differing from 30.
  6. Run choco search -s http://localhost/index.json --page-size 50.
  7. Ensure that 50 results are returned along with a warning about the page size differing from 30.
  8. Run choco search -s http://localhost/index.json --page-size 500.
  9. Ensure that an error is displayed indicating that the page size cannot exceed 100 (this did not change in this PR, is just a soundness check)
  10. Run choco search -s http://localhost/index.json --page-size 100.
  11. Ensure that 100 results are returned along with a warning about the page size differing from 30.

Operating Systems Testing

  • Windows 10 22H2

Change Types Made

  • Bug fix (non-breaking change).
  • Feature / Enhancement (non-breaking change).
  • Breaking change (fix or feature that could cause existing functionality to change).
  • Documentation changes.
  • PowerShell code changes.

Change Checklist

  • Requires a change to the documentation.
  • Documentation has been updated.
  • Tests to cover my changes, have been added.
  • All new and existing tests passed?
  • PowerShell code changes: PowerShell v3 compatibility checked?

Related Issue

@corbob
Copy link
Member Author

corbob commented Oct 31, 2024

This PR is in draft while I formalize the testing and other aspects of it.

Copy link
Member

@vexx32 vexx32 left a comment

Choose a reason for hiding this comment

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

This looks like it works correctly in testing, and the code looks pretty code so far. Nice work!

@corbob corbob marked this pull request as ready for review November 4, 2024 20:11
Copy link
Member

@vexx32 vexx32 left a comment

Choose a reason for hiding this comment

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

Code looks good, but am realising we should definitely add a regression test for this in terms of functionality.

If a page size has been specified, use it. Also emit a warning if it's
something other than 30 as there are known issues with some feeds.
The integration test for PageSize broke because the message was changed
in chocolatey#3507. This updated the test to use the new wording.
Add tests to exercise the messages added for page sizes and ensure they
are behaving as expected.
Copy link
Member

@vexx32 vexx32 left a comment

Choose a reason for hiding this comment

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

That looks good, thanks for sorting this out! 💜

@vexx32 vexx32 merged commit 3b7baf7 into chocolatey:develop Nov 5, 2024
5 checks passed
@corbob corbob deleted the 3502-sleet branch November 5, 2024 17:56
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.

Use the user-provided page size when querying repositories
2 participants