Skip to content

Conversation

@shanbady
Copy link
Contributor

What are the relevant tickets?

Closes #1772

Description (What does it do?)

Fixes the intermittent test failure in test_similar_resources_endpoint_only_returns_published

How can this be tested?

  1. checkout main
  2. decorate learning_resources/views_test.py::test_similar_resources_endpoint_only_returns_published with @pytest.mark.parametrize('execution_number', range(100)) and run it via pytest learning_resources/views_test.py::test_similar_resources_endpoint_only_returns_published -s -vvv -x to see it fail
  3. checkout this branch and try the same and note it succeeds every time

@shanbady shanbady added the Needs Review An open Pull Request that is ready for review label Oct 29, 2024
@shanbady shanbady marked this pull request as ready for review October 29, 2024 14:18
@ChristopherChudzicki ChristopherChudzicki self-assigned this Oct 30, 2024
Copy link
Contributor

@ChristopherChudzicki ChristopherChudzicki left a comment

Choose a reason for hiding this comment

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

Approving because this works, but:

  1. What was causing the failures before?
  2. Mentioning: I see that get_similar_resources does
            return LearningResource.objects.for_search_serialization().filter(
            id__in=[
                resource.id
                for resource in response.hits
                if resource.id != value_doc["id"] and resource.published
            ]
        )
    
    So I guess this tests the last part of that filter. I think we also exclude unpublished stuff from the search indexes (do we?) . But if I'm correct, no harm in doing it here, too.

@shanbady
Copy link
Contributor Author

shanbady commented Oct 30, 2024

Approving because this works, but:

  1. What was causing the failures before?

  2. Mentioning: I see that get_similar_resources does

            return LearningResource.objects.for_search_serialization().filter(
            id__in=[
                resource.id
                for resource in response.hits
                if resource.id != value_doc["id"] and resource.published
            ]
        )
    

    So I guess this tests the last part of that filter. I think we also exclude unpublished stuff from the search indexes (do we?) . But if I'm correct, no harm in doing it here, too.

The initial issue happened when the document id we pass into the view happens to also be the published=True resource we test in the response. The fix was to ensure that the document id we pass in is not in the list of mocked resources for the response

@shanbady shanbady merged commit 97b2f13 into main Oct 30, 2024
11 checks passed
@shanbady shanbady deleted the shanbady/flaky-test-similar-resources-endpoint branch October 30, 2024 13:43
@odlbot odlbot mentioned this pull request Oct 30, 2024
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Needs Review An open Pull Request that is ready for review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Flaky test learning_resources/views_test.py::test_similar_resources_endpoint_only_returns_published

3 participants