Make Trial's asynchronous erorr reporting story less subtle

Open
#9,188 3 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
5/5
Estimated time
Over a week
Newbie friendliness
35/100
Issue type
Feature
Clarity
Mostly clear
Activity status
Stale
Tech stack
python
Domain
testing

Research direction

Start with the SynchronousTestCase docstring and Trial's handling of legacy log events with isError=True and a failure attribute. Compare log.err, Logger.error, Treq's RequestSequence documentation, and the addCleanup workaround. Done requires an agreed asynchronous failure API or deprecation path, with tests confirming the intended failure result.

Written by the indexing model from the issue text.

Description

enhancement new priority-normal trial
twm's avatar @twm reported
Trac ID trac#9188
Type enhancement
Created 2017-06-20 02:08:40Z

The SynchronousTestCase docstring has this to say on the subject:

If an error is logged during the test run, the test will fail with an error. See log.err.

This is rather deceptive. What Trial actually does is observe the legacy log system for events with isError=True and a failure attribute. Calling log.err('foo') will not cause a test to fail, nor will calling Logger.error().

I believe that this behavior should be changed to not treat failures specially. (We'd have to deprecate the old behavior first of course, as this would be a backwards-incompatible change.)

Why not just update the documentation? Well, it's useful to have a way to asynchronously report failures, for example in Treq's RequestSequence class. But the naive approach of using {{Logger.error}} directly results in tests that succeed when they should fail. For an example, see Treq's docs.

Alternatively, we could add explicit functionality to Trial to be used for reporting failures asynchronously, instead of overloading logging to do so. Right now user code can report a failure asynchronously by using addCleanup to register a function which raises, but this results in a test result of ERROR rather than FAIL as it occurs during teardown.

Searchable metadata 
trac-id__9188 9188
type__enhancement enhancement
reporter__twm twm
priority__normal normal
milestone__None None
branch__ 
branch_author__ 
status__new new
resolution__None None
component__trial trial
keywords__None None
time__1497924520640537 1497924520640537
changetime__1497980923268625 1497980923268625
version__None None
owner__None None
Dominant language
Python
Stars
6k
Forks
1.2k
Avg merge
2d 10h
Merged PRs (30d)
10

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from twisted/twisted

All issues in twisted/twisted

Similar issues

More Python issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.