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

Added test to capture edge cases with unsorted count #180

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ezrasingh
Copy link

@ezrasingh ezrasingh commented Aug 9, 2024

Description

This PR introduces a new test to handle edge cases when querying for nearest_n_within. The test is designed to ensure that the nearest_n_within function behaves as expected, particularly with the max_qty and sorted parameters.

Key Changes:

  • Added a new test function can_query_nearest_n_items_unsorted_max_qty in src/float/query/nearest_n_within.rs.
  • The test covers scenarios with both non-zero and zero values for max_qty, verifying that the function returns the correct number of results when sorted or unsorted.

Confirmation of Fix:

This test confirms that the fix introduced in PR #175 for issue #168 works as intended.

However, it also reveals that the function behaves differently when max_qty is set to 0, depending on whether the results are sorted or not. When sorted is true, the function handles max_qty = 0 correctly, but when sorted is false, it fails to return the expected empty result set.

This addition helps to ensure that edge cases are handled correctly and that the nearest_n_within function performs consistently under various conditions.

Copy link
Owner

@sdd sdd left a comment

Choose a reason for hiding this comment

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

Thanks for another great contribution - approved! I'll make sure I get the unreleased changes out today, sorry for the delay.

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