Currently, in Launchpad, multipart package is in version 0.2.4. We wish to
upgrade it to 1.2.1 for it to be compatible with further versions of Python.
But this upgrade throws multiple test suite failures in buildbot.
This package is used only once in the codebase, for parsing an HTTP request in
a single test. As a result, it was expected to be a simple upgrade at first,
which proved incorrect. And weirdly, the failures had nothing to do with that
particular test.
It turns out, Zope also uses multipart in its HTTP parsings, which made it
sneakily depend on multipart from our perspective. This partially explains the
doctest failures we got previously, which exclusively happened on doctest
sections with http() method calls.
The failed tests on buildbot:
xx-bugtask-assignee-widget.rst
xx-filebug-attachments.rst
xx-productseries-review.rst
xx-productseries.rst
An example test (part of xx-productseries.rst):
>>> print(
... http(
... r"""
... POST /firefox/+spec/svg-support/+setproductseries HTTP/1.1
... Authorization: Basic [email protected]:test
... Referer: https://launchpad.test/
... Content-Type: multipart/form-data;
boundary=---------------------------26999413214087432371486976730
...
... -----------------------------26999413214087432371486976730
... Content-Disposition: form-data; name="field.productseries"
...
... 2
... -----------------------------26999413214087432371486976730
... Content-Disposition: form-data;
name="field.productseries-empty-marker"
...
... 1
... -----------------------------26999413214087432371486976730
... Content-Disposition: form-data; name="field.whiteboard"
...
... would be great to have, but has high risk
... -----------------------------26999413214087432371486976730
... Content-Disposition: form-data; name="field.actions.continue"
...
... Continue
... -----------------------------26999413214087432371486976730--
... """
... )
... ) # noqa
HTTP/1.1 303 See Other
...
Content-Length: 0
...
Location: http://.../firefox/+spec/svg-support
...
(Note that the test failures are reproducible in local environments, just
requiring the upgrade to multipart)
The hypothesis: As stated in multipart’s version 1.0.0 changelog, the criteria
for an HTTP request’s formatting has been increased within the package, turning
a lot more cutthroat. As a result, the HTTP requests written with r-string and
doc-string formatting, which was what all the failed doctests did, is
insufficient when it comes to the expected format of multipart.
To test this, I changed a part of one of the doctests (xx-productseries.rst) to
be extremely explicit in its formatting of HTTP requests:
>>> print(
... http(
... "POST /firefox/+spec/svg-support/+setproductseries HTTP/1.1\r\n"
... + "Authorization: Basic [email protected]:test\r\n"
... + "Referer: https://launchpad.test/\r\n";
... + "Content-Type: multipart/form-data;
boundary=---------------------------26999413214087432371486976730\r\n"
... + '\r\n'
... +
"-----------------------------26999413214087432371486976730\r\n"
... + 'Content-Disposition: form-data;
name="field.productseries"\r\n'
... + '\r\n'
... + '2\r\n'
... +
'-----------------------------26999413214087432371486976730\r\n'
... + 'Content-Disposition: form-data;
name="field.productseries-empty-marker"\r\n'
... + '\r\n'
... + '1\r\n'
... +
'-----------------------------26999413214087432371486976730\r\n'
... + 'Content-Disposition: form-data; name="field.whiteboard"\r\n'
... + '\r\n'
... + 'would be great to have, but has high risk\r\n'
... +
'-----------------------------26999413214087432371486976730\r\n'
... + 'Content-Disposition: form-data;
name="field.actions.continue"\r\n'
... + '\r\n'
... + 'Continue\r\n'
... +
'-----------------------------26999413214087432371486976730--\r\n'
... ))
HTTP/1.1 303 See Other
...
Content-Length: 0
...
Location: http://.../firefox/+spec/svg-support
…
With the above format, the failures ceased to exist. It seems these doctest
break the format of HTTP request standard, and the overall tests break because
of it.
(Note that the way the tests break were pretty sneaky themselves, returning a
“200 OK” status code and all. There weren’t any indications that the format
could be broken.)
(Once again, note that most HTTP requests are constructed through function
calls and other similar mechanisms, automatically, with no human input. As a
result, this kind of error could only happen in unittests and doctests like
this which write the HTTP requests within strings themselves (which was stated
within the multipart 1.0.0 changelog as a possible breakpoint))
Thus, my proposed solution is simple, rewrite the parts of doctests which call
the http() method in a way that fit the standards. Either through such an
explicit matter as I showed, or through using other proper HTTP request classes
or methods, which are probably already used in the codebase somewhere.
--
https://code.launchpad.net/~ilkeremrekoc/launchpad/+git/launchpad/+merge/479274
Your team Launchpad code reviewers is requested to review the proposed merge of
~ilkeremrekoc/launchpad:upgrade-multipart-discussion into launchpad:master.
_______________________________________________
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : [email protected]
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help : https://help.launchpad.net/ListHelp
