Opened 13 years ago

Closed 13 years ago

Last modified 5 years ago

#3708 defect closed fixed (fixed)

AMP unit tests use object identity comparison where it is inappropriate

Reported by: Jean-Paul Calderone Owned by:
Priority: normal Milestone:
Component: core Keywords: pypy
Cc: Branch: branches/less-object-identity-testing-3708
branch-diff, diff-cov, branch-cov, buildbot
Author: mwh

Description

For example,

    def test_valueTooLong(self):
        """
        Verify that attempting to send value longer than 64k will immediately
        raise an exception.
        """
        c, s, p = connectedServerAndClient()
        L = []
        x = "H" * (0xffff+1)
        tl = self.assertRaises(amp.TooLong, c.sendHello, x)
        p.flush()
        self.failIf(tl.isKey)
        self.failUnless(tl.isLocal)
        self.failUnlessIdentical(tl.keyName, 'hello')
        self.failUnlessIdentical(tl.value, x)
        self.failUnless(str(len(x)) in repr(tl))
        self.failUnless("value" in repr(tl))
        self.failUnless('hello' in repr(tl))

The assertion about keyName is invalid. It passes on CPython because the "hello" in the test and the "hello" in sendHello are part of the same compilation unit, so they end up creating references to the same string object. This is pretty clearly an implementation detail, though.

Attachments (2)

test_amp.py (76.5 KB) - added by cary 13 years ago.
test_amp.patch (506 bytes) - added by cary 13 years ago.

Download all attachments as: .zip

Change History (8)

Changed 13 years ago by cary

Attachment: test_amp.py added

Changed 13 years ago by cary

Attachment: test_amp.patch added

comment:1 Changed 13 years ago by cary

Keywords: review added
Owner: Glyph deleted

Ignore test_amp.py. Tested on cpython 2.5. I don't have pypy setup on my box.

comment:2 Changed 13 years ago by Michael Hudson-Doyle

Author: mwh
Branch: branches/less-object-identity-testing-3708

(In [26555]) Branching to 'less-object-identity-testing-3708'

comment:3 Changed 13 years ago by Michael Hudson-Doyle

(In [26556]) Apply cary's patch.

Refs #3708.

comment:4 Changed 13 years ago by Michael Hudson-Doyle

Resolution: fixed
Status: newclosed

(In [26558]) Merge less-object-identity-testing-3708: Remove inappropriate use of failUnlessIdentical

An AMP test used failUnlessIdentical in a way that relied on the CPython compiler's implicit sharing of identifier-like strings.

Author: cary Reviewer: mwh Fixes: #3708

comment:5 Changed 11 years ago by <automation>

comment:6 Changed 5 years ago by hawkowl

Keywords: review removed

[mass edit] Removing review from closed tickets.

Note: See TracTickets for help on using tickets.