Opened 4 years ago

Closed 4 years ago

#6322 task closed fixed (fixed)

Missing tests for NotImplementedError in t.protocols.basic

Reported by: Thijs Triemstra Owned by: Thijs Triemstra
Priority: normal Milestone:
Component: core Keywords:
Cc: Thijs Triemstra Branch: branches/notimplemented-tests-6322
branch-diff, diff-cov, branch-cov, buildbot
Author: thijs

Description

Several methods of classes belonging to twisted.protocols.basic raise NotImplementedError and according to coverage these are not tested.

Change History (7)

comment:1 Changed 4 years ago by Thijs Triemstra

Author: thijs
Branch: branches/notimplemented-tests-6322

(In [37214]) Branching to 'notimplemented-tests-6322'

comment:2 Changed 4 years ago by Thijs Triemstra

(In [37215]) add tests, news file. refs #6322

comment:3 Changed 4 years ago by Thijs Triemstra

Keywords: review added

Added tests and forced a build. Please review.

comment:4 Changed 4 years ago by Tom Prince

Keywords: review removed
Owner: set to Thijs Triemstra
  1. The documentation for IntNTestCaseMixin.NotImplementedError has an incorrect reference.
  2. org as a local variable name? ('proto' would be more descriptive, and consistent with the other tests)
  3. (optional) "When <method> is not overridden in a subclass, calling it raises NotImplemented" maybe

Please commit after addressing the above issues.

comment:5 Changed 4 years ago by Thijs Triemstra

(In [37249]) address review comments. refs #6322

comment:6 in reply to:  4 Changed 4 years ago by Thijs Triemstra

Status: newassigned

Replying to tom.prince:

  1. The documentation for IntNTestCaseMixin.NotImplementedError has an incorrect reference.
  2. org as a local variable name? ('proto' would be more descriptive, and consistent with the other tests)
  3. (optional) "When <method> is not overridden in a subclass, calling it raises NotImplemented" maybe

Please commit after addressing the above issues.

Thanks for the review. I'll merge when the new build is green.

comment:7 Changed 4 years ago by Thijs Triemstra

Resolution: fixed
Status: assignedclosed

(In [37279]) Merge notimplemented-tests-6322: Add tests for the methods that raise NotImplementedError in twisted.protocols.basic.

Author: thijs Reviewer: tom.prince Fixes: #6322

Note: See TracTickets for help on using tickets.