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

Projects
None yet
2 participants
@Mordax
Contributor

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 from Issue #5103 - Added test for flipping signs on signed type edge case to Tests for flipping signs on signed type edge case Oct 15, 2018

}
)";
compileAndRun(sourceCode);
ABI_CHECK(callContractFunction("f()"), encodeArgs(u256(0)));

This comment has been minimized.

@chriseth

chriseth Oct 15, 2018

Contributor

The function returns true, so this should be encodeArgs(true). Apart from that, perfect!

This comment has been minimized.

@Mordax

Mordax Oct 15, 2018

Contributor

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

This comment has been minimized.

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%) ⬇️

@Mordax Mordax force-pushed the Mordax:issue-5103 branch from 29d8b6b to 81768da Oct 15, 2018

@chriseth

This comment has been minimized.

Contributor

chriseth commented Oct 15, 2018

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

@Mordax Mordax force-pushed the Mordax:issue-5103 branch from 81768da to 134f5cb Oct 15, 2018

@Mordax

This comment has been minimized.

Contributor

Mordax commented Oct 18, 2018

Squashed the commits

@chriseth chriseth merged commit 4987c12 into ethereum:develop Oct 18, 2018

17 checks passed

ci/circleci: build_emscripten Your tests passed on CircleCI!
Details
ci/circleci: build_x86_linux Your tests passed on CircleCI!
Details
ci/circleci: build_x86_mac Your tests passed on CircleCI!
Details
ci/circleci: docs Your tests passed on CircleCI!
Details
ci/circleci: test_buglist Your tests passed on CircleCI!
Details
ci/circleci: test_check_spelling Your tests passed on CircleCI!
Details
ci/circleci: test_check_style Your tests passed on CircleCI!
Details
ci/circleci: test_emscripten_external Your tests passed on CircleCI!
Details
ci/circleci: test_emscripten_solcjs Your tests passed on CircleCI!
Details
ci/circleci: test_x86_linux Your tests passed on CircleCI!
Details
ci/circleci: test_x86_mac Your tests passed on CircleCI!
Details
codecov/patch 100% of diff hit (target 87.87%)
Details
codecov/project 85.45% remains the same compared to 036929a
Details
codecov/project/syntax 83.86% remains the same compared to 036929a
Details
codecov/project/tests 93.21% (+<.01%) compared to 036929a
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment