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

test(registrations): fix flaky test #1023

Merged
merged 1 commit into from
Jan 13, 2025

Conversation

danipran
Copy link
Contributor

test_price_group_list_ordering failed in certain circumstances, e.g. pytest -k order. This PR fixes that issue.

PriceGroupViewSet orders the price groups by description, description is a translated field, and PriceGroupFactory overrides all languages with "<lang> Price Group <seq>", where <lang> is the language code and the <seq> is a growing sequence of numbers. In certain circumstances, this causes the order to become something unexpected, e.g. with the descriptions EN Price Group 9 and EN Price Group 10, the latter price groups would be placed first (because "10" > "9").

Solved the issue by setting the translation directly and making sure that the correct translation is used.

Also refactored the test a bit. I'm not sure if I agree with the test using paging since we're testing whether the order of the price groups is correct or not and it adds unnecessary complexity, but decided to keep it as it is.

Refs: LINK-2210

@danipran danipran marked this pull request as ready for review January 10, 2025 14:32
@danipran danipran requested a review from a team January 10, 2025 14:32
@terovirtanen
Copy link
Contributor

LINKEDEVENTS-API branch is deployed to platta: https://linkedevents-pr1023.api.dev.hel.ninja 🚀🚀🚀

@danipran danipran force-pushed the LINK-2210/fix-price-group-list-ordering-test branch from b69348c to 74c1e4b Compare January 13, 2025 09:22
@danipran danipran requested a review from tuomas777 January 13, 2025 09:32
@terovirtanen
Copy link
Contributor

LINKEDEVENTS-API branch is deployed to platta: https://linkedevents-pr1023.api.dev.hel.ninja 🚀🚀🚀

Copy link
Contributor

@tuomas777 tuomas777 left a comment

Choose a reason for hiding this comment

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

Great 👍 🦜

@danipran danipran merged commit 73f5b51 into main Jan 13, 2025
21 checks passed
@danipran danipran deleted the LINK-2210/fix-price-group-list-ordering-test branch January 13, 2025 12:25
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.

3 participants