Skip to content

Commit

Permalink
Drop destructor from CommandProcess
Browse files Browse the repository at this point in the history
There is code that sends a SIGTERM to the process in case
there is no error code information. I believe in this case
sending SIGTERM will not kill the process (defunct) and I
also don't see in what good condition we would be entering
this state.
  • Loading branch information
schaefi committed Feb 19, 2024
1 parent 48817a6 commit ba27291
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 20 deletions.
7 changes: 0 additions & 7 deletions kiwi/command_process.py
Original file line number Diff line number Diff line change
Expand Up @@ -141,13 +141,6 @@ def _update_progress(
'[ INFO ]: Processing'
)

def __del__(self):
if self.command and self.command.get_error_code() is None:
log.info(
'Terminating subprocess %d', self.command.get_pid()
)
self.command.kill()


class CommandIterator:
"""
Expand Down
27 changes: 14 additions & 13 deletions test/unit/command_process_test.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import unittest.mock as mock
import logging
from unittest.mock import patch
from unittest.mock import (
patch, Mock
)
from pytest import (
raises, fixture
)
Expand Down Expand Up @@ -61,7 +62,7 @@ def setup_method(self, cls):

@patch('kiwi.command.Command')
def test_returncode(self, mock_command):
command = mock.Mock()
command = Mock()
mock_command.return_value = command
process = CommandProcess(command)
assert process.returncode() == command.process.returncode
Expand Down Expand Up @@ -146,15 +147,15 @@ def test_create_match_method(self, mock_command):
)
assert match_method('a', 'b') is True

@patch('kiwi.command.Command')
def test_destructor(self, mock_command):
process = CommandProcess(mock_command)
process.command.command.process.returncode = None
process.command.command.process.pid = 42
process.command.command.process.kill = mock.Mock()
process.__del__()
process.command.command.process.kill.assert_called_once_with()

def test_command_iterator(self):
iterator = CommandIterator(mock.Mock())
iterator = CommandIterator(Mock())
assert iterator.__iter__() == iterator

def test_kill(self):
iterator = CommandIterator(Mock())
iterator.kill()
iterator.command.process.kill.assert_called_once_with()

def test_get_pid(self):
iterator = CommandIterator(Mock())
assert iterator.get_pid() == iterator.command.process.pid

0 comments on commit ba27291

Please sign in to comment.