Skip to content

test: run: refactor signal handler tests#2407

Merged
efiop merged 1 commit into
treeverse:masterfrom
efiop:master
Aug 19, 2019
Merged

test: run: refactor signal handler tests#2407
efiop merged 1 commit into
treeverse:masterfrom
efiop:master

Conversation

@efiop
Copy link
Copy Markdown
Contributor

@efiop efiop commented Aug 19, 2019

Current test is testing for signal handler restoration in an obscure
way. As a result, the test wasn't actually testing anything, because
KeyboardInterrupt is raised by default_int_handler, and so raising it
in a mock won't be caught by signal handlers, because they are dealing
with real singnals.

Fix #2406

Signed-off-by: Ruslan Kuprieiev ruslan@iterative.ai

  • Have you followed the guidelines in our
    Contributing document?

  • Does your PR affect documented changes or does it add new functionality
    that should be documented? If yes, have you created a PR for
    dvc.org documenting it or at
    least opened an issue for it? If so, please add a link to it.


@efiop efiop force-pushed the master branch 2 times, most recently from da397d8 to 5a7db1c Compare August 19, 2019 10:20
Current test is testing for signal handler restoration in an obscure
way. As a result, the test wasn't actually testing anything, because
KeyboardInterrupt is raised by default_int_handler, and so raising it
in a mock won't be caught by signal handlers, because they are dealing
with real singnals.

Fix treeverse#2406

Signed-off-by: Ruslan Kuprieiev <ruslan@iterative.ai>
@efiop efiop mentioned this pull request Aug 19, 2019
35 tasks
@efiop efiop merged commit f0c12ea into treeverse:master Aug 19, 2019
@efiop efiop assigned efiop and unassigned efiop Aug 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

local variable 'old_handler' referenced before assignment

1 participant