diff options
author | Florian Bruhin <me@the-compiler.org> | 2021-03-23 12:53:59 +0100 |
---|---|---|
committer | Florian Bruhin <me@the-compiler.org> | 2021-03-23 12:53:59 +0100 |
commit | 4f4d6dc035937a4446a9b90b9313639d74c7267c (patch) | |
tree | 3284b0f03122b2d3d26dbfa6e7a705acb88d7da9 | |
parent | 43fb839f2c47a684531a7ae180dd5498b663dd12 (diff) | |
parent | 42978469d9d66cc84045454aa7d7adb64b2c0d9b (diff) | |
download | qutebrowser-4f4d6dc035937a4446a9b90b9313639d74c7267c.tar.gz qutebrowser-4f4d6dc035937a4446a9b90b9313639d74c7267c.zip |
Merge branch 'dev'
28 files changed, 847 insertions, 223 deletions
@@ -53,6 +53,9 @@ disallow_untyped_defs = True [mypy-qutebrowser.browser.webengine.webengineinspector] disallow_untyped_defs = True +[mypy-qutebrowser.misc.guiprocess] +disallow_untyped_defs = True + [mypy-qutebrowser.misc.objects] disallow_untyped_defs = True diff --git a/doc/changelog.asciidoc b/doc/changelog.asciidoc index edbd69513..b70de14cb 100644 --- a/doc/changelog.asciidoc +++ b/doc/changelog.asciidoc @@ -31,6 +31,8 @@ Added - New `input.media_keys` setting which can be used to disable Chromium's handling of media keys. +- New `:process` command (and associated `qute://process` pages) which can be + used to view and terminate/kill external processes spawned by qutebrowser. Changed ~~~~~~~ @@ -49,6 +51,22 @@ Changed clearer what kind of error occurred exactly. - The completion now also shows bindings starting with `set-cmd-text` in its third column, such as `o` for `:open`. +- When `:spawn` is used with the `-m` / `--output-messages` flag, the output now + appears live, while the process is running. +- When a shown message replaces an existing related one (e.g. for zoom levels), + the replacing now also works even if a different message was shown in between. + +Fixed +~~~~~ + +- When an editor exits with a != 0 exit status, the temporary editor file is now + persisted. This already was the case when the editor crashed. + +Removed +~~~~~~~ + +- The `qute://spawn-output` page used by `:spawn -o` is now removed, as it's + replaced by the new `qute://process` pages. [[v2.1.1]] v2.1.1 (unreleased) diff --git a/doc/help/commands.asciidoc b/doc/help/commands.asciidoc index fdd485b28..3386386ef 100644 --- a/doc/help/commands.asciidoc +++ b/doc/help/commands.asciidoc @@ -87,6 +87,7 @@ possible to run or bind multiple commands by separating them with `;;`. |<<nop,nop>>|Do nothing. |<<open,open>>|Open a URL in the current/[count]th tab. |<<print,print>>|Print the current/[count]th tab. +|<<process,process>>|Manage processes spawned by qutebrowser. |<<quickmark-add,quickmark-add>>|Add a new quickmark. |<<quickmark-del,quickmark-del>>|Delete a quickmark. |<<quickmark-load,quickmark-load>>|Load a quickmark. @@ -939,6 +940,23 @@ Print the current/[count]th tab. ==== count The tab index to print. +[[process]] +=== process +Syntax: +:process ['pid'] ['action']+ + +Manage processes spawned by qutebrowser. + +Note that processes with a successful exit get cleaned up after 1h. + +==== positional arguments +* +'pid'+: The process ID of the process to manage. +* +'action'+: What to do with the given process: + + - show: Show information about the process. + - terminate: Try to gracefully terminate the process (SIGTERM). + - kill: Kill the process forcefully (SIGKILL). + + [[quickmark-add]] === quickmark-add Syntax: +:quickmark-add 'url' 'name'+ diff --git a/qutebrowser/app.py b/qutebrowser/app.py index cdeffa7d3..cf5145511 100644 --- a/qutebrowser/app.py +++ b/qutebrowser/app.py @@ -72,6 +72,7 @@ from qutebrowser.utils import (log, version, message, utils, urlutils, objreg, # We import those to run the cmdutils.register decorators. from qutebrowser.mainwindow.statusbar import command from qutebrowser.misc import utilcmds +from qutebrowser.browser import commands # pylint: enable=unused-import diff --git a/qutebrowser/browser/browsertab.py b/qutebrowser/browser/browsertab.py index 581d33507..cc358a810 100644 --- a/qutebrowser/browser/browsertab.py +++ b/qutebrowser/browser/browsertab.py @@ -943,7 +943,7 @@ class AbstractTab(QWidget): _insecure_hosts: Set[str] = set() def __init__(self, *, win_id: int, - mode_manager: modeman.ModeManager, + mode_manager: 'modeman.ModeManager', private: bool, parent: QWidget = None) -> None: utils.unused(mode_manager) # needed for mypy diff --git a/qutebrowser/browser/commands.py b/qutebrowser/browser/commands.py index f1710adb9..5f90237ea 100644 --- a/qutebrowser/browser/commands.py +++ b/qutebrowser/browser/commands.py @@ -25,7 +25,7 @@ import functools from typing import cast, Callable, Dict, Union from PyQt5.QtWidgets import QApplication, QTabBar -from PyQt5.QtCore import pyqtSlot, Qt, QUrl, QEvent, QUrlQuery +from PyQt5.QtCore import Qt, QUrl, QEvent, QUrlQuery from qutebrowser.commands import userscripts, runners from qutebrowser.api import cmdutils @@ -1077,19 +1077,19 @@ class CommandDispatcher: log.procs.debug("Executing {} with args {}, userscript={}".format( cmd, args, userscript)) - @pyqtSlot() - def _on_proc_finished(): + def _on_proc_finished(proc): if output: tb = objreg.get('tabbed-browser', scope='window', window='last-focused') - tb.load_url(QUrl('qute://spawn-output'), newtab=True) + tb.load_url(QUrl(f'qute://process/{proc.pid}'), newtab=True) if userscript: def _selection_callback(s): try: runner = self._run_userscript( s, cmd, args, verbose, output_messages, count) - runner.finished.connect(_on_proc_finished) + runner.finished.connect(functools.partial( + _on_proc_finished, runner.proc)) except cmdutils.CommandError as e: message.error(str(e)) @@ -1112,7 +1112,7 @@ class CommandDispatcher: "detailed error") else: proc.start(cmd, args) - proc.finished.connect(_on_proc_finished) + proc.finished.connect(functools.partial(_on_proc_finished, proc)) def _run_userscript(self, selection, cmd, args, verbose, output_messages, count): @@ -1542,8 +1542,8 @@ class CommandDispatcher: elif going_up and tab.scroller.pos_px().y() > old_scroll_pos.y(): message.info("Search hit TOP, continuing at BOTTOM") else: - message.warning("Text '{}' not found on page!".format(text), - replace=True) + message.warning(f"Text '{text}' not found on page!", + replace='find-in-page') @cmdutils.register(instance='command-dispatcher', scope='window', maxsplit=0) diff --git a/qutebrowser/browser/eventfilter.py b/qutebrowser/browser/eventfilter.py index 78a6678cc..0b5fab096 100644 --- a/qutebrowser/browser/eventfilter.py +++ b/qutebrowser/browser/eventfilter.py @@ -164,7 +164,7 @@ class TabEventFilter(QObject): return True perc = int(100 * factor) - message.info("Zoom level: {}%".format(perc), replace=True) + message.info(f"Zoom level: {perc}%", replace='zoom-level') self._tab.zoom.set_factor(factor) return True diff --git a/qutebrowser/browser/hints.py b/qutebrowser/browser/hints.py index 333c532d3..7cdd0fd84 100644 --- a/qutebrowser/browser/hints.py +++ b/qutebrowser/browser/hints.py @@ -36,7 +36,7 @@ from PyQt5.QtWidgets import QLabel from qutebrowser.config import config, configexc from qutebrowser.keyinput import modeman, modeparsers, basekeyparser from qutebrowser.browser import webelem, history -from qutebrowser.commands import userscripts, runners +from qutebrowser.commands import runners from qutebrowser.api import cmdutils from qutebrowser.utils import usertypes, log, qtutils, message, objreg, utils if TYPE_CHECKING: @@ -272,7 +272,7 @@ class HintActions: msg = "Yanked URL to {}: {}".format( "primary selection" if sel else "clipboard", urlstr) - message.info(msg, replace=context.rapid) + message.info(msg, replace='rapid-hints' if context.rapid else None) def run_cmd(self, url: QUrl, context: HintContext) -> None: """Run the command based on a hint URL.""" @@ -320,6 +320,9 @@ class HintActions: elem: The QWebElement to use in the userscript. context: The HintContext to use. """ + # lazy import to avoid circular import issues + from qutebrowser.commands import userscripts + cmd = context.args[0] args = context.args[1:] env = { diff --git a/qutebrowser/browser/qutescheme.py b/qutebrowser/browser/qutescheme.py index cb04586ff..cfb238188 100644 --- a/qutebrowser/browser/qutescheme.py +++ b/qutebrowser/browser/qutescheme.py @@ -41,11 +41,11 @@ from qutebrowser.browser import pdfjs, downloads, history from qutebrowser.config import config, configdata, configexc from qutebrowser.utils import (version, utils, jinja, log, message, docutils, resources, objreg, standarddir) +from qutebrowser.misc import guiprocess from qutebrowser.qt import sip pyeval_output = ":pyeval was never called" -spawn_output = ":spawn was never called" csrf_token = None @@ -283,10 +283,24 @@ def qute_pyeval(_url: QUrl) -> _HandlerRet: return 'text/html', src -@add_handler('spawn-output') -def qute_spawn_output(_url: QUrl) -> _HandlerRet: - """Handler for qute://spawn-output.""" - src = jinja.render('pre.html', title='spawn output', content=spawn_output) +@add_handler('process') +def qute_process(url: QUrl) -> _HandlerRet: + """Handler for qute://process.""" + path = url.path()[1:] + try: + pid = int(path) + except ValueError: + raise UrlInvalidError(f"Invalid PID {path}") + + try: + proc = guiprocess.all_processes[pid] + except KeyError: + raise NotFoundError(f"No process {pid}") + + if proc is None: + raise NotFoundError(f"Data for process {pid} got cleaned up.") + + src = jinja.render('process.html', title=f'Process {pid}', proc=proc) return 'text/html', src diff --git a/qutebrowser/browser/shared.py b/qutebrowser/browser/shared.py index 41e7734f8..f2ff629f1 100644 --- a/qutebrowser/browser/shared.py +++ b/qutebrowser/browser/shared.py @@ -453,7 +453,7 @@ def _execute_fileselect_command( loop.exec() if tmpfilename is None: - selected_files = proc.final_stdout.splitlines() + selected_files = proc.stdout.splitlines() else: try: with open(tmpfilename, mode='r', encoding=sys.getfilesystemencoding()) as f: diff --git a/qutebrowser/browser/webengine/webenginequtescheme.py b/qutebrowser/browser/webengine/webenginequtescheme.py index 586f64bb1..64361f7c4 100644 --- a/qutebrowser/browser/webengine/webenginequtescheme.py +++ b/qutebrowser/browser/webengine/webenginequtescheme.py @@ -109,8 +109,7 @@ class QuteSchemeHandler(QWebEngineUrlSchemeHandler): QWebEngineUrlRequestJob.RequestFailed, } exctype = type(e) - log.network.error("{} while handling qute://* URL".format( - exctype.__name__)) + log.network.error(f"{exctype.__name__} while handling qute://* URL: {e}") job.fail(errors[exctype]) except qutescheme.Redirect as e: qtutils.ensure_valid(e.url) diff --git a/qutebrowser/commands/userscripts.py b/qutebrowser/commands/userscripts.py index 2ab29a2fd..f74d3ef59 100644 --- a/qutebrowser/commands/userscripts.py +++ b/qutebrowser/commands/userscripts.py @@ -117,7 +117,7 @@ class _BaseUserscriptRunner(QObject): super().__init__(parent) self._cleaned_up = False self._filepath = None - self._proc = None + self.proc = None self._env: MutableMapping[str, str] = {} self._text_stored = False self._html_stored = False @@ -168,12 +168,12 @@ class _BaseUserscriptRunner(QObject): if env is not None: self._env.update(env) - self._proc = guiprocess.GUIProcess( + self.proc = guiprocess.GUIProcess( 'userscript', additional_env=self._env, output_messages=output_messages, verbose=verbose, parent=self) - self._proc.finished.connect(self.on_proc_finished) - self._proc.error.connect(self.on_proc_error) - self._proc.start(cmd, args) + self.proc.finished.connect(self.on_proc_finished) + self.proc.error.connect(self.on_proc_error) + self.proc.start(cmd, args) def _cleanup(self): """Clean up temporary files.""" @@ -199,7 +199,7 @@ class _BaseUserscriptRunner(QObject): fn, e)) self._filepath = None - self._proc = None + self.proc = None self._env = {} self._text_stored = False self._html_stored = False diff --git a/qutebrowser/completion/models/miscmodels.py b/qutebrowser/completion/models/miscmodels.py index b45127651..7642bf904 100644 --- a/qutebrowser/completion/models/miscmodels.py +++ b/qutebrowser/completion/models/miscmodels.py @@ -20,6 +20,7 @@ """Functions that return miscellaneous completion models.""" import datetime +import itertools from typing import List, Sequence, Tuple from qutebrowser.config import config, configdata @@ -302,3 +303,25 @@ def undo(*, info): cat = listcategory.ListCategory("Closed tabs", entries, sort=False) model.add_category(cat) return model + + +def process(*, info): + """A CompletionModel filled with processes.""" + utils.unused(info) + from qutebrowser.misc import guiprocess + model = completionmodel.CompletionModel(column_widths=(10, 10, 80)) + for what, processes in itertools.groupby( + (p for p in guiprocess.all_processes.values() if p is not None), + lambda proc: proc.what): + + # put successful processes last + sorted_processes = sorted( + processes, + key=lambda proc: proc.outcome.state_str() == 'successful', + ) + + entries = [(str(proc.pid), proc.outcome.state_str(), str(proc)) + for proc in sorted_processes] + cat = listcategory.ListCategory(what.capitalize(), entries, sort=False) + model.add_category(cat) + return model diff --git a/qutebrowser/components/zoomcommands.py b/qutebrowser/components/zoomcommands.py index 877bb1552..72715b730 100644 --- a/qutebrowser/components/zoomcommands.py +++ b/qutebrowser/components/zoomcommands.py @@ -37,7 +37,7 @@ def zoom_in(tab: apitypes.Tab, count: int = 1, quiet: bool = False) -> None: except ValueError as e: raise cmdutils.CommandError(e) if not quiet: - message.info("Zoom level: {}%".format(int(perc)), replace=True) + message.info("Zoom level: {}%".format(int(perc)), replace='zoom-level') @cmdutils.register() @@ -55,7 +55,7 @@ def zoom_out(tab: apitypes.Tab, count: int = 1, quiet: bool = False) -> None: except ValueError as e: raise cmdutils.CommandError(e) if not quiet: - message.info("Zoom level: {}%".format(int(perc)), replace=True) + message.info("Zoom level: {}%".format(int(perc)), replace='zoom-level') @cmdutils.register() @@ -92,4 +92,4 @@ def zoom(tab: apitypes.Tab, except ValueError: raise cmdutils.CommandError("Can't zoom {}%!".format(int_level)) if not quiet: - message.info("Zoom level: {}%".format(int_level), replace=True) + message.info("Zoom level: {}%".format(int_level), replace='zoom-level') diff --git a/qutebrowser/html/process.html b/qutebrowser/html/process.html new file mode 100644 index 000000000..2a024cedf --- /dev/null +++ b/qutebrowser/html/process.html @@ -0,0 +1,32 @@ +{% extends "styled.html" %} + +{% block content %} +<header><h1>Process {{ proc.pid }}: {{ proc.cmd }}</h1></header> + +<h2>Info</h2> + +<table> + <tr> + <td><b>Command</b></td> + <td><pre>{{ proc }}</pre></td> + </tr> + <tr> + <td><b>Status<b></td> + <td>{{ proc.outcome }}</td> + </tr> +</table> + +<h2>Standard output</h2> +{% if proc.stdout %} +<pre>{{ proc.stdout }}</pre> +{% else %} +No output. +{% endif %} + +<h2>Standard error</h2> +{% if proc.stderr %} +<pre>{{ proc.stderr }}</pre> +{% else %} +No output. +{% endif %} +{% endblock %} diff --git a/qutebrowser/mainwindow/mainwindow.py b/qutebrowser/mainwindow/mainwindow.py index ae30c6352..d0723742a 100644 --- a/qutebrowser/mainwindow/mainwindow.py +++ b/qutebrowser/mainwindow/mainwindow.py @@ -38,7 +38,7 @@ from qutebrowser.utils import (message, log, usertypes, qtutils, objreg, utils, from qutebrowser.mainwindow import messageview, prompt from qutebrowser.completion import completionwidget, completer from qutebrowser.keyinput import modeman -from qutebrowser.browser import commands, downloadview, hints, downloads +from qutebrowser.browser import downloadview, hints, downloads from qutebrowser.misc import crashsignal, keyhintwidget, sessions, objects from qutebrowser.qt import sip @@ -381,6 +381,8 @@ class MainWindow(QWidget): self._add_overlay(self._completion, self._completion.update_geometry) def _init_command_dispatcher(self): + # Lazy import to avoid circular imports + from qutebrowser.browser import commands self._command_dispatcher = commands.CommandDispatcher( self.win_id, self.tabbed_browser) objreg.register('command-dispatcher', diff --git a/qutebrowser/mainwindow/messageview.py b/qutebrowser/mainwindow/messageview.py index dedc311ee..58fcb3683 100644 --- a/qutebrowser/mainwindow/messageview.py +++ b/qutebrowser/mainwindow/messageview.py @@ -19,7 +19,7 @@ """Showing messages above the statusbar.""" -from typing import MutableSequence +from typing import MutableSequence, Optional from PyQt5.QtCore import pyqtSlot, pyqtSignal, QTimer, Qt from PyQt5.QtWidgets import QWidget, QVBoxLayout, QLabel, QSizePolicy @@ -32,9 +32,16 @@ class Message(QLabel): """A single error/warning/info message.""" - def __init__(self, level, text, replace, parent=None): + def __init__( + self, + level: usertypes.MessageLevel, + text: str, + replace: Optional[str], + parent: QWidget = None, + ) -> None: super().__init__(text, parent) self.replace = replace + self.level = level self.setAttribute(Qt.WA_StyledBackground, True) self.setWordWrap(True) qss = """ @@ -112,14 +119,25 @@ class MessageView(QWidget): self.hide() self._clear_timer.stop() - @pyqtSlot(usertypes.MessageLevel, str, bool) - def show_message(self, level, text, replace=False): + @pyqtSlot(usertypes.MessageLevel, str, str) + def show_message( + self, + level: usertypes.MessageLevel, + text: str, + replace: str = None, + ) -> None: """Show the given message with the given MessageLevel.""" if text == self._last_text: return - if replace and self._messages and self._messages[-1].replace: - self._remove_message(self._messages.pop()) + if replace: # None -> QString() -> '' + existing = [msg for msg in self._messages if msg.replace == replace] + if existing: + assert len(existing) == 1, existing + assert existing[0].level == level, (existing, level) + existing[0].setText(text) + self.update_geometry.emit() + return widget = Message(level, text, replace=replace, parent=self) self._vbox.addWidget(widget) diff --git a/qutebrowser/misc/editor.py b/qutebrowser/misc/editor.py index b399c01a6..5741c6b47 100644 --- a/qutebrowser/misc/editor.py +++ b/qutebrowser/misc/editor.py @@ -62,8 +62,13 @@ class ExternalEditor(QObject): self._watcher = QFileSystemWatcher(parent=self) if watch else None self._content = None - def _cleanup(self): - """Clean up temporary files after the editor closed.""" + def _cleanup(self, *, successful): + """Clean up temporary files after the editor closed. + + Args: + successful: Whether the editor exited successfully, i.e. the file can be + deleted. + """ assert self._remove_file is not None if (self._watcher is not None and not sip.isdeleted(self._watcher) and @@ -78,13 +83,16 @@ class ExternalEditor(QObject): assert self._proc is not None - try: - if self._proc.exit_status() != QProcess.CrashExit: + if successful: + try: os.remove(self._filename) - except OSError as e: - # NOTE: Do not replace this with "raise CommandError" as it's - # executed async. - message.error("Failed to delete tempfile... ({})".format(e)) + except OSError as e: + # NOTE: Do not replace this with "raise CommandError" as it's + # executed async. + message.error("Failed to delete tempfile... ({})".format(e)) + else: + message.info(f"Keeping file {self._filename} as the editor process exited " + "abnormally") @pyqtSlot(int, QProcess.ExitStatus) def _on_proc_closed(self, _exitcode, exitstatus): @@ -101,14 +109,16 @@ class ExternalEditor(QObject): # No error/cleanup here, since we already handle this in # on_proc_error. return + # do a final read to make sure we don't miss the last signal + assert self._proc is not None self._on_file_changed(self._filename) self.editing_finished.emit() - self._cleanup() + self._cleanup(successful=self._proc.outcome.was_successful()) @pyqtSlot(QProcess.ProcessError) def _on_proc_error(self, _err): - self._cleanup() + self._cleanup(successful=False) def edit(self, text, caret_position=None): """Edit a given text. diff --git a/qutebrowser/misc/guiprocess.py b/qutebrowser/misc/guiprocess.py index 95bfd64af..f99be39fa 100644 --- a/qutebrowser/misc/guiprocess.py +++ b/qutebrowser/misc/guiprocess.py @@ -19,14 +19,116 @@ """A QProcess which shows notifications in the GUI.""" +import dataclasses import locale import shlex +from typing import Mapping, Sequence, Dict, Optional from PyQt5.QtCore import (pyqtSlot, pyqtSignal, QObject, QProcess, - QProcessEnvironment) + QProcessEnvironment, QByteArray, QUrl, Qt) -from qutebrowser.utils import message, log, utils -from qutebrowser.browser import qutescheme +from qutebrowser.utils import message, log, utils, usertypes +from qutebrowser.api import cmdutils, apitypes +from qutebrowser.completion.models import miscmodels + + +all_processes: Dict[int, Optional['GUIProcess']] = {} +last_pid: Optional[int] = None + + +@cmdutils.register() +@cmdutils.argument('tab', value=cmdutils.Value.cur_tab) +@cmdutils.argument('pid', completion=miscmodels.process) +@cmdutils.argument('action', choices=['show', 'terminate', 'kill']) +def process(tab: apitypes.Tab, pid: int = None, action: str = 'show') -> None: + """Manage processes spawned by qutebrowser. + + Note that processes with a successful exit get cleaned up after 1h. + + Args: + pid: The process ID of the process to manage. + action: What to do with the given process: + + - show: Show information about the process. + - terminate: Try to gracefully terminate the process (SIGTERM). + - kill: Kill the process forcefully (SIGKILL). + """ + if pid is None: + if last_pid is None: + raise cmdutils.CommandError("No process executed yet!") + pid = last_pid + + try: + proc = all_processes[pid] + except KeyError: + raise cmdutils.CommandError(f"No process found with pid {pid}") + + if proc is None: + raise cmdutils.CommandError(f"Data for process {pid} got cleaned up") + + if action == 'show': + tab.load_url(QUrl(f'qute://process/{pid}')) + elif action == 'terminate': + proc.terminate() + elif action == 'kill': + proc.terminate(kill=True) + else: + raise utils.Unreachable(action) + + +@dataclasses.dataclass +class ProcessOutcome: + + """The outcome of a finished process.""" + + what: str + running: bool = False + status: Optional[QProcess.ExitStatus] = None + code: Optional[int] = None + + def was_successful(self) -> bool: + """Whether the process exited successfully. + + This must not be called if the process didn't exit yet. + """ + assert self.status is not None, "Process didn't finish yet" + assert self.code is not None + return self.status == QProcess.NormalExit and self.code == 0 + + def __str__(self) -> str: + if self.running: + return f"{self.what.capitalize()} is running." + elif self.status is None: + return f"{self.what.capitalize()} did not start." + + assert self.status is not None + assert self.code is not None + + if self.status == QProcess.CrashExit: + return f"{self.what.capitalize()} crashed." + elif self.was_successful(): + return f"{self.what.capitalize()} exited successfully." + + assert self.status == QProcess.NormalExit + # We call this 'status' here as it makes more sense to the user - + # it's actually 'code'. + return f"{self.what.capitalize()} exited with status {self.code}." + + def state_str(self) -> str: + """Get a short string describing the state of the process. + + This is used in the :process completion. + """ + if self.running: + return 'running' + elif self.status is None: + return 'not started' + elif self.status == QProcess.CrashExit: + return 'crashed' + elif self.was_successful(): + return 'successful' + else: + return 'unsuccessful' class GUIProcess(QObject): @@ -37,11 +139,11 @@ class GUIProcess(QObject): cmd: The command which was started. args: A list of arguments which gets passed. verbose: Whether to show more messages. + running: Whether the underlying process is started. + what: What kind of thing is spawned (process/editor/userscript/...). + Used in messages. _output_messages: Show output as messages. - _started: Whether the underlying process is started. _proc: The underlying QProcess. - _what: What kind of thing is spawned (process/editor/userscript/...). - Used in messages. Signals: error/finished/started signals proxied from QProcess. @@ -51,26 +153,42 @@ class GUIProcess(QObject): finished = pyqtSignal(int, QProcess.ExitStatus) started = pyqtSignal() - def __init__(self, what, *, verbose=False, additional_env=None, - output_messages=False, parent=None): + def __init__( + self, + what: str, + *, + verbose: bool = False, + additional_env: Mapping[str, str] = None, + output_messages: bool = False, + parent: QObject = None, + ): super().__init__(parent) - self._what = what + self.what = what self.verbose = verbose self._output_messages = output_messages - self._started = False - self.cmd = None - self.args = None + self.outcome = ProcessOutcome(what=what) + self.cmd: Optional[str] = None + self.args: Optional[Sequence[str]] = None + self.pid: Optional[int] = None - self.final_stdout: str = "" - self.final_stderr: str = "" + self.stdout: str = "" + self.stderr: str = "" + + self._cleanup_timer = usertypes.Timer(self, 'process-cleanup') + self._cleanup_timer.setTimerType(Qt.VeryCoarseTimer) + self._cleanup_timer.setInterval(3600 * 1000) # 1h + self._cleanup_timer.timeout.connect(self._on_cleanup_timer) + self._cleanup_timer.setSingleShot(True) self._proc = QProcess(self) + self._proc.setReadChannel(QProcess.StandardOutput) self._proc.errorOccurred.connect(self._on_error) self._proc.errorOccurred.connect(self.error) self._proc.finished.connect(self._on_finished) self._proc.finished.connect(self.finished) self._proc.started.connect(self._on_started) self._proc.started.connect(self.started) + self._proc.readyRead.connect(self._on_ready_read) # type: ignore[attr-defined] if additional_env is not None: procenv = QProcessEnvironment.systemEnvironment() @@ -78,14 +196,48 @@ class GUIProcess(QObject): procenv.insert(k, v) self._proc.setProcessEnvironment(procenv) + def __str__(self) -> str: + if self.cmd is None or self.args is None: + return f'<unknown {self.what} command>' + return ' '.join(shlex.quote(e) for e in [self.cmd] + list(self.args)) + + def _decode_data(self, qba: QByteArray) -> str: + """Decode data coming from a process.""" + encoding = locale.getpreferredencoding(do_setlocale=False) + return qba.data().decode(encoding, 'replace') + + @pyqtSlot() + def _on_ready_read(self) -> None: + if not self._output_messages: + return + + while True: + text = self._decode_data(self._proc.readLine()) # type: ignore[arg-type] + if not text: + break + + if '\r' in text and not utils.is_windows: + # Crude handling of CR for e.g. progress output. + # Discard everything before the last \r in the new input, then discard + # everything after the last \n in self.stdout. + text = text.rsplit('\r', maxsplit=1)[-1] + if '\n' in self.stdout: + self.stdout = self.stdout.rsplit('\n', maxsplit=1)[0] + '\n' + else: + self.stdout = '' + + self.stdout += text + + message.info(self._elide_output(self.stdout), replace=f"stdout-{self.pid}") + @pyqtSlot(QProcess.ProcessError) - def _on_error(self, error): + def _on_error(self, error: QProcess.ProcessError) -> None: """Show a message if there was an error while spawning.""" if error == QProcess.Crashed and not utils.is_windows: # Already handled via ExitStatus in _on_finished return - what = f"{self._what} {self.cmd!r}" + what = f"{self.what} {self.cmd!r}" error_descriptions = { QProcess.FailedToStart: f"{what.capitalize()} failed to start", QProcess.Crashed: f"{what.capitalize()} crashed", @@ -106,100 +258,111 @@ class GUIProcess(QObject): message.error(msg) + def _elide_output(self, output: str) -> str: + """Shorten long output before showing it.""" + output = output.strip() + lines = output.splitlines() + count = len(lines) + threshold = 20 + + if count > threshold: + lines = [ + f'[{count - threshold} lines hidden, see :process for the full output]' + ] + lines[-threshold:] + output = '\n'.join(lines) + + return output + @pyqtSlot(int, QProcess.ExitStatus) - def _on_finished(self, code, status): + def _on_finished(self, code: int, status: QProcess.ExitStatus) -> None: """Show a message when the process finished.""" - self._started = False log.procs.debug("Process finished with code {}, status {}.".format( code, status)) - encoding = locale.getpreferredencoding(do_setlocale=False) - stderr = self._proc.readAllStandardError().data().decode( - encoding, 'replace') - stdout = self._proc.readAllStandardOutput().data().decode( - encoding, 'replace') + self.outcome.running = False + self.outcome.code = code + self.outcome.status = status + + self.stderr += self._decode_data(self._proc.readAllStandardError()) + self.stdout += self._decode_data(self._proc.readAllStandardOutput()) if self._output_messages: - if stdout: - message.info(stdout.strip()) - if stderr: - message.error(stderr.strip()) - - if status == QProcess.CrashExit: - exitinfo = "{} crashed.".format(self._what.capitalize()) - message.error(exitinfo) - elif status == QProcess.NormalExit and code == 0: - exitinfo = "{} exited successfully.".format( - self._what.capitalize()) + if self.stdout: + message.info( + self._elide_output(self.stdout), replace=f"stdout-{self.pid}") + if self.stderr: + message.error(self._elide_output(self.stderr)) + + if self.outcome.was_successful(): if self.verbose: - message.info(exitinfo) + message.info(str(self.outcome)) + self._cleanup_timer.start() else: - assert status == QProcess.NormalExit - # We call this 'status' here as it makes more sense to the user - - # it's actually 'code'. - exitinfo = ("{} exited with status {}, see :messages for " - "details.").format(self._what.capitalize(), code) - message.error(exitinfo) - - if stdout: - log.procs.error("Process stdout:\n" + stdout.strip()) - if stderr: - log.procs.error("Process stderr:\n" + stderr.strip()) - - qutescheme.spawn_output = self._spawn_format(exitinfo, stdout, stderr) - self.final_stdout = stdout - self.final_stderr = stderr - - def _spawn_format(self, exitinfo, stdout, stderr): - """Produce a formatted string for spawn output.""" - stdout = (stdout or "(No output)").strip() - stderr = (stderr or "(No output)").strip() - - spawn_string = ("{}\n" - "\nProcess stdout:\n {}" - "\nProcess stderr:\n {}").format(exitinfo, - stdout, stderr) - return spawn_string + if self.stdout: + log.procs.error("Process stdout:\n" + self.stdout.strip()) + if self.stderr: + log.procs.error("Process stderr:\n" + self.stderr.strip()) + message.error(str(self.outcome) + " See :process for details.") @pyqtSlot() - def _on_started(self): + def _on_started(self) -> None: """Called when the process started successfully.""" log.procs.debug("Process started.") - assert not self._started - self._started = True + assert not self.outcome.running + self.outcome.running = True - def _pre_start(self, cmd, args): + def _pre_start(self, cmd: str, args: Sequence[str]) -> None: """Prepare starting of a QProcess.""" - if self._started: + if self.outcome.running: raise ValueError("Trying to start a running QProcess!") self.cmd = cmd self.args = args - fake_cmdline = ' '.join(shlex.quote(e) for e in [cmd] + list(args)) - log.procs.debug("Executing: {}".format(fake_cmdline)) + log.procs.debug(f"Executing: {self}") if self.verbose: - message.info('Executing: ' + fake_cmdline) + message.info(f'Executing: {self}') - def start(self, cmd, args): + def start(self, cmd: str, args: Sequence[str]) -> None: """Convenience wrapper around QProcess::start.""" log.procs.debug("Starting process.") self._pre_start(cmd, args) self._proc.start(cmd, args) + self._post_start() self._proc.closeWriteChannel() - def start_detached(self, cmd, args): + def start_detached(self, cmd: str, args: Sequence[str]) -> bool: """Convenience wrapper around QProcess::startDetached.""" log.procs.debug("Starting detached.") self._pre_start(cmd, args) - ok, _pid = self._proc.startDetached( + ok, self.pid = self._proc.startDetached( cmd, args, None) # type: ignore[call-arg] if not ok: - message.error("Error while spawning {}".format(self._what)) + message.error("Error while spawning {}".format(self.what)) return False log.procs.debug("Process started.") - self._started = True + self.outcome.running = True + self._post_start() return True - def exit_status(self): - return self._proc.exitStatus() + def _post_start(self) -> None: + """Register this process and remember the process ID after starting.""" + self.pid = self._proc.processId() + all_processes[self.pid] = self + global last_pid + last_pid = self.pid + + @pyqtSlot() + def _on_cleanup_timer(self) -> None: + """Remove the process from all registered processes.""" + log.procs.debug(f"Cleaning up data for {self.pid}") + assert self.pid in all_processes + all_processes[self.pid] = None + self.deleteLater() + + def terminate(self, kill: bool = False) -> None: + """Terminate or kill the process.""" + if kill: + self._proc.kill() + else: + self._proc.terminate() diff --git a/qutebrowser/utils/message.py b/qutebrowser/utils/message.py index e2d922202..50a438637 100644 --- a/qutebrowser/utils/message.py +++ b/qutebrowser/utils/message.py @@ -23,7 +23,7 @@ """Message singleton so we don't have to define unneeded signals.""" import traceback -from typing import Any, Callable, Iterable, List, Tuple, Union +from typing import Any, Callable, Iterable, List, Tuple, Union, Optional from PyQt5.QtCore import pyqtSignal, pyqtBoundSignal, QObject @@ -42,7 +42,7 @@ def _log_stack(typ: str, stack: str) -> None: log.message.debug("Stack for {} message:\n{}".format(typ, stack_text)) -def error(message: str, *, stack: str = None, replace: bool = False) -> None: +def error(message: str, *, stack: str = None, replace: str = None) -> None: """Display an error message. Args: @@ -60,7 +60,7 @@ def error(message: str, *, stack: str = None, replace: bool = False) -> None: global_bridge.show(usertypes.MessageLevel.error, message, replace) -def warning(message: str, *, replace: bool = False) -> None: +def warning(message: str, *, replace: str = None) -> None: """Display a warning message. Args: @@ -72,7 +72,7 @@ def warning(message: str, *, replace: bool = False) -> None: global_bridge.show(usertypes.MessageLevel.warning, message, replace) -def info(message: str, *, replace: bool = False) -> None: +def info(message: str, *, replace: str = None) -> None: """Display an info message. Args: @@ -198,8 +198,7 @@ class GlobalMessageBridge(QObject): show_message: Show a message arg 0: A MessageLevel member arg 1: The text to show - arg 2: Whether to replace other messages with - replace=True. + arg 2: A message ID (as string) to replace, or None. prompt_done: Emitted when a prompt was answered somewhere. ask_question: Ask a question to the user. arg 0: The Question object to ask. @@ -210,7 +209,7 @@ class GlobalMessageBridge(QObject): mode_left: Emitted when a keymode was left in any window. """ - show_message = pyqtSignal(usertypes.MessageLevel, str, bool) + show_message = pyqtSignal(usertypes.MessageLevel, str, str) prompt_done = pyqtSignal(usertypes.KeyMode) ask_question = pyqtSignal(usertypes.Question, bool) mode_left = pyqtSignal(usertypes.KeyMode) @@ -219,7 +218,7 @@ class GlobalMessageBridge(QObject): def __init__(self, parent: QObject = None) -> None: super().__init__(parent) self._connected = False - self._cache: List[Tuple[usertypes.MessageLevel, str, bool]] = [] + self._cache: List[Tuple[usertypes.MessageLevel, str, Optional[str]]] = [] def ask(self, question: usertypes.Question, blocking: bool, *, @@ -239,7 +238,7 @@ class GlobalMessageBridge(QObject): def show(self, level: usertypes.MessageLevel, text: str, - replace: bool = False) -> None: + replace: str = None) -> None: """Show the given message.""" if self._connected: self.show_message.emit(level, text, replace) diff --git a/scripts/dev/run_vulture.py b/scripts/dev/run_vulture.py index 612b88637..1f0018488 100755 --- a/scripts/dev/run_vulture.py +++ b/scripts/dev/run_vulture.py @@ -61,7 +61,7 @@ def whitelist_generator(): # noqa: C901 yield 'scripts.utils.bg_colors' yield 'qutebrowser.misc.sql.SqliteErrorCode.CONSTRAINT' yield 'qutebrowser.misc.throttle.Throttle.set_delay' - yield 'qutebrowser.misc.guiprocess.GUIProcess.final_stderr' + yield 'qutebrowser.misc.guiprocess.GUIProcess.stderr' # Qt attributes yield 'PyQt5.QtWebKit.QWebPage.ErrorPageExtensionReturn().baseUrl' @@ -80,7 +80,6 @@ def whitelist_generator(): # noqa: C901 yield 'qutebrowser.utils.jinja.Loader.get_source' yield 'qutebrowser.utils.log.QtWarningFilter.filter' yield 'qutebrowser.browser.pdfjs.is_available' - yield 'qutebrowser.misc.guiprocess.spawn_output' yield 'qutebrowser.utils.usertypes.ExitStatus.reserved' yield 'QEvent.posted' yield 'log_stack' # from message.py diff --git a/tests/end2end/features/qutescheme.feature b/tests/end2end/features/qutescheme.feature index 1424bbf09..039434f1c 100644 --- a/tests/end2end/features/qutescheme.feature +++ b/tests/end2end/features/qutescheme.feature @@ -167,7 +167,7 @@ Feature: Special qute:// pages Scenario: qute://settings CSRF token (webengine) When I open qute://settings And I run :jseval const xhr = new XMLHttpRequest(); xhr.open("GET", "qute://settings/set"); xhr.send() - Then "RequestDeniedError while handling qute://* URL" should be logged + Then "RequestDeniedError while handling qute://* URL: Invalid CSRF token!" should be logged And the error "Invalid CSRF token for qute://settings!" should be shown # pdfjs support diff --git a/tests/helpers/stubs.py b/tests/helpers/stubs.py index 4c56cf76c..6998958b8 100644 --- a/tests/helpers/stubs.py +++ b/tests/helpers/stubs.py @@ -30,7 +30,7 @@ import builtins import importlib import types -from PyQt5.QtCore import pyqtSignal, QPoint, QProcess, QObject, QUrl +from PyQt5.QtCore import pyqtSignal, QPoint, QProcess, QObject, QUrl, QByteArray from PyQt5.QtGui import QIcon from PyQt5.QtNetwork import (QNetworkRequest, QAbstractNetworkCache, QNetworkCacheMetaData) @@ -193,13 +193,18 @@ class FakeNetworkReply: self.headers[key] = value -def fake_qprocess(): - """Factory for a QProcess mock which has the QProcess enum values.""" - m = mock.Mock(spec=QProcess) - for name in ['NormalExit', 'CrashExit', 'FailedToStart', 'Crashed', - 'Timedout', 'WriteError', 'ReadError', 'UnknownError']: - setattr(m, name, getattr(QProcess, name)) - return m +class FakeProcess(QProcess): + + def __init__(self, parent: QObject = None) -> None: + super().__init__(parent) + self.start = mock.Mock(spec=QProcess.start) + self.startDetached = mock.Mock(spec=QProcess.startDetached) + self.readAllStandardOutput = mock.Mock( + spec=QProcess.readAllStandardOutput, return_value=QByteArray(b'')) + self.readAllStandardError = mock.Mock( + spec=QProcess.readAllStandardError, return_value=QByteArray(b'')) + self.terminate = mock.Mock(spec=QProcess.terminate) + self.kill = mock.Mock(spec=QProcess.kill) class FakeWebTabScroller(browsertab.AbstractScroller): @@ -287,6 +292,9 @@ class FakeWebTab(browsertab.AbstractTab): def renderer_process_pid(self): return None + def load_url(self, url): + self._url = url + class FakeSignal: diff --git a/tests/unit/browser/test_qutescheme.py b/tests/unit/browser/test_qutescheme.py index 2ae939596..45474102d 100644 --- a/tests/unit/browser/test_qutescheme.py +++ b/tests/unit/browser/test_qutescheme.py @@ -29,6 +29,7 @@ import pytest from qutebrowser.browser import qutescheme, pdfjs, downloads from qutebrowser.utils import resources +from qutebrowser.misc import guiprocess class TestJavascriptHandler: @@ -73,6 +74,35 @@ class TestJavascriptHandler: qutescheme.qute_javascript(url) +class TestProcessHandler: + + """Test the qute://process endpoint.""" + + def test_invalid_pid(self): + with pytest.raises(qutescheme.UrlInvalidError, match='Invalid PID blah'): + qutescheme.qute_process(QUrl('qute://process/blah')) + + def test_missing_process(self, monkeypatch): + monkeypatch.setattr(guiprocess, 'all_processes', {}) + with pytest.raises(qutescheme.NotFoundError, match='No process 1234'): + qutescheme.qute_process(QUrl('qute://process/1234')) + + def test_existing_process(self, qtbot, py_proc): + proc = guiprocess.GUIProcess('testprocess') + + with qtbot.wait_signal(proc.finished, timeout=5000): + proc.start(*py_proc("print('AT&T')")) + + _mimetype, data = qutescheme.qute_process(QUrl(f'qute://process/{proc.pid}')) + print(data) + + assert f'<title>Process {proc.pid}</title>' in data + assert '-c 'print(' in data + assert 'Testprocess exited successfully.' in data + assert f'<pre>AT&T{os.linesep}</pre>' in data + assert 'No output.' in data # stderr + + class TestHistoryHandler: """Test the qute://history endpoint.""" diff --git a/tests/unit/completion/test_models.py b/tests/unit/completion/test_models.py index 12e623517..45506fe6a 100644 --- a/tests/unit/completion/test_models.py +++ b/tests/unit/completion/test_models.py @@ -31,7 +31,7 @@ from unittest import mock import hypothesis import hypothesis.strategies as hst import pytest -from PyQt5.QtCore import QUrl, QDateTime +from PyQt5.QtCore import QUrl, QDateTime, QProcess try: from PyQt5.QtWebEngineWidgets import ( QWebEngineHistory, QWebEngineHistoryItem @@ -39,7 +39,7 @@ try: except ImportError: pass -from qutebrowser.misc import objects +from qutebrowser.misc import objects, guiprocess from qutebrowser.completion import completer from qutebrowser.completion.models import ( configmodel, listcategory, miscmodels, urlmodel, filepathcategory) @@ -1447,6 +1447,53 @@ def undo_completion_retains_sort_order(tabbed_browser_stubs, info): _check_completions(model, {"Closed tabs": expected}) +def test_process_completion(monkeypatch, stubs, info): + monkeypatch.setattr(guiprocess, 'QProcess', stubs.FakeProcess) + p1 = guiprocess.GUIProcess('testprocess') + p2 = guiprocess.GUIProcess('testprocess') + p3 = guiprocess.GUIProcess('editor') + + p1.pid = 1001 + p1.cmd = 'cmd1' + p1.args = [] + p1.outcome.running = False + p1.outcome.status = QProcess.NormalExit + p1.outcome.code = 0 + + p2.pid = 1002 + p2.cmd = 'cmd2' + p2.args = [] + p2.outcome.running = True + + p3.pid = 1003 + p3.cmd = 'cmd3' + p3.args = [] + p3.outcome.running = False + p3.outcome.status = QProcess.NormalExit + p3.outcome.code = 1 + + monkeypatch.setattr(guiprocess, 'all_processes', { + 1001: p1, + 1002: p2, + 1003: p3, + 1004: None, # cleaned up + }) + + model = miscmodels.process(info=info) + model.set_pattern('') + + expected = { + 'Testprocess': [ + ('1002', 'running', 'cmd2'), + ('1001', 'successful', 'cmd1'), + ], + 'Editor': [ + ('1003', 'unsuccessful', 'cmd3'), + ], + } + _check_completions(model, expected) + + @hypothesis.given(text=hst.text()) def test_listcategory_hypothesis(text): """Make sure we can't produce invalid patterns.""" diff --git a/tests/unit/mainwindow/test_messageview.py b/tests/unit/mainwindow/test_messageview.py index 82328ffea..9e7627635 100644 --- a/tests/unit/mainwindow/test_messageview.py +++ b/tests/unit/mainwindow/test_messageview.py @@ -119,10 +119,11 @@ def test_show_multiple_messages_longer(view, count, expected): @pytest.mark.parametrize('replace1, replace2, length', [ - (False, False, 2), # Two stacked messages - (True, True, 1), # Two replaceable messages - (False, True, 2), # Stacked and replaceable - (True, False, 2), # Replaceable and stacked + (None, None, 2), # Two stacked messages + ('testid', 'testid', 1), # Two replaceable messages + (None, 'testid', 2), # Stacked and replaceable + ('testid', None, 2), # Replaceable and stacked + ('testid1', 'testid2', 2), # Different IDs ]) def test_replaced_messages(view, replace1, replace2, length): """Show two stack=False messages which should replace each other.""" @@ -131,6 +132,28 @@ def test_replaced_messages(view, replace1, replace2, length): assert len(view._messages) == length +def test_replacing_different_severity(view): + view.show_message(usertypes.MessageLevel.info, 'test', replace='testid') + with pytest.raises(AssertionError): + view.show_message(usertypes.MessageLevel.error, 'test 2', replace='testid') + + +def test_replacing_changed_text(view): + view.show_message(usertypes.MessageLevel.info, 'test', replace='testid') + view.show_message(usertypes.MessageLevel.info, 'test 2') + view.show_message(usertypes.MessageLevel.info, 'test 3', replace='testid') + assert len(view._messages) == 2 + assert view._messages[0].text() == 'test 3' + assert view._messages[1].text() == 'test 2' + + +def test_replacing_geometry(qtbot, view): + view.show_message(usertypes.MessageLevel.info, 'test', replace='testid') + + with qtbot.wait_signal(view.update_geometry): + view.show_message(usertypes.MessageLevel.info, 'test 2', replace='testid') + + @pytest.mark.parametrize('button, count', [ (Qt.LeftButton, 0), (Qt.MiddleButton, 0), diff --git a/tests/unit/misc/test_editor.py b/tests/unit/misc/test_editor.py index ee167e8bb..cd21308f8 100644 --- a/tests/unit/misc/test_editor.py +++ b/tests/unit/misc/test_editor.py @@ -24,17 +24,16 @@ import pathlib import os import logging -from PyQt5.QtCore import QProcess import pytest +from PyQt5.QtCore import QProcess from qutebrowser.misc import editor as editormod from qutebrowser.utils import usertypes @pytest.fixture(autouse=True) -def patch_things(config_stub, monkeypatch, stubs): - monkeypatch.setattr(editormod.guiprocess, 'QProcess', - stubs.fake_qprocess()) +def patch_fake_process(config_stub, monkeypatch, stubs): + monkeypatch.setattr(editormod.guiprocess, 'QProcess', stubs.FakeProcess) @pytest.fixture(params=[True, False]) @@ -43,7 +42,7 @@ def editor(caplog, qtbot, request): yield ed with caplog.at_level(logging.ERROR): ed._remove_file = True - ed._cleanup() + ed._cleanup(successful=True) class TestArg: @@ -79,7 +78,7 @@ class TestFileHandling: filename = pathlib.Path(editor._filename) assert filename.exists() assert filename.name.startswith('qutebrowser-editor-') - editor._proc.finished.emit(0, QProcess.NormalExit) + editor._proc._proc.finished.emit(0, QProcess.NormalExit) assert not filename.exists() @pytest.mark.parametrize('touch', [True, False]) @@ -90,35 +89,34 @@ class TestFileHandling: path.touch() editor.edit_file(str(path)) - editor._proc.finished.emit(0, QProcess.NormalExit) + editor._proc._proc.finished.emit(0, QProcess.NormalExit) assert path.exists() - def test_error(self, editor): + def test_error(self, editor, caplog): """Test file handling when closing with an exit status != 0.""" editor.edit("") filename = pathlib.Path(editor._filename) assert filename.exists() - editor._proc._proc.exitStatus = lambda: QProcess.CrashExit - editor._proc.finished.emit(1, QProcess.NormalExit) + with caplog.at_level(logging.ERROR): + editor._proc._proc.finished.emit(1, QProcess.NormalExit) assert filename.exists() filename.unlink() - def test_crash(self, editor): + def test_crash(self, editor, caplog): """Test file handling when closing with a crash.""" editor.edit("") filename = pathlib.Path(editor._filename) assert filename.exists() - editor._proc._proc.exitStatus = lambda: QProcess.CrashExit editor._proc.error.emit(QProcess.Crashed) + with caplog.at_level(logging.ERROR): + editor._proc._proc.finished.emit(0, QProcess.CrashExit) - editor._proc.finished.emit(0, QProcess.CrashExit) assert filename.exists() - filename.unlink() def test_unreadable(self, message_mock, editor, caplog, qtbot): @@ -132,7 +130,7 @@ class TestFileHandling: pytest.skip("File was still readable") with caplog.at_level(logging.ERROR): - editor._proc.finished.emit(0, QProcess.NormalExit) + editor._proc._proc.finished.emit(0, QProcess.NormalExit) assert not filename.exists() msg = message_mock.getmsg(usertypes.MessageLevel.error) assert msg.text.startswith("Failed to read back edited file: ") @@ -169,7 +167,7 @@ class TestFileHandling: fname = msg.text[len(prefix):] with qtbot.wait_signal(editor.editing_finished): - editor._proc.finished.emit(0, QProcess.NormalExit) + editor._proc._proc.finished.emit(0, QProcess.NormalExit) with open(fname, 'r', encoding='utf-8') as f: assert f.read() == 'bar' @@ -212,7 +210,7 @@ def test_modify(qtbot, editor, initial_text, edited_text): f.write(edited_text) with qtbot.wait_signal(editor.file_updated) as blocker: - editor._proc.finished.emit(0, QProcess.NormalExit) + editor._proc._proc.finished.emit(0, QProcess.NormalExit) assert blocker.args == [edited_text] @@ -246,7 +244,7 @@ def test_modify_watch(qtbot): assert blocker.args == ['baz'] with qtbot.assert_not_emitted(editor.file_updated): - editor._proc.finished.emit(0, QProcess.NormalExit) + editor._proc._proc.finished.emit(0, QProcess.NormalExit) def test_failing_watch(qtbot, caplog, monkeypatch): @@ -264,7 +262,7 @@ def test_failing_watch(qtbot, caplog, monkeypatch): _update_file(editor._filename, 'bar') with qtbot.wait_signal(editor.file_updated) as blocker: - editor._proc.finished.emit(0, QProcess.NormalExit) + editor._proc._proc.finished.emit(0, QProcess.NormalExit) assert blocker.args == ['bar'] message = 'Failed to watch path: {}'.format(editor._filename) @@ -281,7 +279,7 @@ def test_failing_unwatch(qtbot, caplog, monkeypatch): editor.edit('foo') with caplog.at_level(logging.ERROR): - editor._proc.finished.emit(0, QProcess.NormalExit) + editor._proc._proc.finished.emit(0, QProcess.NormalExit) message = 'Failed to unwatch paths: [{!r}]'.format(editor._filename) assert caplog.messages[-1] == message diff --git a/tests/unit/misc/test_guiprocess.py b/tests/unit/misc/test_guiprocess.py index 18e926fab..0b707d9db 100644 --- a/tests/unit/misc/test_guiprocess.py +++ b/tests/unit/misc/test_guiprocess.py @@ -19,14 +19,16 @@ """Tests for qutebrowser.misc.guiprocess.""" +import sys import logging import pytest -from PyQt5.QtCore import QProcess +from PyQt5.QtCore import QProcess, QUrl from qutebrowser.misc import guiprocess from qutebrowser.utils import usertypes, utils -from qutebrowser.browser import qutescheme +from qutebrowser.api import cmdutils +from qutebrowser.qt import sip @pytest.fixture() @@ -34,7 +36,7 @@ def proc(qtbot, caplog): """A fixture providing a GUIProcess and cleaning it up after the test.""" p = guiprocess.GUIProcess('testprocess') yield p - if p._proc.state() == QProcess.Running: + if not sip.isdeleted(p._proc) and p._proc.state() != QProcess.NotRunning: with caplog.at_level(logging.ERROR): with qtbot.wait_signal(p.finished, timeout=10000, raising=False) as blocker: @@ -48,22 +50,87 @@ def proc(qtbot, caplog): def fake_proc(monkeypatch, stubs): """A fixture providing a GUIProcess with a mocked QProcess.""" p = guiprocess.GUIProcess('testprocess') - monkeypatch.setattr(p, '_proc', stubs.fake_qprocess()) + monkeypatch.setattr(p, '_proc', stubs.FakeProcess()) return p +class TestProcessCommand: + + @pytest.fixture + def tab(self, fake_web_tab): + return fake_web_tab() + + def test_no_process(self, tab, monkeypatch): + monkeypatch.setattr(guiprocess, 'last_pid', None) + with pytest.raises(cmdutils.CommandError, match='No process executed yet!'): + guiprocess.process(tab) + + def test_last_pid(self, tab, monkeypatch, fake_proc): + monkeypatch.setattr(guiprocess, 'last_pid', 1234) + monkeypatch.setitem(guiprocess.all_processes, 1234, fake_proc) + + guiprocess.process(tab) + assert tab.url() == QUrl('qute://process/1234') + + def test_explicit_pid(self, tab, monkeypatch, fake_proc): + monkeypatch.setattr(guiprocess, 'last_pid', 1234) + monkeypatch.setitem(guiprocess.all_processes, 5678, fake_proc) + + guiprocess.process(tab, 5678) + assert tab.url() == QUrl('qute://process/5678') + + def test_inexistent_pid(self, tab): + with pytest.raises( + cmdutils.CommandError, match='No process found with pid 1337'): + guiprocess.process(tab, 1337) + + def test_cleaned_up_pid(self, tab, monkeypatch): + monkeypatch.setitem(guiprocess.all_processes, 1337, None) + with pytest.raises( + cmdutils.CommandError, match='Data for process 1337 got cleaned up'): + guiprocess.process(tab, 1337) + + def test_terminate(self, tab, monkeypatch, fake_proc): + monkeypatch.setitem(guiprocess.all_processes, 1234, fake_proc) + + guiprocess.process(tab, 1234, 'terminate') + fake_proc._proc.terminate.assert_called_with() + fake_proc._proc.kill.assert_not_called() + + def test_kill(self, tab, monkeypatch, fake_proc): + monkeypatch.setitem(guiprocess.all_processes, 1234, fake_proc) + + guiprocess.process(tab, 1234, 'kill') + fake_proc._proc.kill.assert_called_with() + fake_proc._proc.terminate.assert_not_called() + + +def test_not_started(proc): + assert str(proc.outcome) == 'Testprocess did not start.' + assert proc.outcome.state_str() == 'not started' + assert not proc.outcome.running + assert proc.outcome.status is None + assert proc.outcome.code is None + + with pytest.raises(AssertionError): + proc.outcome.was_successful() + + def test_start(proc, qtbot, message_mock, py_proc): """Test simply starting a process.""" with qtbot.wait_signals([proc.started, proc.finished], timeout=10000, order='strict'): - argv = py_proc("import sys; print('test'); sys.exit(0)") - proc.start(*argv) + cmd, args = py_proc("import sys; print('test'); sys.exit(0)") + proc.start(cmd, args) - expected = proc._spawn_format(exitinfo="Testprocess exited successfully.", - stdout="test", stderr="") assert not message_mock.messages - assert qutescheme.spawn_output == expected - assert proc.exit_status() == QProcess.NormalExit + + assert not proc.outcome.running + assert proc.outcome.status == QProcess.NormalExit + assert proc.outcome.code == 0 + assert str(proc.outcome) == 'Testprocess exited successfully.' + assert proc.outcome.state_str() == 'successful' + assert proc.outcome.was_successful() def test_start_verbose(proc, qtbot, message_mock, py_proc): @@ -72,17 +139,14 @@ def test_start_verbose(proc, qtbot, message_mock, py_proc): with qtbot.wait_signals([proc.started, proc.finished], timeout=10000, order='strict'): - argv = py_proc("import sys; print('test'); sys.exit(0)") - proc.start(*argv) + cmd, args = py_proc("import sys; print('test'); sys.exit(0)") + proc.start(cmd, args) - expected = proc._spawn_format(exitinfo="Testprocess exited successfully.", - stdout="test", stderr="") msgs = message_mock.messages assert msgs[0].level == usertypes.MessageLevel.info assert msgs[1].level == usertypes.MessageLevel.info assert msgs[0].text.startswith("Executing:") assert msgs[1].text == "Testprocess exited successfully." - assert qutescheme.spawn_output == expected @pytest.mark.parametrize('stdout', [True, False]) @@ -102,17 +166,17 @@ def test_start_output_message(proc, qtbot, caplog, message_mock, py_proc, with qtbot.wait_signals([proc.started, proc.finished], timeout=10000, order='strict'): - argv = py_proc(';'.join(code)) - proc.start(*argv) + cmd, args = py_proc(';'.join(code)) + proc.start(cmd, args) if stdout and stderr: stdout_msg = message_mock.messages[0] - stderr_msg = message_mock.messages[1] - msg_count = 2 + stderr_msg = message_mock.messages[-1] + msg_count = 3 # stdout is reported twice (once live) elif stdout: stdout_msg = message_mock.messages[0] stderr_msg = None - msg_count = 1 + msg_count = 2 # stdout is reported twice (once live) elif stderr: stdout_msg = None stderr_msg = message_mock.messages[0] @@ -127,11 +191,106 @@ def test_start_output_message(proc, qtbot, caplog, message_mock, py_proc, if stdout_msg is not None: assert stdout_msg.level == usertypes.MessageLevel.info assert stdout_msg.text == 'stdout text' - assert proc.final_stdout.strip() == "stdout text", proc.final_stdout + assert proc.stdout.strip() == "stdout text", proc.stdout if stderr_msg is not None: assert stderr_msg.level == usertypes.MessageLevel.error assert stderr_msg.text == 'stderr text' - assert proc.final_stderr.strip() == "stderr text", proc.final_stderr + assert proc.stderr.strip() == "stderr text", proc.stderr + + +cr_skip = pytest.mark.skipif( + utils.is_windows, reason='CR handling not implemented on Windows') + + +@pytest.mark.parametrize('line1, line2, expected1, expected2', [ + pytest.param( + 'First line\n', + 'Second line\n', + 'First line', + 'First line\nSecond line', + id='simple-output', + ), + pytest.param( + 'First line', + '\rSecond line', + 'First line', + 'Second line', + id='simple-cr', + marks=cr_skip, + ), + pytest.param( + 'First line\n', + '\rSecond line', + 'First line', + 'First line\nSecond line', + id='cr-after-newline', + marks=cr_skip, + ), + pytest.param( + 'First line\nSecond line\nThird line', + '\rNew line', + 'First line\nSecond line\nThird line', + 'First line\nSecond line\nNew line', + id='cr-multiple-lines', + marks=cr_skip, + ), + pytest.param( + 'First line', + 'Second line\rThird line', + 'First line', + 'Third line', + id='cr-middle-of-string', + marks=cr_skip, + ), +]) +def test_live_messages_output(qtbot, proc, py_proc, message_mock, + line1, line2, expected1, expected2): + proc._output_messages = True + + cmd, args = py_proc(r""" + import time, sys + print(sys.argv[1], flush=True, end='') + time.sleep(0.5) + print(sys.argv[2], flush=True, end='') + """) + args += [line1, line2] + + with qtbot.wait_signal(proc.finished, timeout=5000): + proc.start(cmd, args) + + if utils.is_windows: + expected1 = expected1.replace('\n', '\r\n') + expected2 = expected2.replace('\n', '\r\n') + + assert len(message_mock.messages) == 3 + assert all(msg.level == usertypes.MessageLevel.info + for msg in message_mock.messages) + + assert message_mock.messages[0].text == expected1 + assert message_mock.messages[1].text == expected2 + assert message_mock.messages[2].text == expected2 + + +@pytest.mark.parametrize('i, expected_lines', [ + (20, [str(i) for i in range(1, 21)]), + (25, (['[5 lines hidden, see :process for the full output]'] + + [str(i) for i in range(6, 26)])), +]) +def test_elided_output(qtbot, proc, py_proc, message_mock, i, expected_lines): + proc._output_messages = True + + cmd, args = py_proc(f""" + for i in range(1, {i+1}): + print(str(i)) + """) + + with qtbot.wait_signal(proc.finished, timeout=5000): + proc.start(cmd, args) + + assert all(msg.level == usertypes.MessageLevel.info + for msg in message_mock.messages) + + assert message_mock.messages[-1].text.splitlines() == expected_lines def test_start_env(monkeypatch, qtbot, py_proc): @@ -139,7 +298,7 @@ def test_start_env(monkeypatch, qtbot, py_proc): env = {'QUTEBROWSER_TEST_2': '2'} proc = guiprocess.GUIProcess('testprocess', additional_env=env) - argv = py_proc(""" + cmd, args = py_proc(""" import os import json import sys @@ -151,28 +310,29 @@ def test_start_env(monkeypatch, qtbot, py_proc): with qtbot.wait_signals([proc.started, proc.finished], timeout=10000, order='strict'): - proc.start(*argv) + proc.start(cmd, args) - data = qutescheme.spawn_output - assert 'QUTEBROWSER_TEST_1' in data - assert 'QUTEBROWSER_TEST_2' in data + assert 'QUTEBROWSER_TEST_1' in proc.stdout + assert 'QUTEBROWSER_TEST_2' in proc.stdout def test_start_detached(fake_proc): """Test starting a detached process.""" - argv = ['foo', 'bar'] + cmd = 'foo' + args = ['bar'] fake_proc._proc.startDetached.return_value = (True, 0) - fake_proc.start_detached(*argv) - fake_proc._proc.startDetached.assert_called_with(*list(argv) + [None]) + fake_proc.start_detached(cmd, args) + fake_proc._proc.startDetached.assert_called_with(cmd, args, None) def test_start_detached_error(fake_proc, message_mock, caplog): """Test starting a detached process with ok=False.""" - argv = ['foo', 'bar'] + cmd = 'foo' + args = ['bar'] fake_proc._proc.startDetached.return_value = (False, 0) with caplog.at_level(logging.ERROR): - fake_proc.start_detached(*argv) + fake_proc.start_detached(cmd, args) msg = message_mock.getmsg(usertypes.MessageLevel.error) expected = "Error while spawning testprocess" assert msg.text == expected @@ -181,8 +341,8 @@ def test_start_detached_error(fake_proc, message_mock, caplog): def test_double_start(qtbot, proc, py_proc): """Test starting a GUIProcess twice.""" with qtbot.wait_signal(proc.started, timeout=10000): - argv = py_proc("import time; time.sleep(10)") - proc.start(*argv) + cmd, args = py_proc("import time; time.sleep(10)") + proc.start(cmd, args) with pytest.raises(ValueError): proc.start('', []) @@ -191,12 +351,12 @@ def test_double_start_finished(qtbot, proc, py_proc): """Test starting a GUIProcess twice (with the first call finished).""" with qtbot.wait_signals([proc.started, proc.finished], timeout=10000, order='strict'): - argv = py_proc("import sys; sys.exit(0)") - proc.start(*argv) + cmd, args = py_proc("import sys; sys.exit(0)") + proc.start(cmd, args) with qtbot.wait_signals([proc.started, proc.finished], timeout=10000, order='strict'): - argv = py_proc("import sys; sys.exit(0)") - proc.start(*argv) + cmd, args = py_proc("import sys; sys.exit(0)") + proc.start(cmd, args) def test_cmd_args(fake_proc): @@ -219,8 +379,22 @@ def test_start_logging(fake_proc, caplog): ] -def test_error(qtbot, proc, caplog, message_mock): - """Test the process emitting an error.""" +def test_running(qtbot, proc, py_proc): + """Test proc.outcome while the process is still running.""" + with qtbot.wait_signal(proc.started, timeout=5000): + proc.start(*py_proc("import time; time.sleep(10)")) + assert proc.outcome.running + assert proc.outcome.status is None + assert proc.outcome.code is None + assert str(proc.outcome) == 'Testprocess is running.' + assert proc.outcome.state_str() == 'running' + + with pytest.raises(AssertionError): + proc.outcome.was_successful() + + +def test_failing_to_start(qtbot, proc, caplog, message_mock): + """Test the process failing to start.""" with caplog.at_level(logging.ERROR, 'message'): with qtbot.wait_signal(proc.error, timeout=5000): proc.start('this_does_not_exist_either', []) @@ -233,6 +407,15 @@ def test_error(qtbot, proc, caplog, message_mock): assert msg.text.endswith( "(Hint: Make sure 'this_does_not_exist_either' exists and is executable)") + assert not proc.outcome.running + assert proc.outcome.status is None + assert proc.outcome.code is None + assert str(proc.outcome) == 'Testprocess did not start.' + assert proc.outcome.state_str() == 'not started' + + with pytest.raises(AssertionError): + proc.outcome.was_successful() + def test_exit_unsuccessful(qtbot, proc, message_mock, py_proc, caplog): with caplog.at_level(logging.ERROR): @@ -240,10 +423,18 @@ def test_exit_unsuccessful(qtbot, proc, message_mock, py_proc, caplog): proc.start(*py_proc('import sys; sys.exit(1)')) msg = message_mock.getmsg(usertypes.MessageLevel.error) - expected = "Testprocess exited with status 1, see :messages for details." + expected = "Testprocess exited with status 1. See :process for details." assert msg.text == expected + assert not proc.outcome.running + assert proc.outcome.status == QProcess.NormalExit + assert proc.outcome.code == 1 + assert str(proc.outcome) == 'Testprocess exited with status 1.' + assert proc.outcome.state_str() == 'unsuccessful' + assert not proc.outcome.was_successful() + +@pytest.mark.posix # Can't seem to simulate a crash on Windows def test_exit_crash(qtbot, proc, message_mock, py_proc, caplog): with caplog.at_level(logging.ERROR): with qtbot.wait_signal(proc.finished, timeout=10000): @@ -252,12 +443,14 @@ def test_exit_crash(qtbot, proc, message_mock, py_proc, caplog): os.kill(os.getpid(), signal.SIGSEGV) """)) - expected = ( - "Testprocess exited with status 11, see :messages for details." - if utils.is_windows else "Testprocess crashed." - ) msg = message_mock.getmsg(usertypes.MessageLevel.error) - assert msg.text == expected + assert msg.text == "Testprocess crashed. See :process for details." + + assert not proc.outcome.running + assert proc.outcome.status == QProcess.CrashExit + assert str(proc.outcome) == 'Testprocess crashed.' + assert proc.outcome.state_str() == 'crashed' + assert not proc.outcome.was_successful() @pytest.mark.parametrize('stream', ['stdout', 'stderr']) @@ -265,12 +458,14 @@ def test_exit_unsuccessful_output(qtbot, proc, caplog, py_proc, stream): """When a process fails, its output should be logged.""" with caplog.at_level(logging.ERROR): with qtbot.wait_signal(proc.finished, timeout=10000): - proc.start(*py_proc(""" + proc.start(*py_proc(f""" import sys - print("test", file=sys.{}) + print("test", file=sys.{stream}) sys.exit(1) - """.format(stream))) - assert caplog.messages[-1] == 'Process {}:\ntest'.format(stream) + """)) + assert caplog.messages[-2] == 'Process {}:\ntest'.format(stream) + assert caplog.messages[-1] == ( + 'Testprocess exited with status 1. See :process for details.') @pytest.mark.parametrize('stream', ['stdout', 'stderr']) @@ -292,14 +487,35 @@ def test_stdout_not_decodable(proc, qtbot, message_mock, py_proc): """Test handling malformed utf-8 in stdout.""" with qtbot.wait_signals([proc.started, proc.finished], timeout=10000, order='strict'): - argv = py_proc(r""" + cmd, args = py_proc(r""" import sys # Using \x81 because it's invalid in UTF-8 and CP1252 sys.stdout.buffer.write(b"A\x81B") sys.exit(0) """) - proc.start(*argv) - expected = proc._spawn_format(exitinfo="Testprocess exited successfully.", - stdout="A\ufffdB", stderr="") + proc.start(cmd, args) + assert not message_mock.messages - assert qutescheme.spawn_output == expected + assert proc.stdout == "A\ufffdB" + + +def test_str_unknown(proc): + assert str(proc) == '<unknown testprocess command>' + + +def test_str(proc, py_proc): + proc.start(*py_proc("import sys")) + assert str(proc) in [ + f"'{sys.executable}' -c 'import sys'", # Sometimes sys.executable needs quoting + f"{sys.executable} -c 'import sys'", + ] + + +def test_cleanup(proc, py_proc, qtbot): + proc._cleanup_timer.setInterval(100) + + with qtbot.wait_signal(proc._cleanup_timer.timeout): + proc.start(*py_proc("")) + assert proc.pid in guiprocess.all_processes + + assert guiprocess.all_processes[proc.pid] is None |