-
Notifications
You must be signed in to change notification settings - Fork 6k
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
Tests for flipping signs on signed type edge case #5213
Conversation
} | ||
)"; | ||
compileAndRun(sourceCode); | ||
ABI_CHECK(callContractFunction("f()"), encodeArgs(u256(0))); |
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.
The function returns true
, so this should be encodeArgs(true)
. Apart from that, perfect!
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.
Thanks for the feedback! I did a rebase though, not sure if that's the proper procedure here or if I should've just not updated and pushed the fix anyways.
Edit: Fixed the rebase issue!
Codecov Report
@@ Coverage Diff @@
## develop #5213 +/- ##
===========================================
+ Coverage 87.87% 87.87% +<.01%
===========================================
Files 317 317
Lines 32023 32027 +4
Branches 3826 3826
===========================================
+ Hits 28139 28143 +4
Misses 2586 2586
Partials 1298 1298
|
Great! Could you squash the two commits into a single one? Then this is ready to be merged. |
Squashed the commits |
FIxes #5103
Checklist
Description
I may have missed a few things, it's my first time working on a programming language.
Since Issue #5103 's references EndToEndSuite, I made the test in the SolidityEndToEnd.cpp.
Hopefully the integrated tests won't be too mad. If I'm missing any fundamental things, misunderstood something or if I should add another case in the same test, let me know. Cheers!