Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Improve fallback primary key ordering unit tests #716

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

SupImDos
Copy link
Contributor

@SupImDos SupImDos commented Mar 2, 2025

Description

PR #715 introduced default fallback primary key ordering for unordered querysets. This PR adds some extra unit tests, to ensure that pre-existing ordering from various sources is still respected.

Types of Changes

  • Core
  • Bugfix
  • New feature
  • Enhancement/optimization
  • Documentation

Issues Fixed or Closed by This PR

  • N/A

Checklist

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • I have tested the changes and verified that they work and don't break anything (as well as I can manage).

Summary by Sourcery

Tests:

  • Adds tests to verify that fallback primary key ordering respects pre-existing ordering from various sources, including default model ordering, get_queryset ordering, and GraphQL ordering.

Copy link
Contributor

sourcery-ai bot commented Mar 2, 2025

Reviewer's Guide by Sourcery

This PR adds unit tests to ensure that pre-existing ordering from various sources (model Meta, get_queryset, GraphQL) is respected when using the fallback primary key ordering feature introduced in PR #715. It includes new tests and a new Quiz model to test the get_queryset ordering.

No diagrams generated as the changes look simple and do not need a visual representation.

File-Level Changes

Change Details Files
Added tests to verify that pre-existing ordering from various sources is respected when using fallback primary key ordering.
  • Added a test case for models with default ordering defined in Meta.
  • Added a test case for types with a get_queryset method that applies ordering.
  • Added a test case for types that allow ordering via GraphQL.
tests/test_deterministic_ordering.py
tests/projects/schema.py
tests/projects/faker.py
tests/projects/models.py
Added a Quiz model and related factory to test get_queryset ordering.
  • Added a Quiz model with a title and sequence field.
  • Added a QuizFactory for creating Quiz instances.
  • Added a QuizType to the schema with a get_queryset method that orders by title.
  • Added a quiz_list field to the Query type to expose the QuizType.
tests/projects/schema.py
tests/projects/faker.py
tests/projects/models.py

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!
  • Generate a plan of action for an issue: Comment @sourcery-ai plan on
    an issue to generate a plan of action for it.

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey @SupImDos - I've reviewed your changes - here's some feedback:

Overall Comments:

  • Consider adding comments to the tests to explain the expected ordering behavior in each case.
Here's what I looked at during the review
  • 🟢 General issues: all looks good
  • 🟢 Security: all looks good
  • 🟢 Testing: all looks good
  • 🟢 Complexity: all looks good
  • 🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@SupImDos
Copy link
Contributor Author

SupImDos commented Mar 2, 2025

Hi @bellini666, SourceryAI suggested some useful unit tests in the last PR (#715) that I hadn't committed yet (whoops). There's no new functionality here, just some extra tests if you wanted them 😄

@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.13%. Comparing base (25db851) to head (e77ef68).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #716   +/-   ##
=======================================
  Coverage   88.13%   88.13%           
=======================================
  Files          42       42           
  Lines        3853     3853           
=======================================
  Hits         3396     3396           
  Misses        457      457           

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

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants