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

DM-38165: Suppress traceback for a crashing task in unit test #224

Merged
merged 1 commit into from
Feb 28, 2023
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions tests/test_executors.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
"""Simple unit test for cmdLineFwk module.
"""

import faulthandler
import logging
import signal
import sys
Expand Down Expand Up @@ -186,6 +187,8 @@ class TaskMockCrash:

def runQuantum(self):
_LOG.debug("TaskMockCrash.runQuantum")
# Disable fault handler to suppress long scary traceback.
faulthandler.disable()
Copy link
Member

Choose a reason for hiding this comment

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

I'm not familiar with faulthandler, but this looks like a global setting. Don't you need to re-enable it when the test is complete? Not sure how to do this if the task object is responsible for the setting, though; I'm guessing raise_signal bypasses finally blocks?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is indeed a global setting for the whole process, but in this case it's fine because the process that runs the task is killed immediately on the next line. This task is only ever used in a forked execution (otherwise signal would kill the whole test) so it does not affect faulthandler in the parent process.

Copy link
Member

Choose a reason for hiding this comment

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

Could you add a comment to that effect? It's not at all obvious in context that the task must only be run in an isolated environment.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sorry, I pushed "Merge" already. I'll try to remember adding that on some other ticket.

signal.raise_signal(signal.SIGILL)


Expand Down