Add negative and boundary-condition tests for Election lifecycle#227
Add negative and boundary-condition tests for Election lifecycle#227harshkumarrai wants to merge 1 commit intoAOSSIE-Org:mainfrom
Conversation
📝 WalkthroughWalkthroughA new negative and boundary test suite is added for the Election contract. It covers lifecycle edge cases including voting before/after election periods, duplicate voting, invalid candidates, and premature result retrieval, with assertions for custom revert errors. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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: 1
🤖 Fix all issues with AI agents
In `@blockchain/test/lifecycle/Election.lifecycle.negative.test.js`:
- Around line 55-69: The test currently asserts both votes revert but should
allow the first vote and only revert the second; change the first call to
succeed (remove expect) by invoking election.connect(voter).userVote([0])
normally, then assert the second call reverts with the custom error using await
expect(election.connect(voter).userVote([0])).to.be.revertedWithCustomError(election,
"AlreadyVoted"); ensure you keep the existing timestamp advance and use the same
fixtures (deployElectionFixture, election, voter) and test title "reverts when
user votes twice".
🧹 Nitpick comments (1)
blockchain/test/lifecycle/Election.lifecycle.negative.test.js (1)
5-37: Potential timing issue:nowis captured before contract deployment.The
nowtimestamp is fetched at line 8, but subsequent transactions (deploy(),waitForDeployment(),initialize()) advance the block timestamp. When tests usenow + offsetfor time manipulation, the actual blockchain time may already be past the expected point, causing flaky behavior—especially with small offsets like0or5.Consider capturing
nowafterinitialize()completes, or use larger offsets to ensure timing robustness.♻️ Suggested fix
async function deployElectionFixture(startOffset, endOffset) { const [owner, voter, other] = await ethers.getSigners(); - const now = (await ethers.provider.getBlock("latest")).timestamp; + const preDeployTimestamp = (await ethers.provider.getBlock("latest")).timestamp; const Election = await ethers.getContractFactory("Election"); const election = await Election.deploy(); await election.waitForDeployment(); const electionInfo = { - startTime: now + startOffset, - endTime: now + endOffset, + startTime: preDeployTimestamp + startOffset, + endTime: preDeployTimestamp + endOffset, name: "Test Election", description: "Test Description", }; const candidates = [ { candidateID: 0, name: "A", description: "A" }, { candidateID: 1, name: "B", description: "B" }, ]; await election.initialize( electionInfo, candidates, 0, // resultType 0, // electionId ethers.ZeroAddress, // ballot (unused in negative tests) owner.address, ethers.ZeroAddress // resultCalculator (unused) ); + // Capture timestamp after initialization for accurate time manipulation + const now = (await ethers.provider.getBlock("latest")).timestamp; + return { election, owner, voter, other, now }; }
| it("reverts when user votes twice", async function () { | ||
| const { election, voter } = await deployElectionFixture(0, 1000); | ||
|
|
||
| const latest = await ethers.provider.getBlock("latest"); | ||
| await ethers.provider.send("evm_setNextBlockTimestamp", [latest.timestamp + 1]); | ||
| await ethers.provider.send("evm_mine"); | ||
|
|
||
| await expect( | ||
| election.connect(voter).userVote([0]) | ||
| ).to.be.reverted; | ||
|
|
||
| await expect( | ||
| election.connect(voter).userVote([0]) | ||
| ).to.be.reverted; | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find the Election contract file
fd -t f "Election" --type f | grep -E "\.(sol|test\.js)$" | head -20Repository: AOSSIE-Org/Agora-Blockchain
Length of output: 330
🏁 Script executed:
# Search for custom error definitions related to voting in solidity files
rg -n "error\s+\w*(Already|Duplicate|Voted)" --type solidity -A 1Repository: AOSSIE-Org/Agora-Blockchain
Length of output: 103
🏁 Script executed:
# Also search for all custom errors in Election contract
rg -n "^\s*error\s+" -g "*.sol" | head -30Repository: AOSSIE-Org/Agora-Blockchain
Length of output: 1908
🏁 Script executed:
# Find the deployElectionFixture function definition
rg -n "deployElectionFixture" -B 2 -A 10 | head -50Repository: AOSSIE-Org/Agora-Blockchain
Length of output: 4922
🏁 Script executed:
# Look at the userVote function in Election contract
rg -n "function userVote" -B 2 -A 15 --type solidityRepository: AOSSIE-Org/Agora-Blockchain
Length of output: 103
🏁 Script executed:
# Read the full deployElectionFixture function
cat -n blockchain/test/lifecycle/Election.lifecycle.negative.test.js | sed -n '5,37p'Repository: AOSSIE-Org/Agora-Blockchain
Length of output: 1247
🏁 Script executed:
# Search for other tests that test voting (to see the pattern)
rg -n "userVote" blockchain/test/ -B 3 -A 3 | head -80Repository: AOSSIE-Org/Agora-Blockchain
Length of output: 7459
🏁 Script executed:
# Look at the userVote function implementation
rg -n "function userVote" blockchain/contracts/Election.sol -B 2 -A 20Repository: AOSSIE-Org/Agora-Blockchain
Length of output: 891
🏁 Script executed:
# Check if there's a positive voting test to understand expected behavior
rg -n "should.*vote\|successfully.*vote" blockchain/test/ -i -B 2 -A 10 | head -60Repository: AOSSIE-Org/Agora-Blockchain
Length of output: 53
Test logic error: first vote should succeed, not revert.
This test is named "reverts when user votes twice" but expects both votes to revert. For a valid double-voting test, the first vote should succeed, and only the second vote should revert with a specific error (AlreadyVoted).
Additionally, use .to.be.revertedWithCustomError(election, "AlreadyVoted") instead of generic .to.be.reverted, consistent with other tests in this file.
🐛 Proposed fix
it("reverts when user votes twice", async function () {
- const { election, voter } = await deployElectionFixture(0, 1000);
+ const { election, voter } = await deployElectionFixture(10, 2000);
const latest = await ethers.provider.getBlock("latest");
- await ethers.provider.send("evm_setNextBlockTimestamp", [latest.timestamp + 1]);
+ await ethers.provider.send("evm_setNextBlockTimestamp", [latest.timestamp + 15]);
await ethers.provider.send("evm_mine");
- await expect(
- election.connect(voter).userVote([0])
- ).to.be.reverted;
+ // First vote should succeed
+ await election.connect(voter).userVote([0]);
+ // Second vote should revert with specific error
await expect(
election.connect(voter).userVote([0])
- ).to.be.reverted;
+ ).to.be.revertedWithCustomError(election, "AlreadyVoted");
});🤖 Prompt for AI Agents
In `@blockchain/test/lifecycle/Election.lifecycle.negative.test.js` around lines
55 - 69, The test currently asserts both votes revert but should allow the first
vote and only revert the second; change the first call to succeed (remove
expect) by invoking election.connect(voter).userVote([0]) normally, then assert
the second call reverts with the custom error using await
expect(election.connect(voter).userVote([0])).to.be.revertedWithCustomError(election,
"AlreadyVoted"); ensure you keep the existing timestamp advance and use the same
fixtures (deployElectionFixture, election, voter) and test title "reverts when
user votes twice".
There was a problem hiding this comment.
In this test, the election is intentionally initialized with a zero address ballot to keep the PR scoped to lifecycle boundary checks only. Because of this, the first userVote reverts before reaching the AlreadyVoted check.
I avoided introducing a mock ballot here to keep the PR minimal. If maintainers prefer strict AlreadyVoted validation, I am happy to add that in a follow-up PR with proper mocks.
There was a problem hiding this comment.
@harshkumarrai, thank you for the clarification! Your scoping decision makes sense—keeping the PR focused on lifecycle boundary checks without introducing mock ballot infrastructure is reasonable.
The test is valid as-is for verifying that voting fails with an invalid ballot configuration. To make the test's actual behavior clearer to future readers, consider adding a brief comment above the test explaining that the zero-address ballot causes the revert, so it's testing invalid configuration rather than pure double-voting logic:
// NOTE: Uses zero-address ballot; both votes revert due to invalid config.
// Dedicated AlreadyVoted validation requires ballot mock (deferred).
it("reverts when user votes twice", async function () {A follow-up PR with proper ballot mocks to validate the AlreadyVoted error specifically would be valuable for comprehensive coverage, but it's not blocking for this PR's objectives.
٩(◕‿◕)۶
✏️ Learnings added
Learnt from: harshkumarrai
Repo: AOSSIE-Org/Agora-Blockchain PR: 227
File: blockchain/test/lifecycle/Election.lifecycle.negative.test.js:55-69
Timestamp: 2026-01-27T13:40:36.893Z
Learning: In blockchain/test/lifecycle/Election.lifecycle.negative.test.js, the double-voting test intentionally uses a zero-address ballot to keep the PR scoped to lifecycle checks only. Testing the AlreadyVoted error specifically requires introducing ballot mocks, which is deferred to a follow-up PR.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
1 participant
Description
This PR adds explicit negative and boundary-condition test coverage for the Election contract lifecycle.
The new tests validate that lifecycle constraints are correctly enforced, preventing silent regressions when interacting with elections outside valid time windows or with invalid inputs.
This change is tests-only and does not modify any contract or production logic.
Fixes #226
Type of change
Please mark the options that are relevant.
Checklist:
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.