Opened 6 weeks ago
Closed 4 weeks ago
#36038 closed Cleanup/optimization (wontfix)
Added a test case for the display_for_field function when a FileField is passed.
Reported by: | Antoliny | Owned by: | Antoliny |
---|---|---|---|
Component: | contrib.admin | Version: | dev |
Severity: | Normal | Keywords: | display_for_field, FileFIeld |
Cc: | Triage Stage: | Accepted | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | yes |
Easy pickings: | no | UI/UX: | no |
Description (last modified by )
def display_for_field(value, field, empty_value_display): from django.contrib.admin.templatetags.admin_list import _boolean_icon ... elif isinstance(field, (models.IntegerField, models.FloatField)): return formats.number_format(value) elif isinstance(field, models.FileField) and value: return format_html('<a href="{}">{}</a>', value.url, value) ...
There is no test case for the display_for_field
function when a FileField
object is passed. --> admin_utils test code
When the FileField
handling was added in the above function, I found that the test was conducted indirectly through the FileWidget
readonly attribute. --> test code
However, I believe it would be better to have a direct test case for when a FileField
is passed, considering that display_for_field
function is globally accessible.
Change History (9)
comment:1 by , 6 weeks ago
Owner: | set to |
---|
comment:2 by , 6 weeks ago
Description: | modified (diff) |
---|
comment:3 by , 6 weeks ago
Description: | modified (diff) |
---|
comment:4 by , 6 weeks ago
Component: | Utilities → contrib.admin |
---|---|
Triage Stage: | Unreviewed → Accepted |
Version: | 5.1 → dev |
comment:6 by , 6 weeks ago
Has patch: | set |
---|
comment:7 by , 5 weeks ago
You're right that this is already covered by existing tests and this PR would add double cover.
The existing assert is "strong" enough that changes to display_for_field
for FileField would make the existing test fail.
I would prefer we add tests where we are missing coverage or in parallel with a ticket for which the tests add value. Otherwise, it feels unnecessary
comment:8 by , 5 weeks ago
Patch needs improvement: | set |
---|
comment:9 by , 4 weeks ago
Resolution: | → wontfix |
---|---|
Status: | assigned → closed |
Thank you Antoliny! Missing tests are always welcomed.