chore(test): add vitest v8 coverage with 95% perFile gate by 0xisk · Pull Request #527 · OpenZeppelin/compact-contracts

0xisk · 2026-05-28T12:45:05Z

Types of changes

What types of changes does your code introduce to OpenZeppelin Midnight Contracts?
Put an `` in the boxes that apply

Bugfix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
Documentation Update (if none of the other choices apply)

Fixes https://github.com/OpenZeppelin/compact-contracts/issues/443\

In general this PR increases the testing coverage as possible.
\

PR Checklist

I have read the Contributing Guide
I have added tests that prove my fix is effective or that my feature works. See GUIDELINES.md#testing for more information.
I have added documentation for new methods or changes to existing method behavior. See GUIDELINES.md#documentation for more information.
CI Workflows Are Passing

Further comments

If this is a relatively large or complex change, kick off the discussion by explaining why you chose the solution you did and what alternatives you considered, etc...

Summary by CodeRabbit

Tests
- Added comprehensive test coverage for simulator wiring and private state helpers across core contracts.
- Added coverage verification for public state exposure and secret key handling.
Chores
- Added new test:coverage npm script for coverage reporting.
- Updated Vitest to version 4.1.7 with enhanced coverage configuration.
- Implemented strict per-file coverage thresholds (95%) for improved code quality standards.

Adds @vitest/coverage-v8, a test:coverage script, and the coverage block in vitest.config.ts mirroring the feat/forwarder reference. Covers TS witnesses and simulators; compactc-generated artifact JS is instrumented but dropped after source-map remap (.compact source-map granularity is unreliable for branch attribution).

…to 100% - Adds the three literal tests from the PR #420 review snippets: - `_grantRole should independently check initialization` - `_revokeRole should independently check initialization` - `when revoking a role from an unauthorized accountId that was never granted` - Derives UNAUTHORIZED_ACCOUNT_ID from the existing UNAUTHORIZED_SK. - Adds tests for the SAC simulator's privateState helpers (getCurrentSecretKey, getCommitmentPathWithFindForLeaf, getCommitmentPathWithWitnessImpl), lifting ShieldedAccessControlSimulator.ts to 100% lines/branches/funcs/stmts. Closes #443.

Covers the three previously-untested code paths: the `ledgerExtractor` callback (asserted to return the expected empty `{}` ledger), and both branches of `getCurrentSecretKey` (happy path after `injectSecretKey`, and the defensive `Missing secret key` throw when the secret key field is undefined on the private state).

Covers the previously-untested simulator wiring (`defaultPrivateState` fallback when constructed without options, `ledgerExtractor` returning the expected empty `{}` ledger) and both branches of `getCurrentSecretKey`.

Adds a simulator-wiring test that exercises `getPublicState` and asserts the deterministic `Pausable__isPaused` value before and after pausing, covering the previously-untested `ledgerExtractor` callback.

Adds simulator-wiring and privateState-helper tests that exercise the previously-untested `ledgerExtractor` callback (asserting the expected empty `{}` ledger) and both branches of `getCurrentSecretKey`.

Adds simulator-wiring and privateState-helper tests covering the previously-untested `ledgerExtractor` callback (asserting the deterministic `MultiToken__balances` map starts empty) and both branches of `getCurrentSecretKey`.

Drops `excludeAfterRemap: true` and the `.compact` exclude so v8's source-map remap routes artifact-JS coverage back to the original `.compact` source. The report now measures real circuit-level test thoroughness per module rather than only the TS wrapper layer. Note: compactc source maps remain function-entry-granularity, so branch attribution on `.compact` lines can still be approximate. Upstream: LFDT-Minokawa/compact#465

Adds wiring + privateState-helper tests covering the `ledgerExtractor` callback (asserting an empty `{}` public ledger) and both branches of `getCurrentSecretKey`.

Adds a wiring test that exercises `getPublicState`, asserting the expected empty `{}` public ledger and covering the previously-untested `ledgerExtractor` callback.

Scope the 95% perFile gate to `**/*.ts` (simulator wrappers + witnesses) via a per-pattern threshold. `.compact` coverage stays surfaced in the HTML / text report for visibility but is not gated. Background: compactc emits function-entry-granularity source maps and v8 ends up attributing uncovered functions to `pragma language_version` lines, uncovered statements to doc comments, uncovered branches to ledger declaration lines with no `if` statement, and even surfaces "uncovered" line numbers past EOF. No test additions can move those metrics, so any threshold on `.compact` is fragile noise. Tracking upstream: LFDT-Minokawa/compact#465

coderabbitai · 2026-05-28T12:48:16Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 72baba09-c933-4a1f-a9d3-44ddfb7a5da2

📥 Commits

Reviewing files that changed from the base of the PR and between ed68e80 and 8ed8db0.

⛔ Files ignored due to path filters (1)

yarn.lock is excluded by !**/yarn.lock, !**/*.lock

📒 Files selected for processing (10)

contracts/package.json
contracts/src/access/test/AccessControl.test.ts
contracts/src/access/test/Ownable.test.ts
contracts/src/access/test/ShieldedAccessControl.test.ts
contracts/src/security/test/Pausable.test.ts
contracts/src/token/test/FungibleToken.test.ts
contracts/src/token/test/MultiToken.test.ts
contracts/src/token/test/nonFungibleToken.test.ts
contracts/src/utils/test/utils.test.ts
contracts/vitest.config.ts

Walkthrough

This PR extends test coverage across the contract test suite to verify simulator initialization, public state exposure, and private state handling. Vitest is configured with v8 coverage reporting at 95% thresholds, while test suites systematically add simulator wiring and privateState helper tests across all contract modules.

Changes

Test Coverage & Simulator Verification

Layer / File(s)	Summary
Coverage infrastructure setup `contracts/package.json`, `contracts/vitest.config.ts`	Adds `test:coverage` npm script, bumps vitest to `^4.1.7`, and configures v8 coverage provider with text/HTML reporters, artifact inclusion globs, and 95% per-file thresholds.
AccessControl module test coverage `contracts/src/access/test/AccessControl.test.ts`, `contracts/src/access/test/Ownable.test.ts`, `contracts/src/access/test/ShieldedAccessControl.test.ts`	AccessControl and Ownable gain simulator wiring and privateState helpers tests verifying secret-key retrieval and missing-key errors. ShieldedAccessControl adds initialization tests for `_grantRole`/`_revokeRole`, a revocation behavior test for unauthorized accounts, and comprehensive privateState helpers covering secret-key retrieval and Merkle-path behavior for empty/populated commitment states.
Token contract test coverage `contracts/src/token/test/FungibleToken.test.ts`, `contracts/src/token/test/MultiToken.test.ts`, `contracts/src/token/test/nonFungibleToken.test.ts`	All three token contracts gain simulator wiring tests verifying empty public ledger state and privateState helpers confirming getCurrentSecretKey() returns injected keys and throws on missing secrets.
Pausable and utility test coverage `contracts/src/security/test/Pausable.test.ts`, `contracts/src/utils/test/utils.test.ts`	Pausable adds simulator wiring verifying `Pausable__isPaused` state transitions; utils adds simulator wiring confirming getPublicState() returns empty object.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

OpenZeppelin/compact-contracts#444: Refactors AccessControl to introduce wit_AccessControlSK-driven identity and typed AccessControlPrivateState.secretKey with simulator helpers, which this PR now covers with tests.
OpenZeppelin/compact-contracts#466: Introduces pk→sk Ownable refactor with secret-key-based ownership wiring that this PR's Ownable test coverage directly validates.
OpenZeppelin/compact-contracts#483: Refactors MultiToken to introduce typed MultiTokenPrivateState and witness behavior that this PR's MultiToken simulator and privateState tests now exercise.

Poem

🐰 With whiskers twitching, we weave tests anew,
Coverage glowing in values true,
Secrets kept safe in each private state,
Simulators wired—never too late! 🧪✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The PR title accurately summarizes the main change: adding Vitest v8 coverage with a 95% per-file threshold gate.
Linked Issues check	✅ Passed	The PR fully addresses issue `#443` by adding comprehensive tests for ShieldedAccessControl and multiple simulators, raising coverage across access control and token modules as intended.
Out of Scope Changes check	✅ Passed	All changes directly support the PR objectives: coverage tooling setup, test additions for simulators/helpers, and configuration updates are all within scope of the coverage initiative.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch chore/test-coverage

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint skipped: no ESLint configuration detected in root package.json. To enable, add eslint to devDependencies.

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai · 2026-05-28T12:51:54Z

Actionable comments posted: 0

pepebndc

Approved

0xisk added 11 commits May 28, 2026 13:21

test(access): lift OwnableSimulator to 100% coverage

a7cd8de

Covers the previously-untested simulator wiring (`defaultPrivateState` fallback when constructed without options, `ledgerExtractor` returning the expected empty `{}` ledger) and both branches of `getCurrentSecretKey`.

test(security): lift PausableSimulator to 100% coverage

b124487

Adds a simulator-wiring test that exercises `getPublicState` and asserts the deterministic `Pausable__isPaused` value before and after pausing, covering the previously-untested `ledgerExtractor` callback.

test(token): lift FungibleTokenSimulator to 100% coverage

2f35de6

Adds simulator-wiring and privateState-helper tests that exercise the previously-untested `ledgerExtractor` callback (asserting the expected empty `{}` ledger) and both branches of `getCurrentSecretKey`.

test(token): lift MultiTokenSimulator to 100% coverage

0627985

Adds simulator-wiring and privateState-helper tests covering the previously-untested `ledgerExtractor` callback (asserting the deterministic `MultiToken__balances` map starts empty) and both branches of `getCurrentSecretKey`.

test(token): lift NonFungibleTokenSimulator to 100% coverage

ab4ca33

Adds wiring + privateState-helper tests covering the `ledgerExtractor` callback (asserting an empty `{}` public ledger) and both branches of `getCurrentSecretKey`.

test(utils): lift UtilsSimulator to 100% coverage

665d204

Adds a wiring test that exercises `getPublicState`, asserting the expected empty `{}` public ledger and covering the previously-untested `ledgerExtractor` callback.

0xisk requested review from a team as code owners May 28, 2026 12:45

pepebndc approved these changes May 29, 2026

View reviewed changes

0xisk merged commit 38feca3 into main May 29, 2026
10 checks passed

0xisk deleted the chore/test-coverage branch May 29, 2026 09:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(test): add vitest v8 coverage with 95% perFile gate#527

chore(test): add vitest v8 coverage with 95% perFile gate#527
0xisk merged 11 commits into
mainfrom
chore/test-coverage

0xisk commented May 28, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented May 28, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented May 28, 2026

Uh oh!

pepebndc left a comment •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

0xisk commented May 28, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Types of changes

PR Checklist

Further comments

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented May 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Poem

Uh oh!

coderabbitai Bot commented May 28, 2026

Uh oh!

pepebndc left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

0xisk commented May 28, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented May 28, 2026 •

edited

Loading

pepebndc left a comment •

edited

Loading