Opened 5 years ago

Closed 5 years ago

#7680 defect closed fixed (fixed)

Improve test stability of t.w.test.ContentDecoderAgentTests.test_existingHeaders

Reported by: Jonathan Ballet Owned by: hawkowl
Priority: normal Milestone:
Component: web Keywords:
Cc: Branch: branches/contentdecoderagent-test-stability-7680
branch-diff, diff-cov, branch-cov, buildbot
Author: hawkowl

Description

The test at https://twistedmatrix.com/trac/browser/trunk/twisted/web/test/test_agent.py?rev=43358#L1941 uses Headers.getAllRawHeaders() which result is case as a list and compared with another list with more than 1 element.

On my machine, this test (sometimes) fails with the following:

twisted.test.test_sslverify.OpenSSLOptions.test_inspectCertificate
===============================================================================
[FAIL]
Traceback (most recent call last):
  File ".../twisted/twisted/web/test/test_agent.py", line 1961, in test_existingHeaders
    ('Accept-Encoding', ['fizz', 'decoder1,decoder2'])])
  File ".../twisted/twisted/trial/_synctest.py", line 447, in assertEqual
    % (msg, pformat(first), pformat(second)))
twisted.trial.unittest.FailTest: not equal:
a = [('Host', ['example.com']),
 ('Accept-Encoding', ['fizz', 'decoder1,decoder2']),
 ('Foo', ['bar'])]
b = [('Host', ['example.com']),
 ('Foo', ['bar']),
 ('Accept-Encoding', ['fizz', 'decoder1,decoder2'])]

The problem comes from the fact that Headers.getAllRawHeaders() doesn't guarantee the order of the data it returns (since internally it iterates over a dictionary) and for some reason, here the expected result doesn't have the same ordering as the actual result.

The getAllRawHeaders API doesn't say anything about the ordering, so I'm assuming it doesn't really care about it, and I'm proposing a simple fix which sorts the result returned before comparing it in the test.

Attachments (1)

7680-1.patch (912 bytes) - added by Jonathan Ballet 5 years ago.

Download all attachments as: .zip

Change History (9)

Changed 5 years ago by Jonathan Ballet

Attachment: 7680-1.patch added

comment:1 Changed 5 years ago by Jonathan Ballet

Keywords: review added

Trac doesn't show it, but I put a .misc topfile in twisted/web/topfile/7680.misc in the patch.

comment:2 Changed 5 years ago by Jonathan Ballet

(for quicker reference, here the implementation of getAllRawHeaders() : http://twistedmatrix.com/trac/browser/tags/releases/twisted-14.0.2/twisted/web/http_headers.py#L237 )

comment:3 Changed 5 years ago by hawkowl

Owner: set to hawkowl

comment:4 Changed 5 years ago by hawkowl

Component: coreweb

comment:5 Changed 5 years ago by hawkowl

Author: hawkowl
Branch: branches/contentdecoderagent-test-stability-7680

(In [43359]) Branching to contentdecoderagent-test-stability-7680.

comment:6 Changed 5 years ago by hawkowl

(In [43360]) applying patch from multani, refs #7680

comment:7 Changed 5 years ago by hawkowl

Keywords: review removed

Some builders are being cranky, but they're unrelated. LGTM.

comment:8 Changed 5 years ago by hawkowl

Resolution: fixed
Status: newclosed

(In [43362]) Merging contentdecoderagent-test-stability-7680: Improve stability of t.w.test.ContentDecoderAgentTests.test_existingHeaders

Author: multani Reviewer: hawkowl Fixes: #7680

Note: See TracTickets for help on using tickets.