Skip to content
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

testsuite: avoid artificial corefile in test #5641

Merged
merged 1 commit into from
Dec 28, 2023

Conversation

grondo
Copy link
Contributor

@grondo grondo commented Dec 28, 2023

Problem: In t3306-system-routercrash.t SIGSEGV is intentionally sent to a broker to test a broker crash, but this could leave a mysterious corefile in the test directory if the core file ulimit and system configuration allow it.

Call ulimit -c 0 before test_under_flux() to avoid the core file.

Problem: In t3306-system-routercrash.t SIGSEGV is intentionally sent
to a broker to test a broker crash, but this could leave a mysterious
corefile in the test directory if the core file ulimit and system
configuration allow it.

Call `ulimit -c 0` before test_under_flux() to avoid the core file.
Copy link
Member

@chu11 chu11 left a comment

Choose a reason for hiding this comment

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

LGTM!

@grondo
Copy link
Contributor Author

grondo commented Dec 28, 2023

Thanks!

Copy link

codecov bot commented Dec 28, 2023

Codecov Report

Merging #5641 (afd5ecc) into master (6009b67) will decrease coverage by 0.01%.
The diff coverage is n/a.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5641      +/-   ##
==========================================
- Coverage   83.46%   83.45%   -0.01%     
==========================================
  Files         487      487              
  Lines       82931    82931              
==========================================
- Hits        69215    69210       -5     
- Misses      13716    13721       +5     

see 6 files with indirect coverage changes

@mergify mergify bot merged commit beeb4ef into flux-framework:master Dec 28, 2023
34 checks passed
@grondo grondo deleted the t3306-no-core branch December 28, 2023 16:45
@wihobbs
Copy link
Member

wihobbs commented Dec 28, 2023

Based on my understanding of ulimit, these settings will persist after this test, and we use ulimit -c unlimited in shell scripts before the test suite runs to try and capture core files occasionally in GitLab (we tried this with the libterminus tests for a while).

Not a big deal, it hasn't really helped us yet, but anyways...I noticed this is contained to t3306, is there a way to restore whatever settings were before after this test is over?

@wihobbs
Copy link
Member

wihobbs commented Dec 28, 2023

I guess in order to do ^ you'd have to save something like COREFILE_SIZE=$(ulimit -c) before changing ulimit -c 0, then restore at the end of the test with ulimit -c $COREFILE_SIZE. @grondo is that how you would go about this?

@grondo
Copy link
Contributor Author

grondo commented Dec 28, 2023

Based on my understanding of ulimit, these settings will persist after this test, and we use ulimit -c unlimited in shell scripts before the test suite runs to try and capture core files occasionally in GitLab (we tried this with the libterminus tests for a while).

ulimit(1) and the setrlimit(2) system call it uses only affect the current process and its descendants (see the NOTES section in the setrlimit(2) man page above).

You can test this for yourself by starting a new shell, adjusting the resource limit, then exiting and see the parent shell's resource limit is unchanged:

$ ulimit -c
16
$ bash 
$ ulimit -c unlimited
$ ulimit -c
unlimited
$ exit
exit
$ ulimit -c
16

Since ulimit -c unlimited is called within a sharness test, it will only persist for the tests within that one test script.

@grondo
Copy link
Contributor Author

grondo commented Dec 28, 2023

Though I should mention that all tests within a single sharness test do run in the same shell and what you did above would work I think if you wanted to adjust the ulimit for a single test_expect_success for example. (Alternately, you could run the test in a subshell)

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

Successfully merging this pull request may close these issues.

3 participants