Skip to content

Conversation

martinlehoux
Copy link

No description provided.

Copy link

codecov bot commented Aug 12, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.13%. Comparing base (9ccb0f9) to head (59b7b83).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #931      +/-   ##
==========================================
- Coverage   88.19%   88.13%   -0.06%     
==========================================
  Files          32       32              
  Lines        1008     1003       -5     
  Branches      105      104       -1     
==========================================
- Hits          889      884       -5     
  Misses        101      101              
  Partials       18       18              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Member

@auvipy auvipy left a comment

Choose a reason for hiding this comment

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

we will need more detail and context before moving forward with this. also tests are necessary

@martinlehoux
Copy link
Author

Hi, sorry if I did not respect some process. I was browsing through this code and saw this inefficiency. Being a refactor, I don't what test I should add, the expected behaviour is exactly the same as before, but removes an extra database query

@auvipy
Copy link
Member

auvipy commented Aug 13, 2025

can you share some screenshots please as starter?

@cclauss
Copy link
Contributor

cclauss commented Aug 13, 2025

Looking at the original code, I would say that the minimum tests should assert the result where:

  1. the happy path is taken (i.e., no exceptions are raised).
  2. cls.DoesNotExist is raised.
  3. MultipleObjectsReturned is raised.

Please use those tests to ensure that identical results are returned with the current code and the proposed code.

@martinlehoux
Copy link
Author

Do you think class TestDuplicatesMixin is not sufficient today? I should improve this one test?

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