Skip to content
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

Merged
merged 1 commit into from
Oct 18, 2018

Conversation

Mordax
Copy link
Contributor

@Mordax Mordax commented Oct 14, 2018

FIxes #5103

Checklist

  • Code compiles correctly
  • [?] All tests are passing //I pray they do
  • [?] New tests have been created which fail without the change (if possible)
  • Used meaningful commit messages

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!

@chriseth chriseth changed the title Issue #5103 - Added test for flipping signs on signed type edge case Tests for flipping signs on signed type edge case Oct 15, 2018
}
)";
compileAndRun(sourceCode);
ABI_CHECK(callContractFunction("f()"), encodeArgs(u256(0)));
Copy link
Contributor

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!

Copy link
Contributor Author

@Mordax Mordax Oct 15, 2018

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
Copy link

codecov bot commented Oct 15, 2018

Codecov Report

Merging #5213 into develop will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@             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
Flag Coverage Δ
#all 87.87% <100%> (ø) ⬆️
#syntax 28.29% <25%> (-0.01%) ⬇️

@chriseth
Copy link
Contributor

Great! Could you squash the two commits into a single one? Then this is ready to be merged.

@Mordax
Copy link
Contributor Author

Mordax commented Oct 18, 2018

Squashed the commits

@chriseth chriseth merged commit 4987c12 into ethereum:develop Oct 18, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants