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

Add Tests for aes encryption backend #422

Open
wants to merge 345 commits into
base: master
Choose a base branch
from

Conversation

pastorhudson
Copy link
Contributor

This adds two tests:

  • Tests that a string can be encrypted
  • Tests that an encrypted string can be decrypted

I would like to fix the pycrypto vulnerability as mentioned in #385
So I figured the first step is to add test coverage.

skoczen and others added 30 commits October 27, 2017 14:35
chillipeper and others added 24 commits May 3, 2019 14:07
hangouts is dead and Google is the prime suspect
Fix slack pip install version condition (missing comma)
@Ashex
Copy link
Collaborator

Ashex commented Nov 28, 2019

What do you think about including test for exceptions? The unfortunate part is that the code uses the catch-all exception so we don't have a very precise way to test for that.

@pastorhudson
Copy link
Contributor Author

@Ashex I could probably assert that the exception message was written to the log. Would that satisfy the case?

@Ashex
Copy link
Collaborator

Ashex commented Nov 28, 2019

The aim would be to increase test coverage, I think checking that it was written to the log may not increase coverage of the code in question since we want to test for the exception itself being thrown. But do correct me if I'm wrong.

@pastorhudson
Copy link
Contributor Author

I meant if I force an exception by breaking it in the test, and then asserting the log was written it should execute the code.

@Ashex
Copy link
Collaborator

Ashex commented Nov 28, 2019

Okay understood, I think that would be a worthwhile test!

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.