django
django copied to clipboard
Fixed #35728 -- Computed error messages in assertions only on test failures.
Trac ticket number
ticket-35728
Branch description
There are two main approaches used.
- Return early if the assertion will pass; if not compute error messages and pass them on to unittest's assertions. Passing on to unittest's assertions has the benefit of sometimes providing more helpful error messages, e.g. when asserting sequence equality, the difference is shown in the report. But sometimes this is just prepends
False is not trueto an error message. ~~2. Compute the error message in the failing conditional branch, forself.fail(msg).~~
A ~~additional~~ (now removed) commit db0cfda3a110392a99f31b54125b38ba2646a90d makes sure that we use __contains__ for checking existence of an element in HTML assertions. This has the benefit of using Element._count(elem, count=False) under the hood, and short-circuiting when a single match is found.
from django.test.html import parse_html
parsed_needle = parse_html("<span>Hello</span>")
parsed_haystack = parse_html(
"<div>" + ("<span>Other</span>" * 100) +
"<span>Hello</span>" * 2 +
("<span>Other</span>" * 100) + "</div>"
)
%timeit parsed_needle in parsed_haystack
%timeit parsed_haystack.count(parsed_needle) # Double the time.
Checklist
- [x] This PR targets the
mainbranch. - [x] The commit message is written in past tense, mentions the ticket number, and ends with a period.
- [x] I have checked the "Has patch" ticket flag in the Trac system.
- [x] I have added or updated relevant tests.
- [ ] I have added or updated relevant docs, including release notes if applicable.
- [ ] I have attached screenshots in both light and dark modes for any UI changes.
Hello @adamchainz! Should I extend the patch for the other assertions as well, e.g. assertTemplateUsed() / assertRedirect() where computing the messages does not seem particularly expensive?
Hello @adamchainz! Should I extend the patch for the other assertions as well, e.g.
assertTemplateUsed()/assertRedirect()where computing the messages does not seem particularly expensive?
No, I think those are safe.
I took a stab at adding benchmarks in https://github.com/django/django-asv/pull/91/.