diff --git a/porcupine/utils.py b/porcupine/utils.py index d490fbe0b..7e5259e03 100644 --- a/porcupine/utils.py +++ b/porcupine/utils.py @@ -706,6 +706,7 @@ def run_in_thread( done_callback: Callable[[bool, Union[str, _T]], None], *, check_interval_ms: int = 100, + daemon: bool = True, ) -> None: """Run ``blocking_function()`` in another thread. @@ -720,6 +721,11 @@ def run_in_thread( Internally, this function checks whether the thread has completed every 100 milliseconds by default (so 10 times per second). Specify *check_interval_ms* to customize this. + + Unlike :class:`threading.Thread`, this function uses a daemon thread by + default. This means that the thread will end forcefully when Porcupine + exits, and it might not get a chance to finish whatever it is doing. Pass + ``daemon=False`` to change this. """ root = porcupine.get_main_window() # any widget would do @@ -730,8 +736,6 @@ def thread_target() -> None: nonlocal value nonlocal error_traceback - # the logging module uses locks so calling it from another - # thread should be safe try: value = blocking_function() except Exception: @@ -747,7 +751,7 @@ def check() -> None: else: done_callback(False, error_traceback) - thread = threading.Thread(target=thread_target) + thread = threading.Thread(target=thread_target, daemon=daemon) thread.start() root.after_idle(check) diff --git a/tests/conftest.py b/tests/conftest.py index 54fe0b631..c444505d0 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -10,12 +10,13 @@ import sys import tempfile import tkinter +import traceback import appdirs import pytest import porcupine -from porcupine import dirs, get_main_window, get_tab_manager, plugins, tabs +from porcupine import dirs, get_main_window, get_tab_manager, plugins, tabs, utils from porcupine.__main__ import main @@ -100,6 +101,19 @@ def porcusession(monkeypatch_dirs): porcupine.quit() +# utils.run_in_thread() can make tests fragile +@pytest.fixture +def dont_run_in_thread(monkeypatch): + def func(blocking_function, done_callback, check_interval_ms=69, daemon=True): + try: + result = blocking_function() + except Exception: + done_callback(False, traceback.format_exc()) + else: + done_callback(True, result) + monkeypatch.setattr(utils, 'run_in_thread', func) + + @pytest.fixture def tabmanager(porcusession): assert not get_tab_manager().tabs(), "something hasn't cleaned up its tabs" diff --git a/tests/test_directory_tree_plugin.py b/tests/test_directory_tree_plugin.py index 2df70ce5c..44a76c062 100644 --- a/tests/test_directory_tree_plugin.py +++ b/tests/test_directory_tree_plugin.py @@ -90,14 +90,6 @@ def get_project_names(): assert get_project_names() == ['c', 'a'] -@pytest.fixture -def dont_run_in_thread(monkeypatch): - def func(blocking_function, done_callback, check_interval_ms=69): - done_callback(True, blocking_function()) - - monkeypatch.setattr(utils, 'run_in_thread', func) - - @pytest.mark.skipif(shutil.which('git') is None, reason="git not found") def test_added_and_modified_content(tree, tmp_path, monkeypatch, dont_run_in_thread): monkeypatch.chdir(tmp_path) diff --git a/tests/test_mergeconflict_plugin.py b/tests/test_mergeconflict_plugin.py index c307c39d9..a6ed82d34 100644 --- a/tests/test_mergeconflict_plugin.py +++ b/tests/test_mergeconflict_plugin.py @@ -1,4 +1,3 @@ -import os import pathlib import shutil import subprocess @@ -22,14 +21,15 @@ @pytest.mark.skipif(shutil.which('git') is None, reason="need git to make merge conflicts") -@pytest.mark.skipif( - os.getenv('GITHUB_ACTIONS') == 'true', reason="somehow doesn't work in gh actions" -) def test_merge_conflict_string(tmp_path, monkeypatch, capfd): monkeypatch.chdir(tmp_path) file_content = 'before\nhello\nafter\n' subprocess.run(['git', 'init']) + # No --global, only affects test repo + subprocess.run(['git', 'config', 'user.name', 'foo']) + subprocess.run(['git', 'config', 'user.email', 'foo@example.com']) + pathlib.Path('foo.txt').write_text(file_content) subprocess.run(['git', 'add', 'foo.txt']) subprocess.run(['git', 'commit', '-m', 'create foo.txt']) @@ -41,7 +41,7 @@ def test_merge_conflict_string(tmp_path, monkeypatch, capfd): subprocess.run(['git', 'commit', '--all', '-m', 'hello my friend']) subprocess.run(['git', 'merge', 'other_branch']) assert pathlib.Path('foo.txt').read_text() == merge_conflict_string - capfd.readouterr() # prevents unnecessary prints from git + capfd.readouterr() # hide unnecessary prints from git def test_find_merge_conflicts(filetab): diff --git a/tests/test_pastebin_plugin.py b/tests/test_pastebin_plugin.py index 69838206f..b8ba8fa50 100644 --- a/tests/test_pastebin_plugin.py +++ b/tests/test_pastebin_plugin.py @@ -1,7 +1,5 @@ -import os import re import socket -import sys import threading import time import tkinter @@ -100,8 +98,7 @@ def test_success_dialog(monkeypatch): dialog.destroy() -@pytest.mark.skipif(sys.platform == 'darwin', reason="freezes Mac CI if menubar stuff is buggy") -def test_lots_of_stuff_with_localhost_termbin(filetab, monkeypatch, tabmanager): +def test_lots_of_stuff_with_localhost_termbin(filetab, monkeypatch, tabmanager, dont_run_in_thread): with socket.socket() as termbin: termbin.bind(('localhost', 0)) termbin.listen(1) @@ -138,10 +135,7 @@ def fake_wait_window(success_dialog): assert thread_done and fake_wait_window_done -@pytest.mark.skipif( - os.getenv('GITHUB_ACTIONS') == 'true', reason="somehow doesn't work with gh actions" -) -def test_paste_error_handling(monkeypatch, caplog, mocker, tabmanager, filetab): +def test_paste_error_handling(monkeypatch, caplog, mocker, tabmanager, filetab, dont_run_in_thread): monkeypatch.setattr(pastebin_module, 'DPASTE_URL', 'ThisIsNotValidUrlStart://wat') mocker.patch('porcupine.utils.errordialog')