-
Notifications
You must be signed in to change notification settings - Fork 178
Add mock candidates to election creation tests #218
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughTests were updated to pass a candidates array to ElectionFactory.createElection instead of a numeric placeholder, aligning test calls with the contract's expected Election.Candidate[] parameter and resolving ethers v6 ABI fragment errors. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @blockchain/test/ElectionFactory.test.js:
- Around line 18-29: The mockCandidates array includes a candidateID field that
the contract ignores because Election.initialize overwrites IDs with the loop
index; update the test by removing the candidateID properties from
mockCandidates (or adjust them to match the contract-generated IDs like 0, 1)
and update any assertions that referenced the previous IDs accordingly so tests
reflect the contract's actual IDs generated in initialize.
🧹 Nitpick comments (4)
blockchain/test/ElectionFactory.test.js (4)
52-52: Minor: Add spacing after the comma for consistency.There's a minor spacing inconsistency in the function call.
♻️ Proposed fix
- await factory.createElection(mockElectionInfo,mockCandidates, 0, 0); + await factory.createElection(mockElectionInfo, mockCandidates, 0, 0);
60-61: Minor: Add spacing after the comma for consistency.Same spacing inconsistency as line 52.
♻️ Proposed fix
- await factory.createElection(mockElectionInfo,mockCandidates, 0, 0); - await factory.createElection(mockElectionInfo,mockCandidates, 0, 0); + await factory.createElection(mockElectionInfo, mockCandidates, 0, 0); + await factory.createElection(mockElectionInfo, mockCandidates, 0, 0);
71-71: Minor: Add spacing after the comma for consistency.There's inconsistent spacing in this function call (space after first comma but not the second).
♻️ Proposed fix
- await factory.createElection(mockElectionInfo, mockCandidates,0, 0); + await factory.createElection(mockElectionInfo, mockCandidates, 0, 0);
50-67: Optional: Consider adding tests for candidate-related edge cases.While the current changes successfully fix the ABI mismatch, you might want to enhance test coverage by adding test cases for:
- Creating elections with an empty candidates array
- Creating elections with a single candidate
- Verifying that candidates are correctly stored and retrievable after election creation
These additional tests would ensure the candidate functionality is robust, but they can be deferred to a future PR if desired.
Updated candidateID values for consistent test behavior.
There was a problem hiding this 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 (3)
blockchain/test/ElectionFactory.test.js (3)
53-53: Add space after commas for consistency.Minor formatting:
mockElectionInfo,mockCandidatesshould bemockElectionInfo, mockCandidatesto follow standard JavaScript style conventions.✨ Formatting fix
- await factory.createElection(mockElectionInfo,mockCandidates, 0, 0); + await factory.createElection(mockElectionInfo, mockCandidates, 0, 0);Apply the same spacing fix to lines 61 and 62.
Also applies to: 61-62
52-58: Consider verifying candidate data in tests.The tests create elections with
mockCandidatesbut don't assert that the candidates were properly stored. Adding verification would strengthen test coverage and confirm the fix fully resolves the ABI mismatch issue.💡 Example assertion to add
After creating an election, you could retrieve and verify candidate data:
it("Should create a new election", async function () { await factory.createElection(mockElectionInfo, mockCandidates, 0, 0); expect(await factory.electionCount()).to.equal(1); const openElections = await factory.getOpenElections(); expect(openElections.length).to.equal(1); // Verify candidates were stored (example - adjust based on actual contract API) // const election = await ethers.getContractAt("Election", openElections[0]); // const candidateCount = await election.getCandidateCount(); // expect(candidateCount).to.equal(2); });
72-72: Add space after comma for consistency.Minor formatting:
mockCandidates,0should bemockCandidates, 0.✨ Formatting fix
- await factory.createElection(mockElectionInfo, mockCandidates,0, 0); + await factory.createElection(mockElectionInfo, mockCandidates, 0, 0);
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
blockchain/test/ElectionFactory.test.js
🔇 Additional comments (1)
blockchain/test/ElectionFactory.test.js (1)
17-30: Excellent fix—candidateID values now match contract behavior.The
candidateIDvalues have been corrected to0, 1, aligning with the IDs the contract'sinitializefunction generates via its loop index. The comment clearly explains that the contract overwrites these values, which helps future maintainers understand the test setup.
Description
This PR updates the election creation test by adding mock candidates to ensure tests align with the current contract behavior and ABI expectations.
Previously, the election creation tests assumed candidate-independent initialization, which no longer reflects the contract’s current required inputs. Introducing mock candidates makes the tests deterministic, realistic, and prevents the earlier test failures caused by ABI mismatches.
Scope of this change is only limited to tests. No contract or application logic is modified.
Fixes #216
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.