Skip to content
This repository has been archived by the owner on Oct 28, 2021. It is now read-only.

Change exit calls to returns in testeth, closes #4667 #4971

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Grubbly
Copy link

@Grubbly Grubbly commented Apr 20, 2018

return should be used over exit calls because return guarentees the destructor is called for locally scoped objects.

@codecov-io
Copy link

codecov-io commented Apr 20, 2018

Codecov Report

Merging #4971 into develop will increase coverage by 0.07%.
The diff coverage is 0%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #4971      +/-   ##
===========================================
+ Coverage     61.1%   61.17%   +0.07%     
===========================================
  Files          350      350              
  Lines        28298    28291       -7     
  Branches      2886     2889       +3     
===========================================
+ Hits         17291    17308      +17     
+ Misses       10048    10021      -27     
- Partials       959      962       +3

@gumb0 gumb0 requested review from pirapira and winsvega April 24, 2018 08:49
@@ -608,7 +608,7 @@ void ImportTest::checkAllowedNetwork(string const& _network)
// Can't use boost at this point
std::cerr << TestOutputHelper::get().testName() + " Specified Network not found: "
<< _network << "\n";
exit(1);
return;
Copy link
Member

Choose a reason for hiding this comment

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

The caller of this function should terminate the test when this happens.

@@ -127,12 +127,12 @@ Options::Options(int argc, const char** argv)
if (arg == "--help")
{
printHelp();
exit(0);
return;
Copy link
Member

Choose a reason for hiding this comment

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

Make sure that the process terminates when this constructor takes this path.

}
else if (arg == "--version")
{
printVersion();
exit(0);
return;
Copy link
Member

Choose a reason for hiding this comment

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

Make sure that the process terminates after the execution takes this path.

Copy link
Member

Choose a reason for hiding this comment

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

You can throw an exception instead of silently returning.

@@ -147,7 +147,7 @@ Options::Options(int argc, const char** argv)
g_logVerbosity = 13;
#else
cerr << "--vmtrace option requires a build with cmake -DVMTRACE=1\n";
exit(1);
return;
Copy link
Member

Choose a reason for hiding this comment

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

Make sure that the process terminates after the execution takes this path.

@@ -232,7 +232,7 @@ Options::Options(int argc, const char** argv)
else
{
std::cerr << "Options file not found! Default options at: tests/src/randomCodeOptions.json\n";
exit(0);
return;
Copy link
Member

Choose a reason for hiding this comment

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

Same as above.

@@ -272,11 +272,11 @@ Options::Options(int argc, const char** argv)
if (maxCodes > 1000 || maxCodes <= 0)
{
cerr << "Argument for the option is invalid! (use range: 1...1000)\n";
exit(1);
return;
Copy link
Member

Choose a reason for hiding this comment

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

Same as above.

}
test::RandomCodeOptions options;
cout << test::RandomCode::get().generate(maxCodes, options) << "\n";
exit(0);
return;
Copy link
Member

Choose a reason for hiding this comment

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

Same as above.

@@ -309,7 +309,7 @@ Options::Options(int argc, const char** argv)
else if (seenSeparator)
{
cerr << "Unknown option: " + arg << "\n";
exit(1);
return;
Copy link
Member

Choose a reason for hiding this comment

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

Same as above.

@@ -322,7 +322,7 @@ Options::Options(int argc, const char** argv)
cerr << "--createRandomTest cannot be used with any of the options: " <<
"trValueIndex, trGasIndex, trDataIndex, nonetwork, singleTest, all, " <<
"stats, filltests, fillchain \n";
exit(1);
return;
Copy link
Member

Choose a reason for hiding this comment

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

Same as above.

@@ -136,7 +136,7 @@ int main(int argc, const char* argv[])
catch (dev::test::InvalidOption const& e)
{
std::cerr << *boost::get_error_info<errinfo_comment>(e) << "\n";
exit(1);
return 1;
Copy link
Member

Choose a reason for hiding this comment

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

This one is fine.

@winsvega
Copy link
Contributor

@chfast what are the plans for testeth in cpp client ?

@chfast
Copy link
Member

chfast commented Apr 24, 2018

@winsvega I don't have any.

@chfast chfast changed the base branch from develop to master August 2, 2018 08:38
@axic axic added the testeth label Aug 9, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants