Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix regression in doctest functional helper: sort headers #259

Merged
merged 2 commits into from Apr 12, 2018

Conversation

Projects
None yet
4 participants
@icemac
Copy link
Member

commented Mar 22, 2018

I took this commit from the plonezope4 branch aka #174.
Is it still relevant?

@icemac icemac requested a review from davisagli Mar 22, 2018

@tseaver

This comment has been minimized.

Copy link
Member

commented Mar 22, 2018

Is this a workaround for the bug fixed in #256?

@icemac

This comment has been minimized.

Copy link
Member Author

commented Apr 6, 2018

@tseaver I do not think so. It seems to solve the problem that the HTTP headers in Zope 4 are not sorted and in random order which even changes between test runs. This makes it difficult to write fix doctests which assert the raw HTTP response. See https://github.com/zopefoundation/Products.ExternalEditor/pull/10/files#diff-cd5c80de291398a1c9b1b8bb98e45e4dR40 what I had to do in Products.ExternalEditor which uses this approach heavily.

@MrTango MrTango referenced this pull request Apr 6, 2018

Closed

DO NOT MERGE! Plonezope4 #174

4 of 4 tasks complete

@icemac icemac force-pushed the fix-doctest branch from a8f481e to bf9e981 Apr 6, 2018

Return body as str not bytes in Python 3.
When printing a bytes object the newlines are printed as \n instead of newlines.

Additionally fix the one test broken by the change.
@icemac

This comment has been minimized.

Copy link
Member Author

commented Apr 6, 2018

This PR help a lot to port doctests to Zope 4. (See zopefoundation/Products.ExternalEditor#10 where I could revert all my changes in the doctest regarding the response checks after using this branch.)
I strongly like to get this PR merged to ease the migration to Zope 4.

@icemac icemac merged commit c4a76d9 into master Apr 12, 2018

3 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.002%) to 78.689%
Details

@icemac icemac deleted the fix-doctest branch Apr 12, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.