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: Don't dump core when using EXPECT_DEATH #14821

Merged
merged 1 commit into from May 2, 2017

Conversation

Projects
None yet
6 participants
@badone
Contributor

badone commented Apr 27, 2017

Signed-off-by: Brad Hubbard bhubbard@redhat.com

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented Apr 27, 2017

@badone the change looks great! i posted badone#1 to coat it with some more sugar, could you take a look?

@badone

This comment has been minimized.

Contributor

badone commented Apr 27, 2017

@tchaikov awesome! I'll tidy up these commits tomorrow, thanks so much.

{
PrCtl unset_dumpable;
ASSERT_DEATH((t << "1" << "2" << "3" << "4" << TextTable::endrow), "");
}

This comment has been minimized.

@tchaikov

tchaikov Apr 27, 2017

Contributor

can remove the { } parenthesis here. this test will end anyway.

unsetprdumpable();
EXPECT_DEATH(pool_opts_t::get_opt_desc("INVALID_OPT"), "");
setprdumpable();
{

This comment has been minimized.

@tchaikov

tchaikov Apr 27, 2017

Contributor

can remove parenthesis here.

unsetprdumpable();
EXPECT_DEATH(missing.add_next_event(e), "");
setprdumpable();
{

This comment has been minimized.

@tchaikov

tchaikov Apr 27, 2017

Contributor

could remove parenthesis here.

unsetprdumpable();
EXPECT_DEATH(delete m,".*");
setprdumpable();
{

This comment has been minimized.

@tchaikov

tchaikov Apr 27, 2017

Contributor

could remove parenthesis here.

This comment has been minimized.

@wjwithagen

wjwithagen Apr 27, 2017

Contributor

@badone @tchaikov
Interesting concept. This was on my to do list, since I usually end up with directories littered with *core files..
It is possible to set a nodump flag globally before starting the tests, but then good dumps also get lost...
Under FreeBSD I think the best way to go is set the rlimit on coresize to 0. (after saving the max limit first.) . That would not be a very difficult addition which I'll do once this PR is in.

PrCtl(int new_state = 0) {
int r = prctl(PR_GET_DUMPABLE);
if (r == -1) {
r = errno;

This comment has been minimized.

@badone

badone Apr 28, 2017

Contributor

@tchaikov should this be "r = -errno;" as above?

This comment has been minimized.

@tchaikov

tchaikov Apr 28, 2017

Contributor

i think we can leave as it is. unlike above, r is only used to store errno here, instead of being used as the return code, so we don't need to negate it.

but it's fine to use r = -errno also, just for consistency but not for correctness.

This comment has been minimized.

@badone

badone Apr 28, 2017

Contributor

ack

@badone

This comment has been minimized.

Contributor

badone commented Apr 28, 2017

@wjwithagen cool

@tchaikov updated

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented Apr 28, 2017

@badone maybe you could squash these two commits into a single one?

test: Don't dump core when using EXPECT_DEATH
Signed-off-by: Kefu Chai <kchai@redhat.com>
Signed-off-by: Brad Hubbard <bhubbard@redhat.com>
@badone

This comment has been minimized.

Contributor

badone commented Apr 28, 2017

Jenkins test this please

@tchaikov tchaikov requested review from liewegas and jdurgin Apr 28, 2017

@jdurgin

good idea and impl @badone and @tchaikov!

@badone

This comment has been minimized.

Contributor

badone commented Apr 28, 2017

Thanks @jdurgin

@liewegas you may have tested this a little early (that's prolly my fault, sorry)?

@yuriw yuriw merged commit 76e94a0 into ceph:master May 2, 2017

3 checks passed

Signed-off-by all commits in this PR are signed
Details
Unmodifed Submodules submodules for project are unmodified
Details
default Build finished.
Details

@badone badone deleted the badone:wip-no-death-coredumps branch May 2, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment