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

Allow filter versions within a range #18

Merged
merged 1 commit into from
Oct 29, 2024

Conversation

rebemartinezgr-factorial
Copy link
Contributor

@rebemartinezgr-factorial rebemartinezgr-factorial commented Oct 28, 2024

🚪 Why?

For Public APi automatic testing epic we will need to read a list of Version within a range

🔑 What?

Allow filtering Versions by range using the name.
In Public API we are using dates as names i.e 2024-01-01, but it will work the same with SemVer because it will use alphabet comparison.

@marachimeno marachimeno force-pushed the version_in_a_range branch 2 times, most recently from ac27ab6 to be48993 Compare October 28, 2024 15:37
@rebemartinezgr-factorial rebemartinezgr-factorial requested review from marachimeno, atd and gtrias and removed request for marachimeno October 28, 2024 15:45
@rebemartinezgr-factorial rebemartinezgr-factorial marked this pull request as ready for review October 28, 2024 15:45
@@ -60,7 +60,7 @@

context 'when given a random value' do
it 'does not returns a version' do
expect(subject.find_next('2024-04-01')).to be_nil
expect { subject.find_next('2025-04-01') }.to raise_error(RuntimeError, 'Version \'2025-04-01\' not found')
Copy link
Contributor Author

Choose a reason for hiding this comment

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

it was returning nil because the value exists but was the last one (just like the context in line 55)


context 'with only to' do
it 'filters versions by <= to' do
expect(subject.in_range(nil, '2024-09-20').count).to eq(3)
Copy link
Contributor

Choose a reason for hiding this comment

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

what if subject.in_range(nil, nil)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no filter is applied, so it returns the full list. do you want me to add this case in the spec?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

or would you expect a different behavior?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it would be great to have this case covered since is the only one this test is missing and is one of the possible combinations :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

@rebemartinezgr-factorial rebemartinezgr-factorial merged commit ca4b3ac into main Oct 29, 2024
1 check passed
@rebemartinezgr-factorial rebemartinezgr-factorial deleted the version_in_a_range branch October 29, 2024 09:57
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