[Django] #36369: Missing test coverage for FORWARD_PROPERTIES entries

[Django]

#36369: Missing test coverage for FORWARD_PROPERTIES entries
-------------------------------------+-------------------------------------
Reporter: Clifford Gama | Type:
| Cleanup/optimization
Status: new | Component: Database
| layer (models, ORM)
Version: dev | Severity: Normal
Keywords: FORWARD_PROPERTIES, | Triage Stage:
Options | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
possibly also with REVERSE_PROPERTIES

With the exception of `_forward_fields_map` you can remove entries of
field-related properties from `FORWARD_PROPERTIES` and see no test
failures. Discovered during #36207
[https://github.com/django/django/pull/19245#discussion_r2062359974
review]
--
Ticket URL: <https://code.djangoproject.com/ticket/36369>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#36369: Missing test coverage for FORWARD_PROPERTIES entries
-------------------------------------+-------------------------------------

Reporter: Clifford Gama | Owner: (none)
Type: | Status: new
Cleanup/optimization |
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: FORWARD_PROPERTIES, | Triage Stage: Accepted
Options |

Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Jacob Walls):

* stage: Unreviewed => Accepted

Comment:

I think this is worth covering, because at a glance it seems like a bug
that `db_returning_fields` is omitted from the entries. A test would help
clarify.
--
Ticket URL: <https://code.djangoproject.com/ticket/36369#comment:1>

[Django]

#36369: Missing test coverage for FORWARD_PROPERTIES entries
-------------------------------------+-------------------------------------
Reporter: Clifford Gama | Owner: (none)
Type: | Status: new
Cleanup/optimization |
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: FORWARD_PROPERTIES, | Triage Stage: Accepted
Options |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Senthil Kumar):

I'm new and would like to take this on. May I assign it to myself?
Assuming it's not time-sensitive, I’ll aim to add the test cases over the
next few days.
--
Ticket URL: <https://code.djangoproject.com/ticket/36369#comment:2>

[Django]

#36369: Missing test coverage for FORWARD_PROPERTIES entries
-------------------------------------+-------------------------------------

Reporter: Clifford Gama | Owner: Senthil
Type: | Kumar
Cleanup/optimization | Status: assigned

Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: FORWARD_PROPERTIES, | Triage Stage: Accepted
Options |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Jacob Walls):

* owner: (none) => Senthil Kumar
* status: new => assigned

Comment:

Thanks for volunteering. There's some discussion in the linked PR review
with Clifford's initial investigation, if that helps any.
--
Ticket URL: <https://code.djangoproject.com/ticket/36369#comment:3>

[Django]

#36369: Missing test coverage for FORWARD_PROPERTIES entries
-------------------------------------+-------------------------------------
Reporter: Clifford Gama | Owner: Senthil
Type: | Kumar
Cleanup/optimization | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: FORWARD_PROPERTIES, | Triage Stage: Accepted
Options |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Senthil Kumar):

Thanks Jacob. I will be creating a PR soon.
--
Ticket URL: <https://code.djangoproject.com/ticket/36369#comment:4>

[Django]

#36369: Missing test coverage for FORWARD_PROPERTIES entries
-------------------------------------+-------------------------------------
Reporter: Clifford Gama | Owner: Senthil
Type: | Kumar
Cleanup/optimization | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: FORWARD_PROPERTIES, | Triage Stage: Accepted
Options |

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Senthil Kumar):

* has_patch: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/36369#comment:5>

[Django]

#36369: Missing test coverage for FORWARD_PROPERTIES entries
-------------------------------------+-------------------------------------
Reporter: Clifford Gama | Owner: Senthil
Type: | Kumar
Cleanup/optimization | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: FORWARD_PROPERTIES, | Triage Stage: Accepted
Options |
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Jacob Walls):

* needs_better_patch: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/36369#comment:6>

[Django]

#36369: Missing test coverage for FORWARD_PROPERTIES entries
-------------------------------------+-------------------------------------
Reporter: Clifford Gama | Owner: Senthil
Type: | Kumar
Cleanup/optimization | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: FORWARD_PROPERTIES, | Triage Stage: Accepted
Options |
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Senthil Kumar):

* needs_better_patch: 1 => 0

--
Ticket URL: <https://code.djangoproject.com/ticket/36369#comment:7>

[Django]

#36369: Missing test coverage for FORWARD_PROPERTIES entries
-------------------------------------+-------------------------------------
Reporter: Clifford Gama | Owner: Senthil
Type: | Kumar
Cleanup/optimization | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: FORWARD_PROPERTIES, | Triage Stage: Accepted
Options |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Senthil Kumar):

Based on your suggestion, I’ve revised the tests to ensure that the
correct caches are expired. As you mentioned above, I’ve also added a
failing test for db_returning_fields.

I have a question: now that we have a failing test, should I go ahead and
add db_returning_fields under FORWARD_PROPERTIES, or should I leave the
test as-is and mark it as expected to fail? I’d appreciate your guidance
on this.
--
Ticket URL: <https://code.djangoproject.com/ticket/36369#comment:8>

[Django]

#36369: Missing test coverage for FORWARD_PROPERTIES entries
-------------------------------------+-------------------------------------
Reporter: Clifford Gama | Owner: Senthil
Type: | Kumar
Cleanup/optimization | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: FORWARD_PROPERTIES, | Triage Stage: Accepted
Options |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Jacob Walls):

I left a note that it looks like additional properties besides
`db_returning_fields` are worth clearing from the cache. Assuming we go
with a `subTest`-style test instead of writing separate cases for each, I
think fixing the behavior and testing in one commit would be simplest.
--
Ticket URL: <https://code.djangoproject.com/ticket/36369#comment:9>

[Django]

#36369: Missing test coverage for FORWARD_PROPERTIES entries
-------------------------------------+-------------------------------------
Reporter: Clifford Gama | Owner: Senthil
Type: | Kumar
Cleanup/optimization | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: FORWARD_PROPERTIES, | Triage Stage: Accepted
Options |
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Jacob Walls):

* needs_better_patch: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/36369#comment:10>