classification
Title: More test coverage for hmac
Type: enhancement Stage: patch review
Components: Library (Lib), Tests Versions: Python 3.10, Python 3.9, Python 3.8
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: CCLDArjun, eamanu, iritkatriel
Priority: normal Keywords: easy, patch

Created on 2012-03-15 17:13 by packetslave, last changed 2021-06-17 11:49 by eamanu.

Files
File name Uploaded Description Edit
test_hmac.patch packetslave, 2012-03-15 17:13 Old review
test_hmac.patch packetslave, 2012-03-17 04:17
Pull Requests
URL Status Linked Edit
PR 26636 open CCLDArjun, 2021-06-10 05:44
Messages (9)
msg155913 - (view) Author: Brian Landers (packetslave) Date: 2012-03-15 17:13
Adding some tests to non-default code paths in hmac.py to get to 100% coverage.
msg156041 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2012-03-16 14:44
Thanks for the patch.  Could you rewrite it using self.assert* methods instead of asserts?

I’m not an hmac expert, so you may have to wait a bit for another core developer to see and commit the patch.
msg156134 - (view) Author: Brian Landers (packetslave) Date: 2012-03-17 04:17
Updated to use the correct assert* methods.
msg156758 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-03-25 15:17
Some style comments:

- don't use "except: self.fail()", just let the exception pass through

- when monkeypatching hmac.new, use a try...finally block so that the mock doesn't stay in place if the test fails for whatever reason

- it's a bit of a nit, but when using "with warnings.catch_warnings", I would put the asserts on the context manager result outside of the "with" block

- I get a warning in test_noncallable_digestmod, it would be nice to silence it:

/home/antoine/cpython/default/Lib/test/test_hmac.py:254: RuntimeWarning: No block_size attribute on given digest object; Assuming 64.
  h = hmac.HMAC(b"key", b"", hmac)
msg172265 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2012-10-06 23:38
IIRC we also need a signed contributor agreement in order to commit your patch.
msg221970 - (view) Author: Mark Lawrence (BreamoreBoy) * Date: 2014-06-30 17:31
If there isn't a signed contributor agreement I'll put up a new version of the patch.

In msg156758 Antoine said 'don't use "except: self.fail()", just let the exception pass through'.  There are several of these in the existing code.  Should they all be removed or must it be done on a case by case basis?
msg379374 - (view) Author: Irit Katriel (iritkatriel) * (Python committer) Date: 2020-10-22 22:37
The patch is out of date and needs to be refreshed.
msg395484 - (view) Author: Arjun (CCLDArjun) * Date: 2021-06-09 21:43
The only things I think we should add to the current hmac tests are test_update_error_handling and test_with_invalid_msg. 

For test_withnoncallable_digestmod(), hmac itself seems it can no longer be used:

>>> hmac.HMAC(b"gggg", None, "hmac")
<module '_hashlib' from '/Users/arjun/Python/Sources/cpython/build/lib.macosx-11.4-x86_64-3.11-pydebug/_hashlib.cpython-311d-darwin.so'>
using new
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/arjun/Python/Sources/cpython/Lib/hmac.py", line 61, in __init__
    self._init_hmac(key, msg, digestmod)
  File "/Users/arjun/Python/Sources/cpython/Lib/hmac.py", line 69, in _init_hmac
    self._hmac = _hashopenssl.hmac_new(key, msg, digestmod=digestmod)
ValueError: unsupported hash type hmac

For tests test_small_block_size and test_no_block_size, a custom .blocksize cannot be set (changing .block_size has no difference on digest()):

>>> hmac.HMAC(b"gggg", None, "md5").blocksize = 15
<module '_hashlib' from '/Users/arjun/Python/Sources/cpython/build/lib.macosx-11.4-x86_64-3.11-pydebug/_hashlib.cpython-311d-darwin.so'>
using new
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: 'HMAC' object attribute 'blocksize' is read-only

```
class myHMAC(hmac.HMAC):
    blocksize = 1
    def __init__(self, key, msg, digestmod_):
        super().__init__(key, msg, digestmod=digestmod_)

h = myHMAC(b"key", b"", digestmod_="md5")
h.digest() # <- works perfectly fine.
```
Does this sound okay? I can go ahead an implement it if so.
msg395593 - (view) Author: Arjun (CCLDArjun) * Date: 2021-06-11 00:49
would the update invalid test belong in the TestVectorsTestCase class or should I make a new one?
History
Date User Action Args
2021-06-17 11:49:39eamanusetnosy: + eamanu
2021-06-11 00:49:53CCLDArjunsetmessages: + msg395593
2021-06-10 05:44:24CCLDArjunsetstage: needs patch -> patch review
pull_requests: + pull_request25222
2021-06-09 21:43:28CCLDArjunsetnosy: + CCLDArjun, - gregory.p.smith, pitrou, christian.heimes, eric.araujo, packetslave, Vijay.Majagaonkar
messages: + msg395484
2021-04-16 23:08:48iritkatrielsetkeywords: + easy
2020-10-22 22:37:19iritkatrielsetversions: + Python 3.8, Python 3.9, Python 3.10, - Python 2.7, Python 3.2, Python 3.3, Python 3.4
nosy: + iritkatriel

messages: + msg379374

components: + Library (Lib)
stage: patch review -> needs patch
2019-03-16 00:08:57BreamoreBoysetnosy: - BreamoreBoy
2014-06-30 17:31:56BreamoreBoysetnosy: + BreamoreBoy
messages: + msg221970
2012-10-06 23:38:37christian.heimessetnosy: + christian.heimes

messages: + msg172265
versions: + Python 3.4
2012-03-28 16:17:56Vijay.Majagaonkarsetnosy: + Vijay.Majagaonkar
2012-03-25 15:17:44pitrousetnosy: + gregory.p.smith, pitrou

messages: + msg156758
stage: patch review
2012-03-17 04:17:38packetslavesetfiles: + test_hmac.patch

messages: + msg156134
2012-03-16 14:44:04eric.araujosetnosy: + eric.araujo

messages: + msg156041
title: More test coverage for hmac.py -> More test coverage for hmac
2012-03-15 17:13:28packetslavecreate