Skip to content

Split up NameAndTypeResolution test cases#4178

Merged
chriseth merged 2 commits intodevelopfrom
name-and-type-tests-split
May 30, 2018
Merged

Split up NameAndTypeResolution test cases#4178
chriseth merged 2 commits intodevelopfrom
name-and-type-tests-split

Conversation

@axic
Copy link
Copy Markdown
Contributor

@axic axic commented May 23, 2018

Depends on #4177.

The reason for this is to fix some bugs and prepare the test cases for easy extraction via scripts (it works very well after this).

char const* text = R"(
contract C {
function f(uint a) pure public {
function f(uint) pure public {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test case was duplicated. There is the other one above this called "warn_unused_function_parameter".

char const* text = R"(
contract C {
function f() pure public returns (uint) {
return 1;
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added this test case.

char const* text = R"(
contract C {
function f() public view returns (uint256 val) { return msg.gas; }
function f() public view returns (uint256 val) { return gasleft(); }
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Swapped one test case hence the confusing change.

@axic
Copy link
Copy Markdown
Contributor Author

axic commented May 23, 2018

Still need to split up about 8 cases.

}

BOOST_AUTO_TEST_CASE(overflow_caused_by_ether_units)
BOOST_AUTO_TEST_CASE(no_overflow_with_large_literal)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you somehow make sure that this an the next test end up next to each other or at least have a similar name or reference each other in a comment?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is the only exception, the others are grouped. Not sure what is the best name for grouping, perhaps overflow_not_caused_with_large_literal_without_denomination.

Copy link
Copy Markdown
Contributor

@chriseth chriseth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please somehow ensure that split test cases reference each other so that the purpose of the test case is clear.

@chriseth
Copy link
Copy Markdown
Contributor

Please rebase.

@axic axic force-pushed the name-and-type-tests-split branch from 69098b7 to 14ee42b Compare May 23, 2018 15:31
@axic
Copy link
Copy Markdown
Contributor Author

axic commented May 23, 2018

Split up some other cases, will squash before merge. I think there are only 3 (albeit monster) current cases left to be split.

@axic axic force-pushed the name-and-type-tests-split branch from 14ee42b to 2d7e090 Compare May 24, 2018 01:48
@axic
Copy link
Copy Markdown
Contributor Author

axic commented May 28, 2018

@chriseth can you review please?

text = R"(
}

BOOST_AUTO_TEST_CASE(comparison_of_function_types_gt)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be lt

chriseth
chriseth previously approved these changes May 28, 2018
Copy link
Copy Markdown
Contributor

@chriseth chriseth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please fix the single lt case together with the rebase and merge.

@axic axic force-pushed the name-and-type-tests-split branch from 2d7e090 to ddc4492 Compare May 30, 2018 05:19
@axic axic changed the title [WIP] Split up NameAndTypeResolution test cases Split up NameAndTypeResolution test cases May 30, 2018
@axic
Copy link
Copy Markdown
Contributor Author

axic commented May 30, 2018

@chriseth updated

@chriseth chriseth merged commit 3f3d6df into develop May 30, 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