Skip to content

Conversation

turkaturki
Copy link
Collaborator

@turkaturki turkaturki commented Sep 15, 2025

Add test case to verify contract behavior with 1-second voting duration Remove redundant type declaration as it's now provided by hardhat-viem

Summary by CodeRabbit

  • New Features

    • None impacting end-users.
  • Tests

    • Added coverage for very short (1s) voting periods to verify start/end times and active state.
    • Updated test invocation for ending a voting period to match the current API usage.
  • Chores

    • Removed redundant local type augmentation in tests to rely on official tooling typings.

Copy link

coderabbitai bot commented Sep 15, 2025

Walkthrough

Removed a custom HardhatRuntimeEnvironment viem type augmentation, added a test for a 1-second voting duration in startVotingPeriod, and updated endVotingPeriod calls to pass an empty-array first argument.

Changes

Cohort / File(s) Summary
Test changes & additions
test/DEVoterVoting.ts
Added a test "Should handle very short durations (1 second)" asserting votingStartTime proximity, votingEndTime == votingStartTime + 1, and isVotingActive; updated endVotingPeriod invocation to endVotingPeriod([], { account: owner.account }).
Type declaration removal
test/DEVoterVoting.ts
Removed manual TypeScript declare module "hardhat/types" augmentation for HardhatRuntimeEnvironment.viem, relying on @nomicfoundation/hardhat-viem types instead.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested reviewers

  • metanodreamer

Poem

A second's hop, a ballot's beat,
I start the clock, then skip a feet.
Types laid down, the calls align,
Empty arrays, results divine.
Thump-thump — the tests all pass, hooray! 🐇

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title "test(DEVoterVoting): add test for very short voting durations" accurately and concisely summarizes the primary change in the PR — adding a test for 1-second voting durations — and follows a clear conventional-commit style with scope. It is directly related to the changeset shown in the raw summary and PR objectives, is specific enough for history scanning, and the omission of the minor TypeScript declaration removal is acceptable because the title highlights the main functional change.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch test/short-duration

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5ad23df and d79960b.

📒 Files selected for processing (1)
  • test/DEVoterVoting.ts (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • test/DEVoterVoting.ts

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.

❤️ Share

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

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
test/DEVoterVoting.ts (1)

93-121: Make the 1‑second test fully deterministic and compare as BigInt.

Avoid Number() casts and assert expiry deterministically to prevent flakiness under different mining settings.

Apply this minimal diff:

-        const duration = 1; // 1 second duration
+        const duration = 1n; // 1 second duration

-        await devoterVoting.write.startVotingPeriod([BigInt(duration)], {
+        await devoterVoting.write.startVotingPeriod([duration], {
           account: owner.account,
         });

         // Get the actual values from the contract
         const votingStartTime = await devoterVoting.read.votingStartTime();
         const votingEndTime = await devoterVoting.read.votingEndTime();
-        const isVotingActive = await devoterVoting.read.isVotingActive();
         
         // Verify that votingEndTime equals votingStartTime + 1 second
-        expect(Number(votingEndTime)).to.equal(Number(votingStartTime) + duration);
+        expect(votingEndTime).to.equal(votingStartTime + duration);
         
         // Verify that the voting period is active
-        expect(isVotingActive).to.be.true;
+        expect(await devoterVoting.read.isVotingActive()).to.be.true;
+
+        // Advance time to ensure the short period elapses and flips to inactive
+        await time.increase(2);
+        expect(await devoterVoting.read.isVotingActive()).to.be.false;

Note: If time.increase isn’t available in your toolbox-viem version, use setNextBlockTimestamp or provider.send("evm_increaseTime", …) + mine. Please confirm.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9b569f6 and 5ad23df.

📒 Files selected for processing (1)
  • test/DEVoterVoting.ts (3 hunks)
🔇 Additional comments (2)
test/DEVoterVoting.ts (2)

12-12: Removing the manual HRE augmentation is correct.

Relying on @nomicfoundation/hardhat-viem’s provided types is the right call. Ensure tsconfig.json includes the plugin’s types so this remains stable even if these top‑of‑file imports are later refactored.


144-144: Correct viem write signature for no‑arg function.

Passing an empty args array to write.endVotingPeriod is the correct pattern; good fix.

Add test case to verify contract behavior with 1-second voting duration
Remove redundant type declaration as it's now provided by hardhat-viem
@metanodreamer metanodreamer merged commit e0e4094 into main Sep 16, 2025
1 check passed
@nowak-token nowak-token deleted the test/short-duration branch September 25, 2025 18:31
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.

2 participants