Opened 5 years ago

Last modified 5 years ago

#30866 closed Cleanup/optimization

Refactor Error Reporting tests in tests/view_tests/tests/test_debug.py — at Version 1

Reported by: Carlton Gibson Owned by:
Component: Error reporting Version: 2.2
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Carlton Gibson)

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.

Change History (1)

comment:1 by Carlton Gibson, 5 years ago

Description: modified (diff)
Note: See TracTickets for help on using tickets.
Back to Top