From 89d3ccae0d544c596e43eff0a0d38afca65cdc4d Mon Sep 17 00:00:00 2001
From: Ruslan Kuprieiev <ruslan@iterative.ai>
Date: Mon, 19 Aug 2019 11:29:58 +0300
Subject: [PATCH] test: run: refactor signal handler tests

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>
---
 tests/func/test_run.py   | 63 ----------------------------------------
 tests/unit/test_stage.py | 25 ++++++++++++++++
 2 files changed, 25 insertions(+), 63 deletions(-)

diff --git a/tests/func/test_run.py b/tests/func/test_run.py
index 8ff5359159..383a4204a7 100644
--- a/tests/func/test_run.py
+++ b/tests/func/test_run.py
@@ -5,9 +5,6 @@
 import mock
 import shutil
 import filecmp
-import subprocess
-import signal
-import threading
 import pytest
 
 from dvc.main import main
@@ -272,66 +269,6 @@ def test_not_found(self):
             )
 
 
-@pytest.mark.skipif(
-    not isinstance(threading.current_thread(), threading._MainThread),
-    reason="Not running in the main thread.",
-)
-@mock.patch.object(subprocess.Popen, "wait", new=KeyboardInterrupt)
-def test_keyboard_interrupt(repo_dir, dvc_repo):
-    assert (
-        main(
-            [
-                "run",
-                "-d",
-                repo_dir.FOO,
-                "-d",
-                repo_dir.CODE,
-                "-o",
-                "out",
-                "-f",
-                "out.dvc",
-                "python",
-                repo_dir.CODE,
-                repo_dir.FOO,
-                "out",
-            ]
-        )
-        == 1
-    )
-
-
-@pytest.mark.skipif(
-    not isinstance(threading.current_thread(), threading._MainThread),
-    reason="Not running in the main thread.",
-)
-def test_keyboard_interrupt_after_second_signal_call(
-    mocker, repo_dir, dvc_repo
-):
-    mocker.patch.object(
-        signal, "signal", side_effect=[None, KeyboardInterrupt]
-    )
-    assert (
-        main(
-            [
-                "run",
-                "-d",
-                repo_dir.FOO,
-                "-d",
-                repo_dir.CODE,
-                "-o",
-                "out",
-                "-f",
-                "out.dvc",
-                "python",
-                repo_dir.CODE,
-                repo_dir.FOO,
-                "out",
-            ]
-        )
-        == 252
-    )
-
-
 class TestRunRemoveOuts(TestDvc):
     def test(self):
         with open(self.CODE, "w+") as fobj:
diff --git a/tests/unit/test_stage.py b/tests/unit/test_stage.py
index a477059a8c..41460f9e6d 100644
--- a/tests/unit/test_stage.py
+++ b/tests/unit/test_stage.py
@@ -2,6 +2,10 @@
 from dvc.stage import Stage, StageUpdateError
 from dvc.dependency.repo import DependencyREPO
 
+import signal
+import threading
+import subprocess
+
 import mock
 import pytest
 from unittest import TestCase
@@ -82,3 +86,24 @@ def test_stage_update(mocker):
     is_repo_import.return_value = False
     with pytest.raises(StageUpdateError):
         stage.update()
+
+
+@pytest.mark.skipif(
+    not isinstance(threading.current_thread(), threading._MainThread),
+    reason="Not running in the main thread.",
+)
+def test_stage_run_ignore_sigint(mocker):
+    stage = Stage(None, "path")
+
+    proc = mocker.Mock()
+    communicate = mocker.Mock()
+    proc.configure_mock(returncode=0, communicate=communicate)
+    popen = mocker.patch.object(subprocess, "Popen", return_value=proc)
+    signal_mock = mocker.patch("signal.signal")
+
+    stage._run()
+
+    assert popen.called_once()
+    assert communicate.called_once_with()
+    signal_mock.assert_any_call(signal.SIGINT, signal.SIG_IGN)
+    assert signal.getsignal(signal.SIGINT) == signal.default_int_handler