openedx / course-discovery

Service providing access to consolidated course and program metadata
https://edx-discovery.readthedocs.io/en/latest/
GNU Affero General Public License v3.0
57 stars 173 forks source link

test: update query count for exclude_expired_course_run test case #4400

Closed DawoudSheraz closed 3 months ago

DawoudSheraz commented 3 months ago

PROD-4087

Description

test_query_count_exclude_expired_course_run has been flaky for some time. When the test fails, it is due to the query count being greater than expected. Inspecting the queries executed, it appears to have two queries for taxonomy courseskills and programskills respectively. When running the test on local, these queries do not show up. Since skills information is first fetched from the cache and then through the db, it looked like what happens here is that sometimes the skills end up in the cache and sometimes they don’t. Indeed, clearing the cache in the test introduced failure on local as well.

When tweaking the query count locally for test_query_count_exclude_expired_course_run, it seems the expected query count (not including 2 taxonomy skills DB calls ) is 9 whether the exclude_expired_course_run` param is True or False.

When looking into serializer, the query param does not impact the DB calls. Instead, it acts as an additional filter on the retrieved data (course-discovery/course_discovery/apps/course_metadata/search_indexes/serializers/course.py at aaac71cf9411a72870c1abc7c2a3bb8a68516a79 · openedx/course-discovery ).

In False test case, the query count is 9, for True, it is 8. The assertNumQueries adds a default threshold of 2, which checks the query count for False between (7, 11) and (6, 10) for True.

Looking a bit further, it seems the pytest-xdist might be the cause. One of the ex-engineers working on course-discovery filed the same issue Cache + xdist · Issue #527 · pytest-dev/pytest-django , which is still open to this day. There are some suggestions like having unique caches for test cases that can be looked into. But for now, I think changing the query count should be good enough.

image