Opened 5 years ago

Closed 5 years ago

#8015 enhancement closed fixed (fixed)

twisted.words.test.test_irc.CTCPQueryTests.test_ctcpQuery_CLIENTINFO relies on stable dict ordering

Reported by: hawkowl Owned by: hawkowl
Priority: normal Milestone: PyPy-support
Component: words Keywords:
Cc: ralphm Branch: branches/clientinfo-pypy-8015
branch-diff, diff-cov, branch-cov, buildbot
Author: hawkowl


This breaks PyPy, and Twisted tested under tox.

Change History (6)

comment:1 Changed 5 years ago by DefaultCC Plugin

Cc: ralphm added

comment:2 Changed 5 years ago by hawkowl

Author: hawkowl
Branch: branches/clientinfo-pypy-8015

(In [45627]) Branching to clientinfo-pypy-8015.

comment:3 Changed 5 years ago by hawkowl

Keywords: review added

comment:4 Changed 5 years ago by Adi Roiban

Keywords: review removed
Owner: set to hawkowl

Changes look good. I don't see the news fragment :)

Just a minor comment.

Do we really need to sort the response of CLIENTINFO? It might be a waste of CPU just to please a test. I was expecting that this patch will just update the test... for example by spliting the result and using assertItemsEqual

Please review my comments and feel free to merge as you wish ... but I think that the problem is with the test and not the implementation :)


comment:5 Changed 5 years ago by hawkowl

There's a misc topfile -- it shows in the GitHub browser.

I asked Glyph about this, and we've deduced that in situations where dict ordering is unstable, sorting is better -- you can tell the Python implementation from the dict ordering, so we should take mild efforts to make that not obvious, for security reasons.

Will merge.

comment:6 Changed 5 years ago by hawkowl

Resolution: fixed
Status: newclosed

(In [45633]) Merge clientinfo-pypy-8015: Make twisted.words.test.test_irc.CTCPQueryTests.test_ctcpQuery_CLIENTINFO not rely on stable dict ordering

Author: hawkowl Reviewer: adiroiban Fixes: #8015

Note: See TracTickets for help on using tickets.