Skip to content

Conversation

hosaka
Copy link
Contributor

@hosaka hosaka commented Oct 28, 2016

Although SHA256 is tested as a part of builtin_hash test, there was no separate moduhashlib test. This also tests SHA1 algo for supported ports.

@hosaka
Copy link
Contributor Author

hosaka commented Oct 28, 2016

There is a separate sha256.py that I just didn't notice. Can we rename it to uhashlib_sha256.py and add a separate uhashlib_sha1.py?

@dpgeorge
Copy link
Member

You don't need .exp files because these tests also run under CPython.

@hosaka
Copy link
Contributor Author

hosaka commented Oct 28, 2016

My oversight, removed .exp!

@dpgeorge
Copy link
Member

Although SHA256 is tested as a part of builtin_hash test

Can you please expand on this... how is SHA256 tested by tests/basics/builtin_hash.py?

@hosaka
Copy link
Contributor Author

hosaka commented Oct 28, 2016

Can you please expand on this

This is incorrect, I have mistakenly though that builtin_hash.py calls functions from moduhashlib. I didn't notice extmod/sha256.py and made an assumption that builtin_hash uses uhashlib, which is obviously incorrect and comes down to my lack of knowledge of how python works internally :)

@dpgeorge
Copy link
Member

This is incorrect, I have mistakenly though that builtin_hash.py calls functions from moduhashlib

Ok, no problem. Then please edit the commit message to reflect this. Also, please make the rename of sha256.py a separate commit. Thanks!

@hosaka
Copy link
Contributor Author

hosaka commented Oct 28, 2016

All done, thanks Damien!

@hosaka hosaka changed the title tests/extmod/uhashlib: Coverage for SHA1 and SHA256. tests/extmod/uhashlib: Coverage for SHA1. Oct 28, 2016
@pfalcon
Copy link
Contributor

pfalcon commented Oct 28, 2016

Merged, thanks!

tannewt added a commit to tannewt/circuitpython that referenced this pull request Feb 24, 2020
We now correctly set the reason for the unit not being ready and
always start the unit.

Fixes micropython#2567
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.

3 participants