Reach 100% coverage (#98)

Author: DarkaMaul

Makefile

@@ -33,8 +33,7 @@ ifneq ($(TESTS),)
 	COV_ARGS :=
 else
 	TEST_ARGS :=
-	#TODO(dm) We need to enforce this but later
-	COV_ARGS := --fail-under 0
+	COV_ARGS := --fail-under 100
 endif
 
 .PHONY: all
@@ -56,7 +55,8 @@ lint: $(VENV)/pyvenv.cfg
 		ruff check
 	cargo fmt --check --manifest-path rust/Cargo.toml
 	cargo fmt --check --manifest-path rust/tsp-asn1/Cargo.toml
-
+	. $(VENV_BIN)/activate && \
+		interrogate -c pyproject.toml .
 
 .PHONY: reformat
 reformat:

pyproject.toml

@@ -22,7 +22,7 @@ dependencies = ["cryptography>=43,<45"]
 [project.optional-dependencies]
 doc = []
 test = ["pytest", "pytest-cov", "pretend", "coverage[toml]"]
-lint = ["ruff >= 0.7,< 0.9"]
+lint = ["ruff >= 0.7,< 0.9", "interrogate"]
 dev = ["rfc3161-client[test,lint,doc]", "maturin>=1.7,<2.0"]
 
 [project.urls]
@@ -52,3 +52,14 @@ select = ["E", "F", "I", "W", "UP", "TCH"]
 
 [tool.coverage.report]
 exclude_also = ["if TYPE_CHECKING:"]
+
+[tool.interrogate]
+# don't enforce documentation coverage for testing, the virtual
+# environment, or the scripts.
+exclude = [
+    ".venv",
+    "test",
+    "scripts",
+]
+ignore-semiprivate = true
+fail-under = 100

src/rfc3161_client/base.py

@@ -49,7 +49,7 @@ def data(self, data: bytes) -> TimestampRequestBuilder:
 
     def hash_algorithm(self, hash_algorihtm: _AllowedHashTypes) -> TimestampRequestBuilder:
         """Set the Hash algorithm used."""
-        if hash_algorihtm not in HashAlgorithm:
+        if not isinstance(hash_algorihtm, HashAlgorithm):
             msg = f"{hash_algorihtm} is not a supported hash."
             raise TypeError(msg)
 

src/rfc3161_client/errors.py

@@ -1,2 +1,7 @@
+"""Errors."""
+
+
 class VerificationError(Exception):
+    """Verification errors are raised when the verification of a Timestamp fails."""
+
     pass

src/rfc3161_client/tsp.py

@@ -1,3 +1,5 @@
+"""Timestamp protocol objects."""
+
 from __future__ import annotations
 
 import abc
@@ -66,6 +68,8 @@ def as_bytes(self) -> bytes:
 
 
 class PKIStatus(enum.IntEnum):
+    """Response status."""
+
     GRANTED = 0
     GRANTED_WITH_MODS = 1
     REJECTION = 2
@@ -75,6 +79,8 @@ class PKIStatus(enum.IntEnum):
 
 
 class TimeStampResponse(metaclass=abc.ABCMeta):
+    """Timestamp response (per RFC 3161)."""
+
     @property
     @abc.abstractmethod
     def status(self) -> int:
@@ -108,6 +114,8 @@ def as_bytes(self) -> bytes:
 
 
 class Accuracy(metaclass=abc.ABCMeta):
+    """Accuracy of the timestamp response."""
+
     @property
     @abc.abstractmethod
     def seconds(self) -> int:
@@ -128,6 +136,8 @@ def micros(self) -> int | None:
 
 
 class TimeStampTokenInfo(metaclass=abc.ABCMeta):
+    """Timestamp token info (per RFC 3161)."""
+
     @property
     @abc.abstractmethod
     def version(self) -> int:
@@ -178,6 +188,8 @@ def name(self) -> cryptography.x509.GeneralName:
 
 
 class SignedData(metaclass=abc.ABCMeta):
+    """Signed data (CMS - RFC 2630)"""
+
     @property
     @abc.abstractmethod
     def version(self) -> int:
@@ -205,6 +217,8 @@ def signer_infos(self) -> set[SignerInfo]:
 
 
 class SignerInfo(metaclass=abc.ABCMeta):
+    """Signer info (RFC 2634)."""
+
     @property
     @abc.abstractmethod
     def version(self) -> int:

src/rfc3161_client/verify.py

@@ -14,6 +14,8 @@
 
 
 class VerifierBuilder:
+    """Builder for a Verifier."""
+
     def __init__(
         self,
         policy_id: cryptography.x509.ObjectIdentifier | None = None,
@@ -121,11 +123,22 @@ def from_request(cls, tsp_request: TimeStampRequest) -> VerifierBuilder:
 
 
 class Verifier(metaclass=abc.ABCMeta):
+    """Verifier.
+
+    This class should not be instantiated directly but through a VerifierBuilder.
+    """
+
     @abc.abstractmethod
-    def verify(self, timestamp_response: TimeStampResponse, hashed_message: bytes) -> bool: ...
+    def verify(self, timestamp_response: TimeStampResponse, hashed_message: bytes) -> bool:
+        """Verify a timestamp response."""
 
 
 class _Verifier(Verifier):
+    """Inner implementation of the Verifier.
+
+    This pattern helps us ensure that the Verifier is never created directly.
+    """
+
     def __init__(
         self,
         policy_id: cryptography.x509.ObjectIdentifier | None,
@@ -190,7 +203,9 @@ def _verify_leaf_certs(self, tsp_response: TimeStampResponse) -> bool:
             leaf_certificate_bytes = next(iter(tsp_response.signed_data.certificates))
             leaf_certificate = cryptography.x509.load_der_x509_certificate(leaf_certificate_bytes)
 
-            if self._tsa_certificate is not None and leaf_certificate != self._tsa_certificate:
+            # Note: The order of comparison is important here since we mock
+            # _tsa_certificate's __ne__ method in tests, rather than leaf_certificate's
+            if self._tsa_certificate is not None and self._tsa_certificate != leaf_certificate:
                 msg = "Embedded certificate does not match the one in the Verification Options."
                 raise VerificationError(msg)
 

test/test_base.py

@@ -7,7 +7,7 @@
 
 
 class TestRequestBuilder:
-    def test_succeeds(self):
+    def test_succeeds(self) -> None:
         message = b"hello"
         request = TimestampRequestBuilder().data(message).build()
         print(request.nonce)
@@ -17,7 +17,7 @@ def test_succeeds(self):
         assert request.nonce is not None
         assert request.policy is None
 
-    def test_data(self):
+    def test_data(self) -> None:
         with pytest.raises(ValueError):
             TimestampRequestBuilder().build()
 
@@ -27,7 +27,7 @@ def test_data(self):
         with pytest.raises(ValueError, match="once"):
             TimestampRequestBuilder().data(b"hello").data(b"world")
 
-    def test_algorithm_sha256(self):
+    def test_algorithm_sha256(self) -> None:
         message = b"random-message"
         request = (
             TimestampRequestBuilder().data(message).hash_algorithm(HashAlgorithm.SHA256).build()
@@ -38,7 +38,7 @@ def test_algorithm_sha256(self):
         digest.update(message)
         assert digest.finalize() == request.message_imprint.message
 
-    def test_algorithm_sha512(self):
+    def test_algorithm_sha512(self) -> None:
         message = b"random-message"
         request = (
             TimestampRequestBuilder().data(message).hash_algorithm(HashAlgorithm.SHA512).build()
@@ -49,15 +49,15 @@ def test_algorithm_sha512(self):
         digest.update(message)
         assert digest.finalize() == request.message_imprint.message
 
-    def test_set_algorithm(self):
-        with pytest.raises(TypeError):
+    def test_set_algorithm(self) -> None:
+        with pytest.raises(TypeError, match="is not a supported hash."):
             TimestampRequestBuilder().hash_algorithm("invalid hash algorihtm")
 
         # Default hash algorithm
         request = TimestampRequestBuilder().data(b"hello").build()
         assert request.message_imprint.hash_algorithm == SHA512_OID
 
-    def test_cert_request(self):
+    def test_cert_request(self) -> None:
         with pytest.raises(TypeError):
             TimestampRequestBuilder().cert_request(cert_request="not valid")
 
@@ -67,7 +67,7 @@ def test_cert_request(self):
         request = TimestampRequestBuilder().cert_request(cert_request=True).data(b"hello").build()
         assert request.cert_req is True
 
-    def test_nonce(self):
+    def test_nonce(self) -> None:
         with pytest.raises(TypeError):
             TimestampRequestBuilder().nonce(nonce="not valid")
 

test/test_rust.py

@@ -4,7 +4,7 @@
 from .common import SHA256_OID, SHA512_OID
 
 
-def test_create_timestamp_request():
+def test_create_timestamp_request() -> None:
     request = create_timestamp_request(
         data=b"hello", nonce=True, cert=False, hash_algorithm=HashAlgorithm.SHA512
     )

test/test_tsp.py

@@ -9,7 +9,7 @@
 
 
 class TestTimestampResponse:
-    def test_parsing_good(self):
+    def test_parsing_good(self) -> None:
         timestamp_response = decode_timestamp_response((_FIXTURE / "response.tsr").read_bytes())
         assert timestamp_response.status == 0
 
@@ -27,7 +27,7 @@ def test_parsing_good(self):
         assert tst_info.accuracy.millis is None
         assert tst_info.accuracy.micros is None
 
-    def test_equality(self):
+    def test_equality(self) -> None:
         timestamp_response = decode_timestamp_response((_FIXTURE / "response.tsr").read_bytes())
         other_response = decode_timestamp_response((_FIXTURE / "other_response.tsr").read_bytes())
 
@@ -36,13 +36,13 @@ def test_equality(self):
             (_FIXTURE / "response.tsr").read_bytes()
         ) == decode_timestamp_response((_FIXTURE / "response.tsr").read_bytes())
 
-    def test_round_trip(self):
+    def test_round_trip(self) -> None:
         timestamp_response = decode_timestamp_response((_FIXTURE / "response.tsr").read_bytes())
         assert timestamp_response == decode_timestamp_response(timestamp_response.as_bytes())
 
 
 class TestTimestampRequest:
-    def test_parsing_good(self):
+    def test_parsing_good(self) -> None:
         timestamp_request: TimeStampRequest = parse_timestamp_request(
             (_FIXTURE / "request.der").read_bytes()
         )
@@ -52,12 +52,12 @@ def test_parsing_good(self):
         assert timestamp_request.policy is None
         assert timestamp_request.message_imprint.message.hex().startswith("9b71d224bd62f3785d96d")
 
-    def test_equality(self):
+    def test_equality(self) -> None:
         timestamp_request = parse_timestamp_request((_FIXTURE / "request.der").read_bytes())
         other_request = parse_timestamp_request((_FIXTURE / "other_request.der").read_bytes())
 
         assert timestamp_request != other_request
 
-    def test_round_trip(self):
+    def test_round_trip(self) -> None:
         timestamp_request = parse_timestamp_request((_FIXTURE / "request.der").read_bytes())
         assert timestamp_request == parse_timestamp_request(timestamp_request.as_bytes())