Skip to content

Conversation

mhindery
Copy link
Contributor

@mhindery mhindery commented May 1, 2020

No description provided.

@mhindery
Copy link
Contributor Author

mhindery commented May 1, 2020

This fixes the tests to succeed on Python 3.8. There was a new test added in #189 which has the same problems as the others where Python 3.8 has different ordering of attributes.

@peppelinux
Copy link
Member

peppelinux commented May 1, 2020

Thank you @mhindery, it was my turn to fix all the things ;)
9a1d06d

I don't love those huge if, too much hardcoded, and more many tests rely on pySAML2 security capabilities, these should be tested there and in djangosaml2 we should only test that the configuration being well loaded.

A good exercise for the future could be that to write less code as possibile , even in the unit tests.
See you on your next PR

@peppelinux peppelinux closed this May 1, 2020
@mhindery
Copy link
Contributor Author

mhindery commented May 1, 2020

@peppelinux I agree that hardcoding the strings is not ideal, this is more to get things succeeding the way it is setup now so it doesn't block other things. That being said, there are some changes in here which should be changed and are not in master yet:

  • mock is builtin in python 3, so you don't have to install it separately, and should import it as from unittest import mock
  • The tox environments are currently not correct, they should match the environments listed above (just the django part)

@peppelinux
Copy link
Member

Please do a PR on top of the current HEAD, I'll accept in a hurry. Thank @mhindery for wht you're doing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants