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

WIP - Test cases for base64 module #15

Merged
merged 5 commits into from
May 20, 2020

Conversation

akash-suresh
Copy link
Contributor

@Zac-HD I have made an attempt to add a few test cases for the base64 module.

I had a doubt - This particular test case test_b85_encode_with_padding_decode_round_trip, is failing for payload = b'\x00'. I set pad = True while b85encode. The expectation is that b85decode, will implicitly remove the padding, but it fails to decode. Let me know your thoughts.

 def test_b85_encode_with_padding_decode_round_trip(self, payload):
        x = base64.b85encode(payload, True)
        self.assertEqual(payload, base64.b85decode(x))

Copy link
Owner

@Zac-HD Zac-HD left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work @akash-suresh, this is a great start 😁

I think you've got the fundamentals down pat, so the comments below are about how we can take it to the next level.

tests/test_encode_decode.py Outdated Show resolved Hide resolved
tests/test_encode_decode.py Outdated Show resolved Hide resolved
tests/test_encode_decode.py Outdated Show resolved Hide resolved
tests/test_encode_decode.py Outdated Show resolved Hide resolved
tests/test_encode_decode.py Outdated Show resolved Hide resolved
@akash-suresh
Copy link
Contributor Author

Thanks @Zac-HD 😄. I shall look at the comments and make changes accordingly.

- Added arguments casefold, map01 to b32decode
- Added arguments casefold to b16encode & b16decode
- Updated wrapcol argument strategy of a85encode to take 0 more often.
- Added argument pad to a85encode & a85decode
- Added argument pad to b85encode & b85decode
- Fixed usage of pad argument in a85decode & b85decode, by manually adding padding when len(payload)!=4
@Zac-HD Zac-HD merged commit f7402ba into Zac-HD:master May 20, 2020
@Zac-HD
Copy link
Owner

Zac-HD commented May 20, 2020

Thanks @akash-suresh! If you're still interested in writing binascii or plistlib tests, go for it 😄

@akash-suresh
Copy link
Contributor Author

Thanks for merging @Zac-HD . This is my first open source contribution 👻 . I have already started on writing tests for binascii, will raise a PR by the end of today. 😄

@Zac-HD
Copy link
Owner

Zac-HD commented May 20, 2020

Congratulations!

@Zac-HD
Copy link
Owner

Zac-HD commented May 20, 2020

Cross-reference: HypothesisWorks/hypothesis#2430

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