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

SFR-2440/search_sort_desc_date #500

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

ayan1229
Copy link
Contributor

@ayan1229 ayan1229 commented Jan 7, 2025

This PR does the following

  • sort search results by descending date order and validates the the language filter selection on the backend

from .utils import assert_response_status

@pytest.mark.parametrize("endpoint, expected_status", [
("/search?query=keyword%3ANASA&sort=date%3Adesc&filter=language%3ASerbian", 200),
Copy link
Contributor

Choose a reason for hiding this comment

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

Recommend using urllib.parse.quote to escape the URL keys just to make it more readable.

Copy link
Contributor

Choose a reason for hiding this comment

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

You can create a separate part of the tuple here for the language filter. Below you can then do

assert item.get('language') == expected_language, f"Unexpected language: {item.get('language')}"

assert response.status_code is not None
assert_response_status(url, response, expected_status)

if expected_status == 200:
Copy link
Contributor

Choose a reason for hiding this comment

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

the assert_response_status above will cause the test to fail if the expected_status is not 200. I think we can remove this if

Comment on lines +23 to +31
previous_date = None
for item in response_json.get('results', []):
current_date = item.get('publication_date')
assert current_date is not None, "Publication date is missing"

if previous_date:
assert current_date >= previous_date, f"Publication dates are not sorted in descending order: {previous_date} > {current_date}"

previous_date = current_date
Copy link
Contributor

@kylevillegas93 kylevillegas93 Jan 7, 2025

Choose a reason for hiding this comment

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

You could simplify this:

publication_dates = [item.get('publication_date') for item in response_json.get('results', [])]
assert all(publication_date is not None for publication_date in publication_dates), f"Some publication dates are missing: {dates}"
assert publication_dates == sorted(publication_dates, reverse=True), f"Dates are not sorted in descending order: {dates}"

assert current_date is not None, "Publication date is missing"

if previous_date:
assert current_date >= previous_date, f"Publication dates are not sorted in descending order: {previous_date} > {current_date}"
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it ascending or descending order?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok probably makes sense to just do the sort order as part of the paramerterization of the test case instead of a separate file

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