py38 Test fixes, Github actions#186
py38 Test fixes, Github actions#186peppelinux merged 2 commits intoIdentityPython:masterfrom mhindery:github-actions
Conversation
|
Hmm I don't really get why this is showing file changes which come from the previous MR, and are already in master 🤔 |
|
Allright, all clean diff now |
|
I there anything blocking this? |
peppelinux
left a comment
peppelinux
left a comment
There was a problem hiding this comment.
I there anything blocking this?
Yes, the pending review. I don't like that if.
You're a doing a great work @mhindery and I can go ahead and merge your PR as it is but I tried to ask to you if there's some alternative to that if.
Let me know, I'm available to merge this today
|
|
||
| saml_request = params['SAMLRequest'][0] | ||
| expected_request = """<samlp:LogoutRequest xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion" xmlns:samlp="urn:oasis:names:tc:SAML:2.0:protocol" Destination="https://idp.example.com/simplesaml/saml2/idp/SingleLogoutService.php" ID="XXXXXXXXXXXXXXXXXXXXXX" IssueInstant="2010-01-01T00:00:00Z" Reason="" Version="2.0"><saml:Issuer Format="urn:oasis:names:tc:SAML:2.0:nameid-format:entity">http://sp.example.com/saml2/metadata/</saml:Issuer><saml:NameID Format="urn:oasis:names:tc:SAML:2.0:nameid-format:transient" SPNameQualifier="http://sp.example.com/saml2/metadata/">58bcc81ea14700f66aeb707a0eff1360</saml:NameID><samlp:SessionIndex>a0123456789abcdef0123456789abcdef</samlp:SessionIndex></samlp:LogoutRequest>""" | ||
| if PY_VERSION < (3, 8): |
There was a problem hiding this comment.
I'd prefer to not use a specialized if. Could we parse the xml in a elementtree object then dump it? It should have the same behaviour and produce the same dump struncture, with the same elements order, in each environments.
Let mw knoe, this would be the best strategy to handle this kind of problems with every future python releases
| py{35,36,37,38}-django22 | ||
| py{36,37,38}-django30 | ||
| py{36,37,38}-djangomaster | ||
| py{3.6,3.7,3.8}-django{2.2,3.0,master} |
There was a problem hiding this comment.
I also remove py35 in my latest commits, Yes, it's time to do that. Really agree
|
I'll review that aspect by myself, and more I'd like to refactor the unit tests because they should run without a real project setup, this should be embedded in the unit test |
3 participants
With a change in the tests to get them to succeed on Python 3.8
Example tests being run: https://github.com/mhindery/djangosaml2/runs/619919241?check_suite_focus=true