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

backend-explicit tests #108

Closed
mattsb42-aws opened this issue Sep 7, 2018 · 4 comments
Closed

backend-explicit tests #108

mattsb42-aws opened this issue Sep 7, 2018 · 4 comments

Comments

@mattsb42-aws
Copy link
Contributor

mattsb42-aws commented Sep 7, 2018

The current test framework loads all cryptographic backends and then tests them all in the same pass.

This has a few unintended effects: it makes it impossible to run the tests without all backends present, it masks failures of some tests that will fail when certain backends are not present (because they use the autoloader), and it makes it easier to forget to enable tests for certain backends.

I've been hacking on this and I have the tests reconfigured to enable testing with isolated backends[1], but as noted above the previous approach was masking tests that currently fail when certain backends are not present.

Assuming that this is a change that would be desirable, what would be your preferred way of handling the changes? In the interest of keeping the PRs small and digestible I would rather not lump the test reworking and any fixes together into the same PR, but pushing only the test rework will make the test suite fail.

[1] https://github.com/mattsb42-aws/python-jose/tree/explicit-dependencies

@mpdavis
Copy link
Owner

mpdavis commented Sep 8, 2018

@mattsb42-aws This is great work. I have had this in the back of my mind for some time.

If it works for you, I could set up a dev branch that we could PR everything into for digestable PRs. Once everything was into the dev bracnh, we could merge that into master.

I created a branch https://github.com/mpdavis/python-jose/tree/backend-explicit-tests

You can PR individual PRs into that branch.

Thanks again.

@mattsb42-aws
Copy link
Contributor Author

Sounds good. #109 created for the initial move.

Related to this, I would like to remove the pyca/cryptography backend's dependency on the ecdsa library. From what I remember looking through it, the only gaps in functionality were a couple small utility functions that it might be better (from my perspective) to simply absorb. The reason for this is that it would allow use of this library without requiring the ecdsa library, which is desirable for us in working to reduce usage of certain libraries.

Once that gap is closed, I would like to see what can be done to remove the dependencies for both ecdsa and rsa when using the pyca/cryptography backend, as neither will be necessary at that point. I toyed with this a couple weeks ago, but couldn't figure out how to have setuptools only install certain dependencies if no extras value is set. This might require a custom setuptools command.

@zejn
Copy link
Collaborator

zejn commented Sep 11, 2018

I was looking for that too when implementing different backends. It appears setuptools still can't deal reliably with this according to pypa/setuptools#1139

@mattsb42-aws
Copy link
Contributor Author

This is resolved with #129

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

No branches or pull requests

3 participants