Skip to content

test: migrate CoverBroker, CoverNFT and CoverViewer unit tests to ethers v6 - #1521

Merged
rackstar merged 2 commits into
release-candidatefrom
test/unit-tests-cover-contracts-ethers-v6-migration
Mar 13, 2026
Merged

rackstar merged 2 commits into
release-candidatefrom
test/unit-tests-cover-contracts-ethers-v6-migration

Conversation

@rackstar

@rackstar rackstar commented Mar 9, 2026 •

Copy link
Copy Markdown
Contributor

Description

Describe the changes made in this PR. A link to the issue is enough if it contains all the relevant information.

Testing

Explain how you tested your changes to ensure they work as expected.

Checklist

  • Performed a self-review of my own code
  • Made corresponding changes to the documentation

Summary by CodeRabbit

  • Tests
    • Added comprehensive unit test coverage for CoverBroker rescue funds functionality, including ETH and ERC20 token scenarios.
    • Added unit test suite for CoverNFT contract functionality including minting, access control, and operator management.
    • Added unit test suite for CoverViewer cover data retrieval and validation.

@roxdanila

Copy link
Copy Markdown
Contributor

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Mar 11, 2026

Copy link
Copy Markdown
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai

coderabbitai Bot commented Mar 11, 2026 •

Copy link
Copy Markdown

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e0fb9e8d-2a11-4601-8436-0ce7cbbae367

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

This pull request introduces new unit test files and test setup utilities for three contracts: CoverBroker (rescueFunds functionality), CoverNFT (comprehensive contract testing), and CoverViewer (views functionality). All additions are purely test-related code across six newly created files.

Changes

Cohort / File(s) Summary
CoverBroker Tests
test/unit/CoverBroker/rescueFunds.js, test/unit/CoverBroker/setup.js
Adds test suite covering rescueFunds functionality including ETH rescue, ERC20 token rescue, and unauthorized access prevention. Includes setup utility that deploys CoverBroker, RegistryMock, and ERC20Mock with configured owner and contracts mapping.
CoverNFT Tests
test/unit/CoverNFT/coverNFT.js, test/unit/CoverNFT/setup.js
Comprehensive test suite for CoverNFT contract covering constructor, minting, approvals, operator management, and descriptor updates with access control validation. Setup utility deploys CoverNFT, CoverNFTDescriptor, and MasterMock with operator configuration.
CoverViewer Tests
test/unit/CoverViewer/views.js, test/unit/CoverViewer/setup.js
Adds test coverage for CoverViewer.getCovers() functionality validating data retrieval and field accuracy. Setup utility deploys CoverViewer, CVMockCover, and MasterMock with proper contract address configuration.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related issues

  • Aligns with ethers v6 migration epic—adds new unit tests and setup utilities for CoverBroker, CoverNFT, and CoverViewer contracts as part of converting test suites to ethers v6.
🚥 Pre-merge checks | ✅ 2
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly and clearly describes the main change: migrating unit tests for three contracts (CoverBroker, CoverNFT, and CoverViewer) to ethers v6, which is confirmed by the raw summary showing new test files added for these three contracts.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch test/unit-tests-cover-contracts-ethers-v6-migration
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch test/unit-tests-cover-contracts-ethers-v6-migration
📝 Coding Plan
  • Generate coding plan for human review comments

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@test/unit/CoverBroker/rescueFunds.js`:
- Around line 62-66: The test currently checks that rescueFunds reverts but only
with a zero balance; first send/fund ETH to the coverBroker contract (use a
signer to send value to coverBroker.target) so coverBroker has a non-zero
balance, then record balanceBefore, call
coverBroker.connect(nonOwner).rescueFunds(ETH) expecting revert, and finally
assert balanceAfter equals balanceBefore to ensure no funds were moved; apply
this change around the existing rescueFunds test block.
- Around line 30-31: The assertions compare bigint balances to number literals
causing type mismatch; change the comparisons to use bigint literals (e.g., 0n)
in the tests that reference brokerBalanceBefore and brokerBalanceAfter (and the
similar assertions at lines 51-52). Update the expect calls that currently
compare to 0 to compare to 0n so values returned by getBalance()/balanceOf()
(bigint) are compared correctly.

In `@test/unit/CoverNFT/coverNFT.js`:
- Around line 140-145: The assertions comparing coverNFT.totalSupply() currently
use numeric literals; under ethers v6 totalSupply() returns a bigint, so update
the expectations to compare against bigint values (e.g., 0n and 1n or
BigInt(0)/BigInt(1)) when asserting coverNFT.totalSupply(), ensuring both the
pre-mint and post-mint expects use bigint comparisons; keep the rest of the test
(the mint call and Transfer event assertion) unchanged.

In `@test/unit/CoverNFT/setup.js`:
- Around line 11-16: The CoverNFT deployment in setup.js uses the symbol 'NXMC'
which diverges from the existing fixtures that use 'NMC'; update the constructor
arguments passed to ethers.deployContract for CoverNFT to use the canonical
symbol 'NMC' (replace 'NXMC' with 'NMC') so tests and fixtures remain
consistent—modify the array passed to ethers.deployContract where CoverNFT is
constructed (the call creating coverNFT) to align the symbol with other
fixtures.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 052e7e99-65db-4234-ad72-766ff2bb7c2b

📥 Commits

Reviewing files that changed from the base of the PR and between 0709725 and 4954ccc.

📒 Files selected for processing (6)
  • test/unit/CoverBroker/rescueFunds.js
  • test/unit/CoverBroker/setup.js
  • test/unit/CoverNFT/coverNFT.js
  • test/unit/CoverNFT/setup.js
  • test/unit/CoverViewer/setup.js
  • test/unit/CoverViewer/views.js

Comment thread test/unit/CoverBroker/rescueFunds.js
Comment thread test/unit/CoverBroker/rescueFunds.js
Comment thread test/unit/CoverNFT/coverNFT.js
Comment thread test/unit/CoverNFT/setup.js
@rackstar

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Mar 11, 2026

Copy link
Copy Markdown
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@rackstar
rackstar force-pushed the test/unit-tests-cover-contracts-ethers-v6-migration branch from d1c3b9b to 5af1580 Compare March 13, 2026 14:45
@rackstar
rackstar merged commit b02f860 into release-candidate Mar 13, 2026
7 checks passed
@rackstar
rackstar deleted the test/unit-tests-cover-contracts-ethers-v6-migration branch March 13, 2026 14:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

test Unit/integration/CI tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Convert CoverViewer unit tests to ethers v6 Convert CoverNFT unit tests to ethers v6 Convert CoverBroker unit tests to ethers v6

2 participants