#6265 enhancement closed fixed (fixed)

SafeRepr.test_brokenReprIncludesID

Reported by: tom.prince Owned by: tom.prince
Priority: normal Milestone:
Component: core Keywords: easy
Cc: thijs Branch: branches/improve-SafeRepr.test_brokenReprIncludesID-6265
(diff, github, buildbot, log)
Author: tomprince Launchpad Bug:

Description

In #5544, exarkun suggests that assertion made by
twisted.python.test.test_reflectpy3.SafeRepr.test_brokenReprIncludesID is too weak.

It should probably assert about what exactly the string should look like, rather than just that it has a particular substring.

Attachments (1)

6265.patch (554 bytes) - added by hash14 19 months ago.
Stricter testing of id() in test_brokenReprIncludesID

Download all attachments as: .zip

Change History (9)

comment:1 Changed 20 months ago by thijs

  • Cc thijs added
  • Keywords easy added

Changed 19 months ago by hash14

Stricter testing of id() in test_brokenReprIncludesID

comment:2 Changed 19 months ago by hash14

Note that the patch assumes that className and formatter.name in twisted.python._reflectpy3._safeFormat will always be BrokenType and repr respectively. Not sure if this is a safe assumption, please advise...

comment:3 Changed 19 months ago by hash14

  • Keywords review added

comment:4 Changed 19 months ago by tom.prince

  • Keywords review removed
  • Owner set to tom.prince

Thanks for your contribution.

When calling safe_repr, the formatter is going to be repr and BrokenType is a name coming from the test-suite, so in this test, it will always be that.

  1. There should be spaces around operators like %.
  2. A comment on why only the first line of the repr is looked at would be helpful for people looking at the code in the future.

I'll fix these two things up, and then merge.

comment:5 Changed 19 months ago by tomprince

  • Author set to tomprince
  • Branch set to branches/improve-SafeRepr.test_brokenReprIncludesID-6265

(In [37111]) Branching to improve-SafeRepr.test_brokenReprIncludesID-6265.

comment:6 Changed 19 months ago by tomprince

(In [37112]) Apply patch from hash14.

Refs #6265.

comment:7 Changed 19 months ago by tom.prince

Also, there was a twistedchecker error: String formatting operations should always use a tuple for non-mapping values

comment:8 Changed 19 months ago by tomprince

  • Resolution set to fixed
  • Status changed from new to closed

(In [37116]) Merge improve-SafeRepr.test_brokenReprIncludesID-6265: Assert about the actual format of the repr.

Author: hash14
Reviewers: tom.prince
Fixes: #6265

Note: See TracTickets for help on using tickets.