Skip to content

test(pq-key-fingerprint/ts): phase 4 - comprehensive test coverage (ENG-1763)#25

Merged
eacet merged 8 commits intomainfrom
feature/eng-1763
Mar 10, 2026
Merged

test(pq-key-fingerprint/ts): phase 4 - comprehensive test coverage (ENG-1763)#25
eacet merged 8 commits intomainfrom
feature/eng-1763

Conversation

@eacet
Copy link
Copy Markdown
Member

@eacet eacet commented Mar 4, 2026

Summary

Package(s)

Languages

  • TypeScript
  • Rust

Checklist

  • Tests pass for all modified packages
  • Linting/formatting passes (biome check, cargo fmt)
  • Both language implementations are consistent (or noted as follow-up)
  • Package README updated if public API changed
  • No unnecessary dependencies added

Related Issues

@greptile-apps
Copy link
Copy Markdown

greptile-apps bot commented Mar 4, 2026

Greptile Summary

This PR adds phase 4 comprehensive test coverage for the pq-key-fingerprint TypeScript package, introducing two new test files (fingerprint.test.ts and integration.test.ts) and a self-contained test-data directory with ML-KEM-512 key fixtures intentionally mirrored from pq-key-encoder to remove cross-package runtime coupling.

Key changes:

  • fingerprint.test.ts: Covers deterministic hash vectors (SHA-256/384/512 in all output encodings), all public input forms (raw bytes, KeyData object, SPKI, PEM, JWK), error-boundary behaviour (InvalidKeyTypeError, UnsupportedDigestError, InvalidFingerprintInputError, RuntimeCapabilityError), and a beforeEach that restores globalThis.crypto between tests.
  • integration.test.ts: End-to-end test that loads the real ML-KEM-512 fixture in every supported format and asserts all five entrypoints produce an identical fingerprint string.
  • Test fixtures: ml_kem_512_pub.der (SPKI binary) and ml_kem_512_pub.pem added under test-data/test-keys/, with a README explaining the intentional duplication.

Notable observations:

  • The 'enforces error translation contract on all public entrypoints' block in fingerprint.test.ts duplicates coverage already present in the pre-existing contract.test.ts, and the new version has a weaker assertion (it omits the not.toBeInstanceOf(KeyEncoderError) invariant checked by contract.test.ts).
  • FIXTURE_DIR, readDer, and readPem helpers are copy-pasted between fingerprint.test.ts and integration.test.ts; extracting them to a shared tests/helpers.ts would reduce maintenance surface.
  • The manual try/catch + sentinel-throw pattern in the error contract loop is functional but less idiomatic than Bun's expect(...).rejects.toBeInstanceOf(...) matcher.

Confidence Score: 4/5

  • This PR is safe to merge — it is test-only with no production code changes, and the new tests meaningfully expand coverage of the fingerprinting API.
  • Score reflects test-only changes with no risk to production behaviour. Minor deductions for duplicated test helpers between the two new files and an overlapping error-contract test block that provides weaker assertions than the existing contract.test.ts.
  • No files require special attention, though fingerprint.test.ts lines 163–190 should be reconciled with the pre-existing contract.test.ts to avoid diverging assertions.

Important Files Changed

Filename Overview
packages/pq-key-fingerprint/ts/tests/fingerprint.test.ts New comprehensive unit test file covering deterministic hash vectors, input format variants, error handling, and crypto availability — slightly reduced confidence due to overlapping error-contract coverage with the pre-existing contract.test.ts and a weaker assertion in that shared block.
packages/pq-key-fingerprint/ts/tests/integration.test.ts New integration test that verifies all five public entrypoints produce identical fingerprints for the same key loaded from bytes, KeyData object, SPKI/DER, PEM, and JWK — well-structured, but duplicates the readDer/readPem helpers already defined in fingerprint.test.ts.
packages/pq-key-fingerprint/test-data/test-keys/README.md New README documenting the intentional duplication of ML-KEM-512 test fixtures from pq-key-encoder to avoid runtime coupling between packages — clear and appropriate.
packages/pq-key-fingerprint/test-data/test-keys/ml_kem_512_pub.pem New ML-KEM-512 public key PEM fixture, mirrored from pq-key-encoder test data for local test independence — no issues found.
packages/pq-key-fingerprint/test-data/test-keys/ml_kem_512_pub.der New ML-KEM-512 public key DER (SPKI) binary fixture, mirrored from pq-key-encoder test data — no issues found.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    subgraph Inputs["Test Inputs"]
        A["VECTOR_BYTES\n(bytes 0x00–0x1F)"]
        B["ml_kem_512_pub.der\n(SPKI/DER fixture)"]
        C["ml_kem_512_pub.pem\n(PEM fixture)"]
    end

    subgraph Entrypoints["Public API Entrypoints"]
        E1["fingerprintPublicKeyBytes()"]
        E2["fingerprintPublicKey()"]
        E3["fingerprintSPKI()"]
        E4["fingerprintPEM()"]
        E5["fingerprintJWK()"]
    end

    subgraph Internal["Internal Pipeline"]
        N1["normalizePublicKeyInput()"]
        N2["fromSPKI() — pq-key-encoder"]
        N3["fromPEM() — pq-key-encoder"]
        N4["fromJWK() — pq-key-encoder"]
        N5["ensurePublicKeyData()"]
        N6["digestBytes() — WebCrypto subtle.digest"]
        N7["encodeFingerprint()"]
    end

    subgraph Outputs["Fingerprint Result"]
        R1["hex string (default)"]
        R2["base64 string"]
        R3["base64url string"]
        R4["Uint8Array bytes"]
    end

    subgraph Errors["Error Boundary — withErrorBoundary()"]
        ERR1["InvalidKeyTypeError"]
        ERR2["InvalidFingerprintInputError"]
        ERR3["UnsupportedDigestError"]
        ERR4["RuntimeCapabilityError"]
    end

    A --> E1 & E2
    B --> E3
    C --> E4
    B -->|toJWK| E5

    E1 --> N1
    E2 --> N1
    E3 --> N2 --> N5
    E4 --> N3 --> N5
    E5 --> N4 --> N5
    N1 --> N5

    N5 --> N6 --> N7 --> R1 & R2 & R3 & R4

    N5 -->|private key| ERR1
    N1 -->|invalid input| ERR2
    N6 -->|no WebCrypto| ERR4
    N7 -->|bad digest| ERR3
Loading

Last reviewed commit: c691db3

@eacet
Copy link
Copy Markdown
Member Author

eacet commented Mar 10, 2026

@codex review

@chatgpt-codex-connector
Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Keep it up!

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@eacet eacet requested a review from snwfdhmp March 10, 2026 11:29
Copy link
Copy Markdown
Member Author

eacet commented Mar 10, 2026

Merge activity

  • Mar 10, 4:45 PM UTC: A user started a stack merge that includes this pull request via Graphite.
  • Mar 10, 4:52 PM UTC: Graphite rebased this pull request as part of a merge.
  • Mar 10, 4:53 PM UTC: @eacet merged this pull request with Graphite.

@eacet eacet changed the base branch from feature/eng-1762 to graphite-base/25 March 10, 2026 16:48
@eacet eacet changed the base branch from graphite-base/25 to main March 10, 2026 16:50
@eacet eacet force-pushed the feature/eng-1763 branch from 68c252b to 0de47c1 Compare March 10, 2026 16:51
@eacet eacet merged commit 6481b61 into main Mar 10, 2026
6 checks passed
@eacet eacet deleted the feature/eng-1763 branch March 10, 2026 16:53
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.

1 participant