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

Fix failing admin tests when testing against Django's main branch #1140

Merged
merged 1 commit into from
Mar 10, 2023

Conversation

ddabble
Copy link
Member

@ddabble ddabble commented Mar 8, 2023

Description

Example of failing tests.

The commit django/django@473283d added a log_entries key to the dict returned by AdminSite.each_context(), with a value of type QuerySet. The cause of the failing tests seems to be that QuerySet hasn't implemented an __eq__() method (see query.py), and since the tests create a new QuerySet instance (when calling the each_context() method) to compare to the arguments passed to mock_render, the instances won't be equal, since they're not the same object.

Related Issue

Motivation and Context

How Has This Been Tested?

Through GitHub Actions.

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • I have run the pre-commit run command to format and lint.
  • 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 added my name and/or github handle to AUTHORS.rst
  • I have added my change to CHANGES.rst
  • All new and existing tests passed.

Example of failing tests: https://github.com/jazzband/django-simple-history/actions/runs/4368637153/jobs/7641472149

The commit django/django@473283d
added a `log_entries` key to the dict returned by `AdminSite.each_context()`, with a value of type `QuerySet`.
The cause of the failing tests seems to be that `QuerySet` hasn't implemented an `__eq__()` method (see
https://github.com/django/django/blob/32d4b61c313be5169137047e9fb3668da20a5d89/django/db/models/query.py#L290),
and since the tests create a new `QuerySet` instance (when calling the `each_context()` method) to compare to
the arguments passed to `mock_render`, the instances won't be equal, since they're not the same object.
@codecov
Copy link

codecov bot commented Mar 8, 2023

Codecov Report

Merging #1140 (e8cc2b2) into master (6a849ce) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1140   +/-   ##
=======================================
  Coverage   97.24%   97.24%           
=======================================
  Files          23       23           
  Lines        1234     1234           
  Branches      200      200           
=======================================
  Hits         1200     1200           
  Misses         16       16           
  Partials       18       18           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@ddabble ddabble merged commit 17ad67c into jazzband:master Mar 10, 2023
@ddabble ddabble deleted the fix/admin-tests-on-djmain branch March 10, 2023 16:48
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.

2 participants