-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
Refactor tests and test EXTCODECOPY Failure Behavior #25
Merged
masonforest
merged 9 commits into
master
from
YAS-119/HardenTheOvm/FixEXTCODECOPYFailureBehavior
Mar 3, 2020
+486
−542
Merged
Changes from 8 commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
fd3f491
Add an EXTCODECOPY test when the code range is out of bounds
masonforest fbdac5d
Use helpers in call opcode tests
masonforest ac33b26
Use new test helpers
masonforest dca287d
Remove unused code
masonforest bcbed77
Remove unused variables
masonforest 1fc27ad
Use precomputed methodIds
masonforest 6e574d7
Add a test to ensure ovmEXTCODECOPY returns zeroed bytes
masonforest 2740483
Lint
masonforest a7ffb92
Change lodash to a devDependency
masonforest File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
I kiiinda am a fan of not requiring
lodash
because it technically doesn't have to be used & adds weight to the package. That said, I also do really enjoy programming with lodash. Maybe if we only used lodash for testing functions & added it only todevDependencies
then it'd be a no brainer? Either way, this is not a strong opinion so I'm personally fine with keeping lodash usage exactly as is.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.
Yeah agreed we shouldn't add dependencies unless their benefit outweighs the cost. I was using native
reduce
butfromPairs
made it a bit more readable. I moved it to adevDependency
so we can punt on that decision for now. I also added it to a list of things we can discuss as a team while we're all in person next week! 😄 -> TypeScript/Solidity Style Guide Discussion