-
Notifications
You must be signed in to change notification settings - Fork 12.3k
Use strict equality in Arrays test helpers #6086
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
Conversation
|
WalkthroughThe pull request updates comparison operators in helper functions within the test utilities. Loose equality checks ( Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 0
🧹 Nitpick comments (1)
test/utils/Arrays.test.js (1)
23-23: Consider using strict inequality for consistency.For consistency with the changes on lines 12 and 18, consider replacing
!=with!==here as well.Apply this diff:
-const hasDuplicates = array => array.some((v, i) => array.indexOf(v) != i); +const hasDuplicates = array => array.some((v, i) => array.indexOf(v) !== i);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
test/utils/Arrays.test.js(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: Redirect rules - solidity-contracts
- GitHub Check: Header rules - solidity-contracts
- GitHub Check: Pages changed - solidity-contracts
- GitHub Check: coverage
- GitHub Check: tests-foundry
- GitHub Check: tests-upgradeable
- GitHub Check: slither
- GitHub Check: tests
- GitHub Check: halmos
🔇 Additional comments (2)
test/utils/Arrays.test.js (2)
12-12: LGTM! Good improvement for consistency.The change from
==to===aligns with JavaScript best practices and improves code consistency, even though the functional behavior remains unchanged sincefindIndexalways returns a number.
18-18: LGTM! Consistent with thelowerBoundchange.The strict equality check maintains consistency with the updated
lowerBoundfunction and follows JavaScript best practices.
Amxx
left a 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.
Note: i is the result of a call to findIndex. So its an integer. Using == or === should not make any difference for integers.
Replace loose equality (
==) with strict equality (===) inlowerBoundandupperBoundtest helper functions to improve code consistency and avoid potential type coercion issues.