test(DEVoterVoting): add test for invalid voting duration #104
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughRemoves a manual HardhatRuntimeEnvironment viem type augmentation, adds a test that rejects starting voting with a non-positive duration (uses 0) asserting "Invalid duration" and inactive state, updates Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Test
participant Contract as DEVoterVoting
rect #E8F1FF
note over Test,Contract: Attempt to start voting with invalid duration
Test->>Contract: startVotingPeriod(duration = 0)
Contract-->>Test: revert "Invalid duration"
end
rect #F0FFF0
note over Test,Contract: End voting using updated call signature
Test->>Contract: endVotingPeriod([], { account: owner })
Contract-->>Test: tx receipt
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
test/DEVoterVoting.ts (3)
86-88: Avoid Number() casts; assert with BigInt to keep full precision.Comparing as BigInt removes unnecessary conversions.
Apply this diff:
-expect(Number(votingEndTime)).to.equal( - Number(votingStartTime) + duration -); +expect(votingEndTime).to.equal(votingStartTime + BigInt(duration));
95-114: Rename test and make revert assertion less brittle (optional)
- Test passes 0, so "non‑positive" (or "zero") is more accurate; switch to 0n and accept either the string revert or a custom error.
Apply this diff:
-it("Should reject start with negative duration", async function () { +it("Should reject start with non-positive duration", async function () { @@ - await expect( - devoterVoting.write.startVotingPeriod([BigInt(0)], { - account: owner.account, - }) - ).to.be.rejectedWith("Invalid duration"); + await expect( + devoterVoting.write.startVotingPeriod([0n], { + account: owner.account, + }) + ).to.be.rejectedWith(/Invalid duration|InvalidDuration/);
12-12: Type augmentation cleanup looks good — declare hardhat-viem or ensure peer deps are installedI ran the check: package.json contains @nomicfoundation/hardhat-toolbox-viem (^4.0.0) and does not list @nomicfoundation/hardhat-viem. The Toolbox bundles hardhat-viem as a peer dependency (so the HRE type-extensions are provided when peer deps are installed), but because peer deps aren't listed in package.json IDEs/TypeScript may not pick up the augmentation — add @nomicfoundation/hardhat-viem to devDependencies or ensure peer deps are installed/pinned. (npmjs.com)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
test/DEVoterVoting.ts(3 hunks)
🔇 Additional comments (2)
test/DEVoterVoting.ts (2)
75-75: Whitespace-only change.
136-136: Explicit empty args array is fine; both forms are valid.Viem contract instance methods accept
(args, options)and allow omittingargsfor no‑arg functions. Using[]is explicit and TS‑friendly;endVotingPeriod({ account })is also valid per docs. (viem.sh)
Add test case to verify that startVotingPeriod rejects invalid (zero) duration Remove redundant type declaration as it's now provided by hardhat-viem
metanodreamer
force-pushed
the
test/non-owner
branch
from
a2f1dfd to
a763f57
Compare
Add test case to verify that startVotingPeriod rejects invalid (zero) duration Remove redundant type declaration as it's now provided by hardhat-viem
Summary by CodeRabbit
Tests
Chores
Style