[Django] #30866: Refactor Error Reporting tests in tests/view_tests/tests/test_debug.py
Django
------------------------------------------------+--------------------------
Reporter: Carlton Gibson | Owner: (none)
Type: Cleanup/optimization | Status: assigned
Component: Error reporting | Version: 2.2
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
------------------------------------------------+--------------------------
The tests in `tests/view_tests/tests/test_debug.py` are all (for want of a
better phrase) ''integration level'' — they test whether particular values
appear in the final output of either HTML, Plain Text, or either version
of the email that's sent to admins.
The trouble with this is that there's lot's of duplicate testing. As is
inevitable, the tests are subtley out of sync. There are gaps in coverage,
for instance in testing whether settings overrides actually work. And
most, for me, looking at resolving the outstanding Error Reporting issues
at the moment, it's **hard** to keep in mind exactly what tests should go
where.
As such, I'd like to refactor the tests. I'm just about to take that on,
but I wanted to confirm that it would be OK before doing so. Coverage will
be maintained. The tests will be clearer and just as effective, if not
more so, and much easier to comprehend/maintain.
General strategy, having looked at it:
All methods go via `ExceptionReporter.get_traceback_data()`, so I want to
reduce all the ''Did the filtering work?'' tests down to that (or a single
output format if testing the `technical_500` view).
Then, the HTML or Plain text formats, use
`ExceptionReporter.get_traceback_data()`. They in turn are used by the
AdminEmailHandler. So not to loose the assurance that the final output is
safe, I'd like to use mocks to assert that `get_traceback_data()` was
actually employed. (Not usually a fan of mocking, but in this case it's
right.)
Additional tests can then focus on format specific questions, such as
whether the correct format is used for Ajax requests, rather than
repeating the filtering logic multiple times.
As I say, primed to go but, wanting to confirm and record the intention.
--
Ticket URL: <https://code.djangoproject.com/ticket/30866>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: Carlton Gibson | Owner: (none)
Cleanup/optimization |
Component: Error reporting | Version: 2.2
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Description changed by Carlton Gibson:
Old description:
New description:
As I say, primed to go but, wanting to confirm and record the intention,
as it's a change in strategy slightly.
--
--
Ticket URL: <https://code.djangoproject.com/ticket/30866#comment:1>
Django
Reporter: Carlton Gibson | Owner: (none)
Cleanup/optimization |
Component: Error reporting | Version: 2.2
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Description changed by Carlton Gibson:
Old description:
> The tests in `tests/view_tests/tests/test_debug.py` are all (for want of
> As I say, primed to go but, wanting to confirm and record the intention,
> as it's a change in strategy slightly.
New description:
The tests in `tests/view_tests/tests/test_debug.py` are all (for want of a
better phrase) ''integration level'' — they test whether particular values
appear in the final output of either HTML, Plain Text, or either version
of the email that's sent to admins.
The trouble with this is that there's lot's of duplicate testing. As is
inevitable, the tests are subtly out of sync. There are gaps in coverage,
for instance in testing whether settings overrides actually work. And
most, for me, looking at resolving the outstanding Error Reporting issues
at the moment, it's **hard** to keep in mind exactly what tests should go
where.
As such, I'd like to refactor the tests. I'm just about to take that on,
but I wanted to confirm that it would be OK before doing so. Coverage will
be maintained. The tests will be clearer and just as effective, if not
more so, and much easier to comprehend/maintain.
General strategy, having looked at it:
All methods go via `ExceptionReporter.get_traceback_data()`, so I want to
reduce all the ''Did the filtering work?'' tests down to that (or a single
output format if testing the `technical_500` view).
Then, the HTML or Plain text formats, use
`ExceptionReporter.get_traceback_data()`. They in turn are used by the
AdminEmailHandler. So not to loose the assurance that the final output is
safe, I'd like to use mocks to assert that `get_traceback_data()` was
actually employed. (Not usually a fan of mocking, but in this case it's
right.)
Additional tests can then focus on format specific questions, such as
whether the correct format is used for Ajax requests, rather than
repeating the filtering logic multiple times.
As I say, primed to go but, wanting to confirm and record the intention,
as it's a change in strategy slightly.
--
--
Ticket URL: <https://code.djangoproject.com/ticket/30866#comment:2>
Django
Reporter: Carlton Gibson | Owner: (none)
Cleanup/optimization |
Component: Error reporting | Version: 2.2
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Description changed by Carlton Gibson:
Old description:
> The tests in `tests/view_tests/tests/test_debug.py` are all (for want of
New description:
The tests in `tests/view_tests/tests/test_debug.py` are all (for want of a
better phrase) ''integration level'' — they test whether particular values
appear in the final output of either HTML, Plain Text, or either version
of the email that's sent to admins.
The trouble with this is that there's lot's of duplicate testing. As is
inevitable, the tests are subtly out of sync. There are gaps in coverage,
for instance in testing whether settings overrides actually work. And
most, for me, looking at resolving the outstanding Error Reporting issues
at the moment, it's **hard** to keep in mind exactly what tests should go
where.
As such, I'd like to refactor the tests. I'm just about to take that on,
but I wanted to confirm that it would be OK before doing so. Coverage will
be maintained. The tests will be clearer and just as effective, if not
more so, and much easier to comprehend/maintain.
General strategy, having looked at it:
All methods go via `ExceptionReporter.get_traceback_data()`, so I want to
reduce all the ''Did the filtering work?'' tests down to that (or a single
output format if testing the `technical_500` view).
Then, the HTML or Plain text formats, use
`ExceptionReporter.get_traceback_data()`. They in turn are used by the
AdminEmailHandler. So not to loose the assurance that the final output is
safe, I'd like to use mocks to assert that `get_traceback_data()` was
actually employed. (Not usually a fan of mocking, but in this case it's
right.)
Additional tests can then focus on format specific questions, such as
whether the correct format is used for Ajax requests, rather than
repeating the filtering logic multiple times.
As I say, primed to go but, wanting to confirm and record the intention,
because it's a change in strategy.
--
--
Ticket URL: <https://code.djangoproject.com/ticket/30866#comment:1>
Django
Reporter: Carlton Gibson | Owner: (none)
Type: Cleanup/optimization | Status: assigned
Component: Error reporting | Version: 2.2
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Claude Paroz):
* stage: Unreviewed => Accepted
Comment:
+1 to more precise testing!
--
Ticket URL: <https://code.djangoproject.com/ticket/30866#comment:2>
Django
Reporter: Carlton Gibson | Owner: (none)
Type: Cleanup/optimization | Status: assigned
Component: Error reporting | Version: 2.2
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Carlton Gibson):
OK, thanks Claude!
--
Ticket URL: <https://code.djangoproject.com/ticket/30866#comment:3>
Django
Reporter: Carlton Gibson | Owner: Carlton
Type: | Gibson
Cleanup/optimization | Status: assigned
Component: Error reporting | Version: 2.2
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Carlton Gibson):
* owner: (none) => Carlton Gibson
--
Ticket URL: <https://code.djangoproject.com/ticket/30866#comment:4>
Django
Reporter: Carlton Gibson | Owner: Carlton
Type: | Gibson
Component: Error reporting | Version: 2.2
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Carlton Gibson):
* status: assigned => closed
* resolution: => wontfix
Comment:
I made a start on this, but got sidelined with other things. Given the
improvements to error reporting for 3.1, and the small number of open
issues on this component, I don't want to give it the extra time to
address this now. (Not currently seeing the ROI.)
I or someone else could come back to it in the future, but pending a PR
I'm going to resolve as wontfix.
--
Ticket URL: <https://code.djangoproject.com/ticket/30866#comment:5>