-
Notifications
You must be signed in to change notification settings - Fork 502
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
Add crash on fail option #1420
Add crash on fail option #1420
Conversation
@@ -61,7 +61,12 @@ class Utest | |||
|
|||
class TestTerminator | |||
{ | |||
protected: | |||
static bool crashOnFail_; | |||
virtual void checkCrashOnFail() const; |
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.
This does not feel right. This seems to be a different TestTerminator rather than something that should be in this interface.
@@ -61,7 +61,12 @@ class Utest | |||
|
|||
class TestTerminator | |||
{ | |||
protected: | |||
static bool crashOnFail_; |
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.
Class attribute used to hold whether we are running in crash-on-fail mode or not.
return "use -h for more extensive help\n" | ||
"usage [-h] [-v] [-vv] [-c] [-p] [-lg] [-ln] [-ri] [-r#] [-f]\n" | ||
" [-g|sg|xg|xsg groupName]... [-n|sn|xn|xsn testName]... [-t groupName.testName]...\n" | ||
" [-b] [-s [randomizerSeed>0]] [\"TEST(groupName, testName)\"]...\n" | ||
" [-o{normal, junit, teamcity}] [-k packageName]\n"; |
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.
This last line of this was too long to be reasonably output on a console without wrapping. Hence, split the string whilst adding the new argument.
void TestTerminator::checkCrashOnFail() const | ||
{ | ||
if (crashOnFail_) | ||
UtestShell::crash(); |
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.
Use existing crash mechanism if running in crash-on-fail mode
@@ -46,7 +46,7 @@ int main(int ac, char **av) | |||
accountant.start(); | |||
#endif | |||
|
|||
CommandLineTestRunner::RunAllTests(ac, av); /* cover alternate method */ | |||
returnValue = CommandLineTestRunner::RunAllTests(ac, av); /* cover alternate method */ |
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.
Bugfix so that 'make check' correctly fails if any of the tests fail
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.
Please, in a separate PR...
TEST(MockSupportTestWithFixture, shouldCrashOnFailureWithCppUTestSetting) | ||
{ | ||
cpputestHasCrashed = false; | ||
fixture.getRegistry()->setCrashOnFail(); | ||
UtestShell::setCrashMethod(crashMethod); | ||
fixture.setTestFunction(unexpectedCallTestFunction_); | ||
|
||
fixture.runAllTests(); | ||
|
||
CHECK(cpputestHasCrashed); | ||
|
||
fixture.getRegistry()->setCrashOnFail(false); | ||
UtestShell::resetCrashMethod(); | ||
} | ||
|
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.
Test that setting crash-on-fail in the registry does cause failing tests to crash
Will split this PR into 3 separate PRs for easier review |
Still need to update the README.md (once the code is agreed) but creating a PR with what I have for discussion