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

Keep expected crash in misc test from producing a core dump #42990

Merged
merged 1 commit into from
Nov 8, 2021

Conversation

ararslan
Copy link
Member

@ararslan ararslan commented Nov 7, 2021

Currently the test in test/misc.jl that tests that a particular kind of read fault crashes has two issues:

  1. It works for the wrong reason. It's testing for !success when evaluating code interpolated directly into the Cmd, but it isn't quoting the code, so the subprocess fails with a syntax error instead of the type of crash the test is expecting.

  2. On some platforms (such as our good ol' pal FreeBSD), this kind of read fault produces a core dump. This a bit annoying because it means that the repo state becomes dirty after running the tests and it may overwrite an existing core dump that was left behind by an earlier issue we'd like to diagnose.

To fix these, we can wrap the code passed to the subprocess in single quotes and on Unix-like systems wrap the subprocess in ulimit -c 0 to avoid producing a core dump.

Currently the test in `test/misc.jl` that tests that a particular kind
of read fault crashes has two issues:

1. It works for the wrong reason. It's testing for `!success` when
   evaluating code interpolated directly into the `Cmd`, but it isn't
   quoting the code, so the subprocess fails with a syntax error instead
   of the type of crash the test is expecting.

2. On some platforms (such as our good ol' pal FreeBSD), this kind of
   read fault produces a core dump. This a bit annoying because it means
   that the repo state becomes dirty after running the tests and it may
   overwrite an existing core dump that was left behind by an earlier
   issue we'd like to diagnose.

To fix these, we can wrap the code passed to the subprocess in single
quotes and on Unix-like systems wrap the subprocess in `ulimit -c 0` to
avoid producing a core dump.
@ararslan ararslan added test This change adds or pertains to unit tests bugfix This change fixes an existing bug cmd Relates to calling of external programs labels Nov 7, 2021
@DilumAluthge
Copy link
Member

DilumAluthge commented Nov 7, 2021

Can we backport this to 1.6 and 1.7?

@DilumAluthge DilumAluthge added backport 1.6 Change should be backported to release-1.6 backport 1.7 ci Continuous integration labels Nov 7, 2021
@ararslan
Copy link
Member Author

ararslan commented Nov 7, 2021

Can we backport this to 1.6 and 1.7?

Sure. Should be a mostly non-functional change.

@DilumAluthge DilumAluthge merged commit fa6dcb0 into master Nov 8, 2021
@DilumAluthge DilumAluthge deleted the aa/no-bsd-misc-coredump branch November 8, 2021 01:46
KristofferC pushed a commit that referenced this pull request Nov 8, 2021
Currently the test in `test/misc.jl` that tests that a particular kind
of read fault crashes has two issues:

1. It works for the wrong reason. It's testing for `!success` when
   evaluating code interpolated directly into the `Cmd`, but it isn't
   quoting the code, so the subprocess fails with a syntax error instead
   of the type of crash the test is expecting.

2. On some platforms (such as our good ol' pal FreeBSD), this kind of
   read fault produces a core dump. This a bit annoying because it means
   that the repo state becomes dirty after running the tests and it may
   overwrite an existing core dump that was left behind by an earlier
   issue we'd like to diagnose.

To fix these, we can wrap the code passed to the subprocess in single
quotes and on Unix-like systems wrap the subprocess in `ulimit -c 0` to
avoid producing a core dump.

(cherry picked from commit fa6dcb0)
@KristofferC KristofferC removed the backport 1.6 Change should be backported to release-1.6 label Nov 10, 2021
@KristofferC
Copy link
Member

Test does not exist on 1.6.

LilithHafner pushed a commit to LilithHafner/julia that referenced this pull request Feb 22, 2022
…g#42990)

Currently the test in `test/misc.jl` that tests that a particular kind
of read fault crashes has two issues:

1. It works for the wrong reason. It's testing for `!success` when
   evaluating code interpolated directly into the `Cmd`, but it isn't
   quoting the code, so the subprocess fails with a syntax error instead
   of the type of crash the test is expecting.

2. On some platforms (such as our good ol' pal FreeBSD), this kind of
   read fault produces a core dump. This a bit annoying because it means
   that the repo state becomes dirty after running the tests and it may
   overwrite an existing core dump that was left behind by an earlier
   issue we'd like to diagnose.

To fix these, we can wrap the code passed to the subprocess in single
quotes and on Unix-like systems wrap the subprocess in `ulimit -c 0` to
avoid producing a core dump.
LilithHafner pushed a commit to LilithHafner/julia that referenced this pull request Mar 8, 2022
…g#42990)

Currently the test in `test/misc.jl` that tests that a particular kind
of read fault crashes has two issues:

1. It works for the wrong reason. It's testing for `!success` when
   evaluating code interpolated directly into the `Cmd`, but it isn't
   quoting the code, so the subprocess fails with a syntax error instead
   of the type of crash the test is expecting.

2. On some platforms (such as our good ol' pal FreeBSD), this kind of
   read fault produces a core dump. This a bit annoying because it means
   that the repo state becomes dirty after running the tests and it may
   overwrite an existing core dump that was left behind by an earlier
   issue we'd like to diagnose.

To fix these, we can wrap the code passed to the subprocess in single
quotes and on Unix-like systems wrap the subprocess in `ulimit -c 0` to
avoid producing a core dump.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix This change fixes an existing bug ci Continuous integration cmd Relates to calling of external programs test This change adds or pertains to unit tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants