From 0b0a5d4932641dde7f74e41cef229abcdae5f9a9 Mon Sep 17 00:00:00 2001 From: Stephan Sokolow Date: Sun, 7 Sep 2025 00:39:56 -0400 Subject: [PATCH 01/12] Fix vestigial mentions of old tooling --- docs/developing.rst | 64 +++++++++-------------------------------- tests/test_quicktile.py | 2 +- 2 files changed, 14 insertions(+), 52 deletions(-) diff --git a/docs/developing.rst b/docs/developing.rst index ba005a7..9f74300 100644 --- a/docs/developing.rst +++ b/docs/developing.rst @@ -41,8 +41,8 @@ On the operating system you intend to use for development: These dependencies fall into one of two categories: - * Source code verification (Flake8_ for static analysis and code style, - MyPy_ for checking type hints, Nose_ for test discovery, and + * Source code verification (Ruff_ for static analysis and code style, + MyPy_ for checking type hints, PyTest_ for test discovery, and `Coverage.py`_ for determining test coverage) * Documentation generation (Sphinx_, `sphinx-autodoc-typehints`_, and `sphinxcontrib-autoprogram`_) @@ -92,8 +92,6 @@ be searched for by running the following command in the project root: grep -E 'XXX|TODO|FIXME' -nR *.py quicktile functional_harness -PyLint_ should also report these. - Regenerating Documentation Graphics ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -175,10 +173,6 @@ Quirks of the Codebase's Structure commands and the shared setup code for them (lumped into a single class) as well as all of the commands themselves. -* The :mod:`quicktile.version` module exists only to allow :file:`setup.py` and - the rest of QuickTile to share a single definition of the version number - without :file:`setup.py` having to import actual QuickTile code. - .. todo:: Figure out a way to get URLs working in Sphinx's Graphviz_ extension that doesn't break when the default CSS downscales the diagram to keep it fitting in the document and then diagram QuickTile's functional @@ -203,8 +197,8 @@ revise them: * All function arguments should bear complete type annotations which pass MyPy's scrutiny and use of :any:`typing.Any` or ``# type: ignore`` must be approved on a case-by-case basis. -* All Flake8_ and PyLint_ complaints must either be resolved or whitelisted. - New ``NOQA`` or ``pylint: disable=`` annotations must include comments +* All Ruff_ complaints must either be resolved or whitelisted. + New whitelisting annotations must include comments justifying their presence, except in self-evident cases such as URLs in docstrings which exceed the line-length limit. * All code within the ``quicktile`` package must have complete API @@ -218,8 +212,7 @@ revise them: completed.) Once your changes are ready, the recommended way to submit them is via -`pull request`_, as this will automatically submit them to the various -continuous testing services that run on the QuickTile repository, as well +`pull request`_, as this will automatically trigger a test run, as well as making it as simple as possible for me to examine and accept them. .. _testing-quicktile: @@ -230,7 +223,9 @@ Testing Your Changes Testing Environment Concerns ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -QuickTile's current minimum compatibility target is Kubuntu Linux 20.04 LTS. +As of this writing, QuickTile's current minimum compatibility target is Kubuntu +Linux 24.04 LTS. This may be broadened as the testing infrastructure is +modernized. If this is not what you are running, I suggest using VirtualBox_ for compatibility testing, as it is easy to set up and has support for virtual @@ -275,54 +270,22 @@ For best results, configure your virtual desktop with the following characterist Automated Testing ^^^^^^^^^^^^^^^^^ -To run a complete set of everything that can be completed quickly, please use +To run a complete set of all tests, please use the following command from the root of the project: .. code-block:: sh ./run_tests.sh -It will perform the majority of the tests which will be run by Travis-CI when -you open a pull request, while still completing in under 5 seconds with a hot -cache on an old 2-core Althon with no SSD. - The following will be run: * MyPy_ to check for violations of the type annotations. -* Flake8_ for basic static analysis and code style checking -* Nose_ and doctest_ to run the unit tests (currently of limited scope) +* Ruff_ for basic static analysis and code style checking +* PyTest_ and doctest_ to run the unit tests (currently of limited scope) * doctest_ to check for broken code examples in the API documentation * Sphinx_'s ``make coverage`` to check documentation coverage (currently of questionable reliability) -While the dependency on system packages such as PyGObject limits its utility, -you may also use tox_ to test that QuickTile's ``setup.py`` packaging process -works properly. (However, bear in mind that you will need to edit ``tox.ini`` -if your system Python is not version 3.8 as found on Kubuntu Linux 20.04 LTS.) - -Bear in mind that, while not yet incorporated into convenient scripts, the -following tests will also be run by the ALE_ analysis plugin for my text editor -when I examine your contribution: - -* Bandit_ (You can run this as ``bandit quicktile`` after installation.) -* PyLint_ (Assuming you have your system configured to complain about - deprecation warnings as I do, I suggest running PyLint as - ``pylint3 --rcfile=pylintrc quicktile 2>/dev/null``) - -While it currently relies on an ugly hack which hard-codes Openbox and -Zenity as dependencies, and does not yet assert that windows wind up -in the expected states, you may also find the beginnings of a functional -test suite useful as a way to exercise the code and check for uncaught -exceptions: - -.. code-block:: sh - - ./test_functional.py -v - -Bear in mind that, even once it is more mature, it will remain excluded -from :file:`run_tests.sh` because it takes too long to be part of a -comfortable edit-test cycle. - In lieu of a proper functional test suite, please manually execute all tiling commands which rely on code you've touched and watch for misbehaviour. @@ -381,7 +344,7 @@ A Bad Example:: .. _Bandit: https://github.com/PyCQA/bandit .. _Coverage.py: https://coverage.readthedocs.io/ .. _doctest: https://docs.python.org/3/library/doctest.html -.. _Flake8: https://pypi.org/project/flake8/ +.. _Ruff: https://docs.astral.sh/ruff/ .. _Gifsicle: https://www.lcdf.org/gifsicle/ .. _GNU Make: https://www.gnu.org/software/make/ .. _Graphviz: https://www.graphviz.org/ @@ -389,9 +352,8 @@ A Bad Example:: .. _Inkscape: https://inkscape.org/ .. _issue tracker: https://github.com/ssokolow/quicktile/issues .. _MyPy: http://mypy-lang.org/ -.. _Nose: https://nose.readthedocs.io/ +.. _PyTest: https://docs.pytest.org/ .. _OptiPNG: http://optipng.sourceforge.net/ -.. _PyLint: https://www.pylint.org/ .. _pull request: https://github.com/ssokolow/quicktile/pulls .. _simple past tense: https://en.wikipedia.org/wiki/Simple_past .. _Sphinx: https://www.sphinx-doc.org/ diff --git a/tests/test_quicktile.py b/tests/test_quicktile.py index 5ec8353..7e1f989 100644 --- a/tests/test_quicktile.py +++ b/tests/test_quicktile.py @@ -1,5 +1,5 @@ # -*- coding: utf-8 -*- -"""Unit Test Suite for QuickTile using Nose test discovery""" +"""Unit Test Suite for QuickTile using PyTest test discovery""" __author__ = "Stephan Sokolow (deitarion/SSokolow)" __license__ = "GNU GPL 2.0 or later" From 2438c57d96865f37af11b21863c82122f8589dd3 Mon Sep 17 00:00:00 2001 From: Stephan Sokolow Date: Sun, 7 Sep 2025 00:40:10 -0400 Subject: [PATCH 02/12] Fix $PWD bug in run_tests.sh --- run_tests.sh | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/run_tests.sh b/run_tests.sh index 0a6d732..16a6ad2 100755 --- a/run_tests.sh +++ b/run_tests.sh @@ -1,14 +1,16 @@ #!/bin/bash cd -- "$(dirname -- "$(readlink -f -- "$0")")" || exit 1 -echo "--== Sphinx (documentation syntax) ==--" -cd docs || exit 1 -make -s SPHINXOPTS=-q coverage | grep -v '+--------------------------------+----------+--------------+' -echo "--== Sphinx (doctests) ==--" -make -s doctest +( + echo "--== Sphinx (documentation syntax) ==--" + cd docs || exit 1 + make -s SPHINXOPTS=-q coverage | grep -v '+--------------------------------+----------+--------------+' + echo "--== Sphinx (doctests) ==--" + make -s doctest +) echo "--== MyPy ==--" mypy --exclude=build --warn-unused-ignores . echo " --== Ruff (static analysis) ==--" ruff check . echo "--== PyTest (unit tests) ==--" -pytest "$@" +python3 -m pytest "$@" From 545d2860ef6f2f7fc45b92e2fa841dd84344b112 Mon Sep 17 00:00:00 2001 From: Stephan Sokolow Date: Sun, 7 Sep 2025 00:41:04 -0400 Subject: [PATCH 03/12] Merge nascent functional tests into PyTest run --- .github/workflows/python-app.yml | 4 +- .../functional_harness/env_general.rst | 2 +- docs/apidocs/functional_harness/x_server.rst | 2 +- docs/apidocs/test_functional.rst | 4 +- docs/developing.rst | 5 +- test_functional.py | 173 ------------------ .../functional_harness}/__init__.py | 0 .../functional_harness}/env_general.py | 0 .../functional_harness}/x_server.py | 0 tests/test_functional.py | 138 ++++++++++++++ 10 files changed, 148 insertions(+), 180 deletions(-) delete mode 100755 test_functional.py rename {functional_harness => tests/functional_harness}/__init__.py (100%) rename {functional_harness => tests/functional_harness}/env_general.py (100%) rename {functional_harness => tests/functional_harness}/x_server.py (100%) create mode 100755 tests/test_functional.py diff --git a/.github/workflows/python-app.yml b/.github/workflows/python-app.yml index 02fc2f5..f4c46ba 100644 --- a/.github/workflows/python-app.yml +++ b/.github/workflows/python-app.yml @@ -17,7 +17,7 @@ jobs: - uses: actions/checkout@v4 - uses: awalsh128/cache-apt-pkgs-action@latest with: - packages: python3 python3-pip python3-setuptools python3-pytest python3-pytest-cov python3-gi python3-xlib python3-dbus gir1.2-glib-2.0 gir1.2-gtk-3.0 gir1.2-wnck-3.0 + packages: python3 python3-pip python3-setuptools python3-pytest python3-pytest-cov python3-gi python3-xlib python3-dbus xvfb openbox gir1.2-glib-2.0 gir1.2-gtk-3.0 gir1.2-wnck-3.0 version: 1.0 - uses: actions/setup-python@v5 with: @@ -38,6 +38,6 @@ jobs: ruff check . --exit-zero --output-format=grouped - name: Test with PyTest run: | - pytest + python3 -m pytest - name: Coveralls uses: coverallsapp/github-action@v2 diff --git a/docs/apidocs/functional_harness/env_general.rst b/docs/apidocs/functional_harness/env_general.rst index 5bfba32..5840ab9 100644 --- a/docs/apidocs/functional_harness/env_general.rst +++ b/docs/apidocs/functional_harness/env_general.rst @@ -1,5 +1,5 @@ Environment Management (``functional_harness/env_general.py``) ============================================================== -.. automodule:: functional_harness.env_general +.. automodule:: tests.functional_harness.env_general :members: diff --git a/docs/apidocs/functional_harness/x_server.rst b/docs/apidocs/functional_harness/x_server.rst index 9de1853..c8d6420 100644 --- a/docs/apidocs/functional_harness/x_server.rst +++ b/docs/apidocs/functional_harness/x_server.rst @@ -1,5 +1,5 @@ X Server Management (``functional_harness/x_server.py``) ======================================================== -.. automodule:: functional_harness.x_server +.. automodule:: tests.functional_harness.x_server :members: diff --git a/docs/apidocs/test_functional.rst b/docs/apidocs/test_functional.rst index 15f9723..6e3e43e 100644 --- a/docs/apidocs/test_functional.rst +++ b/docs/apidocs/test_functional.rst @@ -1,7 +1,7 @@ Functional Tests (``test_functional.py``) ========================================= -.. automodule:: test_functional +.. automodule:: tests.test_functional :members: :exclude-members: TEST_SCRIPT @@ -10,4 +10,4 @@ Functional Tests (``test_functional.py``) Pending proper tests and assertions, the following sequence of commands is executed as an attempt to elicit uncaught exceptions. -.. pprint:: test_functional.TEST_SCRIPT +.. pprint:: tests.test_functional.TEST_SCRIPT diff --git a/docs/developing.rst b/docs/developing.rst index 9f74300..76175fc 100644 --- a/docs/developing.rst +++ b/docs/developing.rst @@ -31,6 +31,9 @@ On the operating system you intend to use for development: 2. Install the :ref:`runtime dependencies `. +3. Install Openbox and Xvfb (eg. ``sudo apt install xvfb openbox``) for the + functional tests. + 3. Either use the following command to install QuickTile's additional development-time dependencies, or manually install the dependencies listed therein: @@ -90,7 +93,7 @@ be searched for by running the following command in the project root: .. code-block:: sh - grep -E 'XXX|TODO|FIXME' -nR *.py quicktile functional_harness + grep -E 'XXX|TODO|FIXME' -nR *.py quicktile tests Regenerating Documentation Graphics ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/test_functional.py b/test_functional.py deleted file mode 100755 index e02649a..0000000 --- a/test_functional.py +++ /dev/null @@ -1,173 +0,0 @@ -#!/usr/bin/env python3 -# -*- coding: utf-8 -*- -"""Beginnings of a functional test harness for QuickTile - -.. todo:: Don't forget to test unusual configurations such as: - - 1. ``monitor-*`` commands with only one monitor - 2. ``workspace-*`` commands with only one workspace - 3. Having screens 1, 2, and 4 but not 0 or 3 (eg. hotplug aftermath) - 4. Having no windows on the desktop - 5. Having no window manager (with and without windows) - 6. Various Xinerama layouts - 7. Test with Xinerama disabled -""" - -from __future__ import (absolute_import, division, print_function, - with_statement, unicode_literals) - -__author__ = "Stephan Sokolow (deitarion/SSokolow)" -__license__ = "MIT" - -#: The sequence of commands to call QuickTile with -TEST_SCRIPT = """ -monitor-next-all -monitor-prev-all -monitor-switch-all -monitor-prev-all - -monitor-next -monitor-prev -monitor-switch -monitor-prev - -bottom -bottom-left -bottom-right -left -center -right -top -top-left -top-right - -move-to-bottom -move-to-bottom-left -move-to-bottom-right -move-to-center -move-to-left -move-to-right -move-to-top -move-to-top-left -move-to-top-right - -bordered -bordered - -always-above -always-above -always-below -always-below -horizontal-maximize -horizontal-maximize -vertical-maximize -vertical-maximize -shade -shade -fullscreen -fullscreen -all-desktops -all-desktops - -trigger-move -trigger-resize - -workspace-send-down -workspace-go-down - -workspace-send-up -workspace-go-up - -workspace-send-left -workspace-go-left - -workspace-send-right -workspace-go-right - -workspace-send-next -workspace-go-next - -workspace-send-prev -workspace-go-prev - -show-desktop -show-desktop - -maximize -maximize - -minimize -""" - -import logging, shlex, subprocess # nosec - -from functional_harness.env_general import background_proc -from functional_harness.x_server import x_server - -log = logging.getLogger(__name__) - - -def run_tests(env): - """Run the old bank of 'commands don't crash' tests""" - lines = [x.split('#')[0].strip() for x in TEST_SCRIPT.split('\n')] - lines = [x for x in lines if x] - for pos, command in enumerate(lines): - log.info("Testing command %d of %d: %s", pos + 1, len(lines), command) - subprocess.check_call(['./quicktile.sh', '--no-excepthook', - command], env=env) # nosec - - -def main(): - """The main entry point, compatible with setuptools entry points.""" - from argparse import ArgumentParser, RawDescriptionHelpFormatter - parser = ArgumentParser(formatter_class=RawDescriptionHelpFormatter, - description='Functional test runner for QuickTile', - epilog="NOTE: Running tests under the Xephyr X server will change " - "their behaviour if they depend on Xvfb's ability to present " - "a desktop with non-sequentially numbered screens.") - parser.add_argument('-v', '--verbose', action="count", - default=2, help="Increase the verbosity. Use twice for extra effect.") - parser.add_argument('-q', '--quiet', action="count", - default=0, help="Decrease the verbosity. Use twice for extra effect.") - parser.add_argument('-X', '--x-server', default="Xvfb", metavar="CMD", - help="The X server to launch for testing (Try 'Xephyr' to debug tests " - "using a live view. The default is '%(default)s'.)") - # Reminder: %(default)s can be used in help strings. - - args = parser.parse_args() - - # Set up clean logging to stderr - log_levels = [logging.CRITICAL, logging.ERROR, logging.WARNING, - logging.INFO, logging.DEBUG] - args.verbose = min(args.verbose - args.quiet, len(log_levels) - 1) - args.verbose = max(args.verbose, 0) - logging.basicConfig(level=log_levels[args.verbose], - format='%(levelname)s: %(message)s') - - log.warning("This does not currently check the results of the tiling " - "operations it requests. As such, it serves only as a way to check " - "for uncaught exceptions being raised in code that isn't yet " - "unit tested.") - log.warning("TODO: Inject a test window into the nested X session so " - "non-windowless commands don't bail out in the common code.") - log.info("Starting test instance of %s", args.x_server) - with x_server(shlex.split(args.x_server), - {0: '1024x768x24', 1: '800x600x24'}) as env: - # TODO: Use a custom configuration file for Openbox - # TODO: Detect once the window manager has started and wait for that - # before running the tests. - # TODO: Proper test windows. - log.info("Starting test copy of Openbox...") - with background_proc(['openbox', '--startup', 'zenity --info'], env): - # TODO: Rework so the process holding the session open can just - # *report* when it's ready. - log.info("Sleeping for 5 seconds...") - import time - time.sleep(5) - run_tests(env) - - -if __name__ == '__main__': - main() - -# vim: set sw=4 sts=4 expandtab : diff --git a/functional_harness/__init__.py b/tests/functional_harness/__init__.py similarity index 100% rename from functional_harness/__init__.py rename to tests/functional_harness/__init__.py diff --git a/functional_harness/env_general.py b/tests/functional_harness/env_general.py similarity index 100% rename from functional_harness/env_general.py rename to tests/functional_harness/env_general.py diff --git a/functional_harness/x_server.py b/tests/functional_harness/x_server.py similarity index 100% rename from functional_harness/x_server.py rename to tests/functional_harness/x_server.py diff --git a/tests/test_functional.py b/tests/test_functional.py new file mode 100755 index 0000000..ca736d3 --- /dev/null +++ b/tests/test_functional.py @@ -0,0 +1,138 @@ +#!/usr/bin/env python3 +# -*- coding: utf-8 -*- +"""Beginnings of a functional test harness for QuickTile + +.. todo:: Don't forget to test unusual configurations such as: + + 1. ``monitor-*`` commands with only one monitor + 2. ``workspace-*`` commands with only one workspace + 3. Having screens 1, 2, and 4 but not 0 or 3 (eg. hotplug aftermath) + 4. Having no windows on the desktop + 5. Having no window manager (with and without windows) + 6. Various Xinerama layouts + 7. Test with Xinerama disabled +""" + +from __future__ import (absolute_import, division, print_function, + with_statement, unicode_literals) + +__author__ = "Stephan Sokolow (deitarion/SSokolow)" +__license__ = "MIT" + +#: The sequence of commands to call QuickTile with +TEST_SCRIPT = [x.split('#')[0].strip() for x in """ +monitor-next-all +monitor-prev-all +monitor-switch-all +monitor-prev-all + +monitor-next +monitor-prev +monitor-switch +monitor-prev + +bottom +bottom-left +bottom-right +left +center +right +top +top-left +top-right + +move-to-bottom +move-to-bottom-left +move-to-bottom-right +move-to-center +move-to-left +move-to-right +move-to-top +move-to-top-left +move-to-top-right + +bordered +bordered + +always-above +always-above +always-below +always-below +horizontal-maximize +horizontal-maximize +vertical-maximize +vertical-maximize +shade +shade +fullscreen +fullscreen +all-desktops +all-desktops + +trigger-move +trigger-resize + +workspace-send-down +workspace-go-down + +workspace-send-up +workspace-go-up + +workspace-send-left +workspace-go-left + +workspace-send-right +workspace-go-right + +workspace-send-next +workspace-go-next + +workspace-send-prev +workspace-go-prev + +show-desktop +show-desktop + +maximize +maximize + +minimize +""".split() if x.split('#')[0].strip()] + +import logging, subprocess # nosec + +import pytest + +from tests.functional_harness.env_general import background_proc +from tests.functional_harness.x_server import x_server + +log = logging.getLogger(__name__) + + +@pytest.fixture(scope="session") +def icewm_session(): + # TODO: Re-add support for specifying 'Xephyr' + with x_server(["Xvfb"], {0: '1024x768x24', 1: '800x600x24'}) as env: + # TODO: Use a custom configuration file for Openbox + # TODO: Detect once the window manager has started and wait for that + # before running the tests. + # TODO: Proper test windows. + log.info("Starting test copy of Openbox...") + with background_proc(['openbox', '--startup', 'zenity --info'], env): + # TODO: Rework so the process holding the session open can just + # *report* when it's ready. + log.info("Sleeping for 5 seconds...") + import time + time.sleep(5) + yield env + + +@pytest.mark.parametrize("command", TEST_SCRIPT) +def test_functional(icewm_session, command): + """Run the old bank of 'commands don't crash' tests""" + log.info("Testing command: %s", command) + subprocess.check_call(['./quicktile.sh', '--no-excepthook', + command], env=icewm_session) # nosec + + +# vim: set sw=4 sts=4 expandtab : From e0bfdeb5d1f6d60e411bd7fe5e8ebf6bf410345c Mon Sep 17 00:00:00 2001 From: Stephan Sokolow Date: Sun, 7 Sep 2025 00:49:27 -0400 Subject: [PATCH 04/12] Revise contributing instructions for new submission flow --- docs/developing.rst | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/docs/developing.rst b/docs/developing.rst index 76175fc..267c10b 100644 --- a/docs/developing.rst +++ b/docs/developing.rst @@ -214,9 +214,10 @@ revise them: not be feasible until further refactoring and test harness work is completed.) -Once your changes are ready, the recommended way to submit them is via -`pull request`_, as this will automatically trigger a test run, as well -as making it as simple as possible for me to examine and accept them. +Once your changes are ready, the standard way to submit them is via `pull +request`_ against the ``master`` branch, as this will automatically trigger +a test run, as well as making it as simple as possible for me to examine and +accept them. .. _testing-quicktile: From 82595bd6b6b5276203b733a56e2b99d565271c47 Mon Sep 17 00:00:00 2001 From: Stephan Sokolow Date: Sun, 7 Sep 2025 01:18:53 -0400 Subject: [PATCH 05/12] Implement "wait for WM" in functional tests properly (enough) (To truly do it properly, after retrieving the window ID from _NET_SUPPORTING_WM_CHECK, it needs to check that the window exists... but given the scenario, that feels like YAGNI.) --- tests/functional_harness/env_general.py | 21 +++++++++++++++++++-- tests/test_functional.py | 25 ++++++++++++++++--------- 2 files changed, 35 insertions(+), 11 deletions(-) diff --git a/tests/functional_harness/env_general.py b/tests/functional_harness/env_general.py index c047d34..e5ca8a1 100644 --- a/tests/functional_harness/env_general.py +++ b/tests/functional_harness/env_general.py @@ -3,14 +3,31 @@ __author__ = "Stephan Sokolow (deitarion/SSokolow)" __license__ = "MIT" -import subprocess # nosec +import os, subprocess # nosec from contextlib import contextmanager # Silence PyLint being flat-out wrong about MyPy type annotations # pylint: disable=unsubscriptable-object # -- Type-Annotation Imports -- -from typing import Any, Generator, Mapping, Union # NOQA +from typing import Any, Dict, Generator, Mapping, Union # NOQA + + +@contextmanager +def os_environ(new_vars): + """Helper to work around APIs that don't [seem] to [easily] take custom + environment variables for things like specifying the X session to use.""" + + old_vars: Dict[str] = {} + for key in new_vars: + old_vars[key] = os.environ[key] + os.environ[key] = new_vars[key] + + try: + yield new_vars + finally: + for key in old_vars: + os.environ[key] = old_vars[key] @contextmanager diff --git a/tests/test_functional.py b/tests/test_functional.py index ca736d3..f4261ec 100755 --- a/tests/test_functional.py +++ b/tests/test_functional.py @@ -99,17 +99,18 @@ minimize """.split() if x.split('#')[0].strip()] -import logging, subprocess # nosec +import logging, subprocess, time # nosec import pytest +from quicktile.wm import WindowManager, Xatom -from tests.functional_harness.env_general import background_proc +from tests.functional_harness.env_general import background_proc, os_environ from tests.functional_harness.x_server import x_server log = logging.getLogger(__name__) -@pytest.fixture(scope="session") +@pytest.fixture(scope="module") def icewm_session(): # TODO: Re-add support for specifying 'Xephyr' with x_server(["Xvfb"], {0: '1024x768x24', 1: '800x600x24'}) as env: @@ -119,12 +120,18 @@ def icewm_session(): # TODO: Proper test windows. log.info("Starting test copy of Openbox...") with background_proc(['openbox', '--startup', 'zenity --info'], env): - # TODO: Rework so the process holding the session open can just - # *report* when it's ready. - log.info("Sleeping for 5 seconds...") - import time - time.sleep(5) - yield env + with os_environ(env): + wm = WindowManager() + + start = time.time() + while wm.get_property(wm.x_root.id, '_NET_SUPPORTING_WM_CHECK', + Xatom.WINDOW) is None: + if time.time() - start > 5: + raise Exception("Timed out waiting for window manager") + else: + time.sleep(0.1) + + yield env @pytest.mark.parametrize("command", TEST_SCRIPT) From 969eca51b4993db31dbba0235cac0d65b84d0b09 Mon Sep 17 00:00:00 2001 From: Stephan Sokolow Date: Sun, 7 Sep 2025 01:24:10 -0400 Subject: [PATCH 06/12] Add Zenity as a dependency for the functional suite (Currently, `zenity --info` is used to provide the target window) --- .github/workflows/python-app.yml | 2 +- docs/developing.rst | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/python-app.yml b/.github/workflows/python-app.yml index f4c46ba..c5a22cc 100644 --- a/.github/workflows/python-app.yml +++ b/.github/workflows/python-app.yml @@ -17,7 +17,7 @@ jobs: - uses: actions/checkout@v4 - uses: awalsh128/cache-apt-pkgs-action@latest with: - packages: python3 python3-pip python3-setuptools python3-pytest python3-pytest-cov python3-gi python3-xlib python3-dbus xvfb openbox gir1.2-glib-2.0 gir1.2-gtk-3.0 gir1.2-wnck-3.0 + packages: python3 python3-pip python3-setuptools python3-pytest python3-pytest-cov python3-gi python3-xlib python3-dbus xvfb openbox zenity gir1.2-glib-2.0 gir1.2-gtk-3.0 gir1.2-wnck-3.0 version: 1.0 - uses: actions/setup-python@v5 with: diff --git a/docs/developing.rst b/docs/developing.rst index 267c10b..99a39e8 100644 --- a/docs/developing.rst +++ b/docs/developing.rst @@ -31,8 +31,8 @@ On the operating system you intend to use for development: 2. Install the :ref:`runtime dependencies `. -3. Install Openbox and Xvfb (eg. ``sudo apt install xvfb openbox``) for the - functional tests. +3. Install Xvfb, Openbox, and Zenity (eg. ``sudo apt install xvfb openbox + zenity``) for the functional tests. 3. Either use the following command to install QuickTile's additional development-time dependencies, or manually install the dependencies listed From 10b6f81f4eba14e0d0d5c40b3ec3ffb6f3fb5d0d Mon Sep 17 00:00:00 2001 From: Stephan Sokolow Date: Sun, 7 Sep 2025 01:28:29 -0400 Subject: [PATCH 07/12] env_general.py: Fix oversight in MyPy annotations --- tests/functional_harness/env_general.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/functional_harness/env_general.py b/tests/functional_harness/env_general.py index e5ca8a1..5a4bb71 100644 --- a/tests/functional_harness/env_general.py +++ b/tests/functional_harness/env_general.py @@ -18,7 +18,7 @@ def os_environ(new_vars): """Helper to work around APIs that don't [seem] to [easily] take custom environment variables for things like specifying the X session to use.""" - old_vars: Dict[str] = {} + old_vars: Dict[str, str] = {} for key in new_vars: old_vars[key] = os.environ[key] os.environ[key] = new_vars[key] From c6b75c37cea6c59be43a933e52efe293c4653403 Mon Sep 17 00:00:00 2001 From: Stephan Sokolow Date: Sun, 7 Sep 2025 01:34:37 -0400 Subject: [PATCH 08/12] env_general.py: Don't assume the host session already has an X server --- tests/functional_harness/env_general.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/functional_harness/env_general.py b/tests/functional_harness/env_general.py index 5a4bb71..1615d13 100644 --- a/tests/functional_harness/env_general.py +++ b/tests/functional_harness/env_general.py @@ -20,7 +20,8 @@ def os_environ(new_vars): old_vars: Dict[str, str] = {} for key in new_vars: - old_vars[key] = os.environ[key] + if key in os.environ: + old_vars[key] = os.environ[key] os.environ[key] = new_vars[key] try: From 39b47d242ccf17b441d5129b536aff4cb8914502 Mon Sep 17 00:00:00 2001 From: Stephan Sokolow Date: Sun, 7 Sep 2025 06:20:14 -0400 Subject: [PATCH 09/12] Harmonize the name of the WM fixture until I'm ready to parameterize (It was a dumb oversight.) --- tests/test_functional.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/test_functional.py b/tests/test_functional.py index f4261ec..88bbbf1 100755 --- a/tests/test_functional.py +++ b/tests/test_functional.py @@ -111,7 +111,7 @@ @pytest.fixture(scope="module") -def icewm_session(): +def openbox_session(): # TODO: Re-add support for specifying 'Xephyr' with x_server(["Xvfb"], {0: '1024x768x24', 1: '800x600x24'}) as env: # TODO: Use a custom configuration file for Openbox @@ -135,11 +135,11 @@ def icewm_session(): @pytest.mark.parametrize("command", TEST_SCRIPT) -def test_functional(icewm_session, command): +def test_functional(openbox_session, command): """Run the old bank of 'commands don't crash' tests""" log.info("Testing command: %s", command) subprocess.check_call(['./quicktile.sh', '--no-excepthook', - command], env=icewm_session) # nosec + command], env=openbox_session) # nosec # vim: set sw=4 sts=4 expandtab : From 1eaf7d7d41956f0cc0c88b46fb2b8dbbb54b6a62 Mon Sep 17 00:00:00 2001 From: Stephan Sokolow Date: Sun, 7 Sep 2025 07:56:31 -0400 Subject: [PATCH 10/12] Switch to pytest-xvfb for functional tests (Since I'd been overlooking broken/unimplemented Zaphod-mode support anyway, so I might as well just ditch tests which rely on it.) --- .github/workflows/python-app.yml | 2 +- docs/apidocs/functional_harness/x_server.rst | 5 - docs/apidocs/tests.rst | 1 - quicktile/wm.py | 2 +- tests/functional_harness/env_general.py | 31 +---- tests/functional_harness/x_server.py | 127 ------------------- tests/test_functional.py | 42 +++--- 7 files changed, 25 insertions(+), 185 deletions(-) delete mode 100644 docs/apidocs/functional_harness/x_server.rst delete mode 100644 tests/functional_harness/x_server.py diff --git a/.github/workflows/python-app.yml b/.github/workflows/python-app.yml index c5a22cc..20449b2 100644 --- a/.github/workflows/python-app.yml +++ b/.github/workflows/python-app.yml @@ -17,7 +17,7 @@ jobs: - uses: actions/checkout@v4 - uses: awalsh128/cache-apt-pkgs-action@latest with: - packages: python3 python3-pip python3-setuptools python3-pytest python3-pytest-cov python3-gi python3-xlib python3-dbus xvfb openbox zenity gir1.2-glib-2.0 gir1.2-gtk-3.0 gir1.2-wnck-3.0 + packages: python3 python3-pip python3-setuptools python3-pytest python3-pytest-cov python3-pytest-xvfb python3-gi python3-xlib python3-dbus xvfb openbox zenity gir1.2-glib-2.0 gir1.2-gtk-3.0 gir1.2-wnck-3.0 version: 1.0 - uses: actions/setup-python@v5 with: diff --git a/docs/apidocs/functional_harness/x_server.rst b/docs/apidocs/functional_harness/x_server.rst deleted file mode 100644 index c8d6420..0000000 --- a/docs/apidocs/functional_harness/x_server.rst +++ /dev/null @@ -1,5 +0,0 @@ -X Server Management (``functional_harness/x_server.py``) -======================================================== - -.. automodule:: tests.functional_harness.x_server - :members: diff --git a/docs/apidocs/tests.rst b/docs/apidocs/tests.rst index 36619c8..c6f6928 100644 --- a/docs/apidocs/tests.rst +++ b/docs/apidocs/tests.rst @@ -11,5 +11,4 @@ rest of the code. test_util test_functional - functional_harness/x_server functional_harness/env_general diff --git a/quicktile/wm.py b/quicktile/wm.py index c385e4a..0f55280 100644 --- a/quicktile/wm.py +++ b/quicktile/wm.py @@ -83,7 +83,7 @@ def __init__(self, self.gdk_display = self.gdk_screen.get_display() try: - self.x_display = x_display or XDisplay() + self.x_display = x_display or XDisplay(self.gdk_display.get_name()) except (UnicodeDecodeError, DisplayConnectionError) as err: raise XInitError("python-xlib failed with %s when asked to open" " a connection to the X server. Cannot bind keys." diff --git a/tests/functional_harness/env_general.py b/tests/functional_harness/env_general.py index 1615d13..e61a683 100644 --- a/tests/functional_harness/env_general.py +++ b/tests/functional_harness/env_general.py @@ -3,7 +3,7 @@ __author__ = "Stephan Sokolow (deitarion/SSokolow)" __license__ = "MIT" -import os, subprocess # nosec +import subprocess # nosec from contextlib import contextmanager # Silence PyLint being flat-out wrong about MyPy type annotations @@ -14,27 +14,9 @@ @contextmanager -def os_environ(new_vars): - """Helper to work around APIs that don't [seem] to [easily] take custom - environment variables for things like specifying the X session to use.""" - - old_vars: Dict[str, str] = {} - for key in new_vars: - if key in os.environ: - old_vars[key] = os.environ[key] - os.environ[key] = new_vars[key] - - try: - yield new_vars - finally: - for key in old_vars: - os.environ[key] = old_vars[key] - - -@contextmanager -def background_proc(argv, env: Mapping[str, Union[bytes, str]], verbose=False, +def background_proc(argv, verbose=False, *args: Any, **kwargs: Any - ) -> Generator[None, None, None]: + ) -> Generator[subprocess.Popen[Any], None, None]: """Context manager for scoping the lifetime of a ``subprocess.Popen`` call :param argv: The command to be executed @@ -44,12 +26,11 @@ def background_proc(argv, env: Mapping[str, Union[bytes, str]], verbose=False, :param kwargs: Keyword arguments to pass to :class:`subprocess.Popen` """ if verbose: - popen_obj = subprocess.Popen( # type: ignore # nosec - argv, env=env, *args, **kwargs) + popen_obj = subprocess.Popen(argv, *args, **kwargs) else: - popen_obj = subprocess.Popen(argv, # type: ignore # nosec + popen_obj = subprocess.Popen(argv, # type: ignore stderr=subprocess.STDOUT, stdout=subprocess.DEVNULL, - env=env, *args, **kwargs) + *args, **kwargs) try: yield popen_obj finally: diff --git a/tests/functional_harness/x_server.py b/tests/functional_harness/x_server.py deleted file mode 100644 index 61336be..0000000 --- a/tests/functional_harness/x_server.py +++ /dev/null @@ -1,127 +0,0 @@ -#!/usr/bin/env python3 -"""Wrapper for easily setting up and tearing down a test X server""" - -__author__ = "Stephan Sokolow (deitarion/SSokolow)" -__license__ = "MIT" - -# Silence PyLint being flat-out wrong about MyPy type annotations -# complaining about my grouped imports -# pylint: disable=unsubscriptable-object,invalid-sequence-index -# pylint: disable=wrong-import-order - -import logging, os, random, shutil, subprocess, tempfile # nosec -from contextlib import contextmanager -from distutils.spawn import find_executable - -# -- Type-Annotation Imports -- -from typing import Dict, Generator, List, Tuple - -log = logging.getLogger(__name__) - - -def _init_x_server(argv: List[str], verbose: bool = False - ) -> Tuple[subprocess.Popen, bytes]: - """Wrapper for starting an X server with the given command line - - :param argv: The command-line to execute - :param verbose: If :any:`False`, redirect the X server's ``stdout`` and - ``stderr`` to :file:`/dev/null` - :returns: The process object for the X server. - - :raises subprocess.CalledProcessError: The X server exited with an - unexpected error. - """ - - # Launch the X server - read_pipe, write_pipe = os.pipe() - argv += ['+xinerama', '-displayfd', str(write_pipe)] - - env: Dict[str, str] = {} - - # pylint: disable=unexpected-keyword-arg,no-member - if verbose: - xproc = subprocess.Popen(argv, pass_fds=[write_pipe], env=env) # nosec - else: - xproc = subprocess.Popen(argv, pass_fds=[write_pipe], # nosec - env=env, stderr=subprocess.STDOUT, stdout=subprocess.DEVNULL) - - display = os.read(read_pipe, 128).strip() - return xproc, display - - -@contextmanager -def x_server(argv: List[str], screens: Dict[int, str] - ) -> Generator[Dict[str, str], None, None]: - """Context manager to launch and then clean up an X server. - - :param argv: The command to launch the test X server and - any arguments not relating to defining the attached screens. - :param screens: A :any:`dict ` mapping screen numbers to - ``WxHxDEPTH`` strings. (eg. ``{0: '1024x768x32'}``) - - :raises subprocess.CalledProcessError: The X server or :command:`xauth` - failed unexpectedly. - :raises FileNotFoundError: Could not find either the :command:`xauth` - command or ``argv[0]``. - :raises PermissionError: Somehow, we lack write permission inside a - directory created by :func:`tempfile.mkdtemp`. - :raises ValueError: ``argv[0]`` was not an X server binary we know how to - specify monitor rectangles for. - (either :command:`Xvfb` or :command:`Xephyr`) - :raises UnicodeDecodeError: The X server's ``-displayfd`` option wrote - a value to the given FD which could not be decoded as UTF-8 when it - should have been part of the 7-bit ASCII subset of UTF-8. - - .. todo:: Either don't accept an arbitrary ``argv`` string as input to - :func:`x_server` or default to a behaviour likely to work with other X - servers rather than erroring out. - """ - # Check for missing requirements - for cmd in ['xauth', argv[0]]: - if not find_executable(cmd): - # pylint: disable=undefined-variable - raise FileNotFoundError( # NOQA - "Cannot find required command {!r}".format(cmd)) - - x_server = None - tempdir = tempfile.mkdtemp() - try: - # Because random.getrandbits gets interpreted as a variable length, - # *ensure* we've got the right number of hex digits - magic_cookie = b'' - while len(magic_cookie) < 32: - magic_cookie += hex(random.getrandbits(128))[2:34].encode('ascii') - magic_cookie = magic_cookie[:32] - assert len(magic_cookie) == 32, len(magic_cookie) # nosec - xauthfile = os.path.join(tempdir, 'Xauthority') - env = {'XAUTHORITY': xauthfile} - - open(xauthfile, 'w').close() # create empty file - - # Convert `screens` into the format Xorg servers expect - screen_argv = [] - for screen_num, screen_geom in screens.items(): - if 'Xvfb' in argv[0]: - screen_argv.extend(['-screen', '%d' % screen_num, screen_geom]) - elif 'Xephyr' in argv[0]: - screen_argv.extend(['-screen', screen_geom]) - else: - raise ValueError("Unrecognized X server. Cannot infer format " - "for specifying screen geometry.") - - # Initialize an X server on a free display number - x_server, display_num = _init_x_server(argv + screen_argv) - - # Set up the environment and authorization - env['DISPLAY'] = ':%s' % display_num.decode('utf8') - subprocess.check_call( # nosec - ['xauth', 'add', env['DISPLAY'], '.', magic_cookie], - env=env) - # FIXME: This xauth call once had a random failure. Retry. - - yield env - - finally: - if x_server: - x_server.terminate() - shutil.rmtree(tempdir) diff --git a/tests/test_functional.py b/tests/test_functional.py index 88bbbf1..5087e46 100755 --- a/tests/test_functional.py +++ b/tests/test_functional.py @@ -104,37 +104,29 @@ import pytest from quicktile.wm import WindowManager, Xatom -from tests.functional_harness.env_general import background_proc, os_environ -from tests.functional_harness.x_server import x_server +from tests.functional_harness.env_general import background_proc log = logging.getLogger(__name__) @pytest.fixture(scope="module") def openbox_session(): - # TODO: Re-add support for specifying 'Xephyr' - with x_server(["Xvfb"], {0: '1024x768x24', 1: '800x600x24'}) as env: - # TODO: Use a custom configuration file for Openbox - # TODO: Detect once the window manager has started and wait for that - # before running the tests. - # TODO: Proper test windows. - log.info("Starting test copy of Openbox...") - with background_proc(['openbox', '--startup', 'zenity --info'], env): - with os_environ(env): - wm = WindowManager() - - start = time.time() - while wm.get_property(wm.x_root.id, '_NET_SUPPORTING_WM_CHECK', - Xatom.WINDOW) is None: - if time.time() - start > 5: - raise Exception("Timed out waiting for window manager") - else: - time.sleep(0.1) - - yield env - - -@pytest.mark.parametrize("command", TEST_SCRIPT) + log.info("Starting test copy of Openbox...") + with background_proc(['openbox', '--startup', 'zenity --info']): + wm = WindowManager() + + start = time.time() + while wm.get_property(wm.x_root.id, '_NET_SUPPORTING_WM_CHECK', + Xatom.WINDOW) is None: + if time.time() - start > 5: + raise Exception("Timed out waiting for window manager") + else: + time.sleep(0.1) + + yield + + +@ pytest.mark.parametrize("command", TEST_SCRIPT) def test_functional(openbox_session, command): """Run the old bank of 'commands don't crash' tests""" log.info("Testing command: %s", command) From b7685de7d4b38fee9332ce1430c6b31bd15c9438 Mon Sep 17 00:00:00 2001 From: Stephan Sokolow Date: Sun, 7 Sep 2025 07:57:23 -0400 Subject: [PATCH 11/12] Add control test for quicktile.sh reporting failures --- tests/test_functional.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/test_functional.py b/tests/test_functional.py index 5087e46..fca7a96 100755 --- a/tests/test_functional.py +++ b/tests/test_functional.py @@ -134,4 +134,8 @@ def test_functional(openbox_session, command): command], env=openbox_session) # nosec +def test_quicktile_sh_reports_failure(openbox_session): + """Verify that quicktile.sh passes the return code through""" + assert subprocess.call(['./quicktile.sh', '--i-am-an-invalid-option']) == 2 + # vim: set sw=4 sts=4 expandtab : From 87d72006ad51b632e3801a37a4002e79ea0f675f Mon Sep 17 00:00:00 2001 From: Stephan Sokolow Date: Sun, 7 Sep 2025 08:02:42 -0400 Subject: [PATCH 12/12] dev_requirements.txt: Add pytest-xvfb --- dev_requirements.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/dev_requirements.txt b/dev_requirements.txt index 596987e..aa55393 100644 --- a/dev_requirements.txt +++ b/dev_requirements.txt @@ -1,6 +1,7 @@ mypy pytest pytest-cov +pytest-xvfb ruff sphinx sphinx-autodoc-typehints[type_comment]