Minimize assumptions about test environment

.github/workflows/test.yaml

@@ -68,10 +68,11 @@ jobs:
           pip freeze
 
       - name: Run tests
-        # Running tests requires sudo permissions, but we also need to preserve
-        # the PATH to have pytest available etc.
+        # Tests needs to be run as root and we have to specify a non-root
+        # non-nobody system user to test with. We also need to preserve the PATH
+        # when running as root.
         run: |
-          sudo -E "PATH=$PATH" bash -c "pytest --cov=systemdspawner"
+          sudo -E "PATH=$PATH" bash -c "pytest --cov=systemdspawner --system-test-user=$(whoami)"
 
       # GitHub action reference: https://github.com/codecov/codecov-action
       - uses: codecov/codecov-action@v3

tests/conftest.py

@@ -1,5 +1,3 @@
-import os
-
 import pytest
 from traitlets.config import Config
 
@@ -15,14 +13,31 @@
 ]
 
 
-def pytest_configure(config):
+def pytest_addoption(parser, pluginmanager):
     """
-    A pytest recognized function to adjust configuration before running tests.
+    A pytest hook to register argparse-style options and ini-style config
+    values.
+
+    We use it to declare command-line arguments.
+
+    ref: https://docs.pytest.org/en/stable/reference/reference.html#pytest.hookspec.pytest_addoption
+    ref: https://docs.pytest.org/en/stable/reference/reference.html#pytest.Parser.addoption
     """
-    config.addinivalue_line(
-        "markers", "github_actions: only to be run in a github actions ci environment"
+    parser.addoption(
+        "--system-test-user",
+        help="Test server spawning for this existing system user",
     )
 
+
+def pytest_configure(config):
+    """
+    A pytest hook to adjust configuration before running tests.
+
+    We use it to declare pytest marks.
+
+    ref: https://docs.pytest.org/en/stable/reference/reference.html#pytest.hookspec.pytest_configure
+    ref: https://docs.pytest.org/en/stable/reference/reference.html#pytest.Config
+    """
     # These markers are registered to avoid warnings triggered by importing from
     # jupyterhub.tests.test_api in test_systemspawner.py.
     config.addinivalue_line("markers", "role: dummy")
@@ -32,26 +47,15 @@ def pytest_configure(config):
     config.addinivalue_line("markers", "services: dummy")
 
 
-def pytest_runtest_setup(item):
-    """
-    Several of these tests work against the host system directly, so to protect
-    users from issues we make these not run.
-    """
-    if not os.environ.get("GITHUB_ACTIONS"):
-        has_github_actions_mark = any(
-            mark for mark in item.iter_markers(name="github_actions")
-        )
-        if has_github_actions_mark:
-            pytest.skip("Skipping test marked safe only for GitHub's CI environment.")
-
-
 @pytest.fixture
 async def systemdspawner_config():
     """
     Represents the base configuration of relevance to test SystemdSpawner.
     """
     config = Config()
     config.JupyterHub.spawner_class = "systemd"
+
+    # set cookie_secret to avoid having jupyterhub create a file
     config.JupyterHub.cookie_secret = "abc123"
 
     return config

tests/test_systemd.py

@@ -8,8 +8,6 @@
 import tempfile
 import time
 
-import pytest
-
 from systemdspawner import systemd
 
 
@@ -24,7 +22,6 @@ def test_get_systemd_version():
     ), "Either systemd wasn't running, or we failed to parse the version into an integer!"
 
 
-@pytest.mark.github_actions
 async def test_simple_start():
     unit_name = "systemdspawner-unittest-" + str(time.time())
     await systemd.start_transient_service(
@@ -38,7 +35,6 @@ async def test_simple_start():
     assert not await systemd.service_running(unit_name)
 
 
-@pytest.mark.github_actions
 async def test_service_failed_reset():
     """
     Test service_failed and reset_service
@@ -61,7 +57,6 @@ async def test_service_failed_reset():
     assert not await systemd.service_failed(unit_name)
 
 
-@pytest.mark.github_actions
 async def test_service_running_fail():
     """
     Test service_running failing when there's no service.
@@ -71,7 +66,6 @@ async def test_service_running_fail():
     assert not await systemd.service_running(unit_name)
 
 
-@pytest.mark.github_actions
 async def test_env_setting():
     unit_name = "systemdspawner-unittest-" + str(time.time())
     with tempfile.TemporaryDirectory() as d:
@@ -113,7 +107,6 @@ async def test_env_setting():
         assert not os.path.exists(env_file)
 
 
-@pytest.mark.github_actions
 async def test_workdir():
     unit_name = "systemdspawner-unittest-" + str(time.time())
     _, env_filename = tempfile.mkstemp()
@@ -133,7 +126,6 @@ async def test_workdir():
             assert text == d
 
 
-@pytest.mark.github_actions
 async def test_slice():
     unit_name = "systemdspawner-unittest-" + str(time.time())
     _, env_filename = tempfile.mkstemp()
@@ -159,7 +151,6 @@ async def test_slice():
         assert b"user.slice" in stdout
 
 
-@pytest.mark.github_actions
 async def test_properties_string():
     """
     Test that setting string properties works
@@ -189,7 +180,6 @@ async def test_properties_string():
             assert text == "/bind-test"
 
 
-@pytest.mark.github_actions
 async def test_properties_list():
     """
     Test setting multiple values for a property
@@ -226,7 +216,6 @@ async def test_properties_list():
             assert text == d
 
 
-@pytest.mark.github_actions
 async def test_uid_gid():
     """
     Test setting uid and gid

tests/test_systemdspawner.py

@@ -1,73 +1,33 @@
-"""
-These tests are running JupyterHub configured with a SystemdSpawner and starting
-a server for the specific user named "runner". It is not meant to be run outside
-a CI system.
-"""
-
-import subprocess
-
-import pytest
 from jupyterhub.tests.mocking import public_url
 from jupyterhub.tests.test_api import add_user, api_request
 from jupyterhub.utils import url_path_join
 from tornado.httpclient import AsyncHTTPClient
 
+from systemdspawner import systemd
 
-def _get_systemdspawner_user_unit(username):
-    """
-    Returns an individual SystemdSpawner's created systemd units representing a
-    specific user server.
 
-    Note that --output=json is only usable in systemd 246+, so we have to rely
-    on this manual parsing.
+async def test_start_stop(hub_app, systemdspawner_config, pytestconfig):
     """
-    unit_name = f"jupyter-{username}-singleuser.service"
-    output = subprocess.check_output(
-        ["systemctl", "list-units", "--no-pager", "--all", "--plain", unit_name],
-        text=True,
-    )
+    Starts a user server, verifies access to its /api/status endpoint, and stops
+    the server.
 
-    user_unit = output.split("\n")[1].split(maxsplit=4)
-    if user_unit[0] != unit_name:
-        return None
+    About using the root and nobody user:
 
-    # Mimics the output we could get from using --output=json in the future when
-    # we can test only against systemd 246+.
-    #
-    # [
-    #     {
-    #         "unit": "jupyter-runner-singleuser.service",
-    #         "load": "loaded",
-    #         "active": "active",
-    #         "sub": "running",
-    #         "description": "/bin/bash -c cd /home/runner && exec jupyterhub-singleuser "
-    #     }
-    # ]
-    #
-    # load   = Reflects whether the unit definition was properly loaded.
-    # active = The high-level unit activation state, i.e. generalization of SUB.
-    # sub    = The low-level unit activation state, values depend on unit type.
-    #
-    return {
-        "unit": user_unit[0],
-        "load": user_unit[1],
-        "active": user_unit[2],
-        "sub": user_unit[3],
-        "description": user_unit[4],
-    }
+      - A jupyter server started as root will error without the user server being
+        passed the --allow-root flag.
 
-
-@pytest.mark.github_actions
-async def test_start_stop(hub_app, systemdspawner_config):
-    """
-    This tests starts the default user server, access its /api/status endpoint,
-    and stops the server.
+      - SystemdSpawner runs the user server with a working directory set to the
+        user's home home directory, which for the nobody user is /nonexistent on
+        ubunutu.
     """
+    # test is skipped unless pytest is run with --system-test-user=USERNAME
+    username = pytestconfig.getoption("--system-test-user", skip=True)
+    unit_name = f"jupyter-{username}-singleuser.service"
+
     test_config = {}
     systemdspawner_config.merge(test_config)
     app = await hub_app(systemdspawner_config)
 
-    username = "runner"
     add_user(app.db, app, name=username)
     user = app.users[username]
 
@@ -82,13 +42,9 @@ async def test_start_stop(hub_app, systemdspawner_config):
     assert r.status_code in {201, 200}, r.text
 
     # verify the server is started via systemctl
-    user_unit = _get_systemdspawner_user_unit(username)
-    assert user_unit
-    assert user_unit["load"] == "loaded"
-    assert user_unit["active"] == "active"
-    assert user_unit["sub"] == "running"
+    assert await systemd.service_running(unit_name)
 
-    # very the server is started by accessing the server's api/status
+    # verify the server is started by accessing the server's api/status
     token = user.new_api_token()
     url = url_path_join(public_url(app, user), "api/status")
     headers = {"Authorization": f"token {token}"}
@@ -108,6 +64,4 @@ async def test_start_stop(hub_app, systemdspawner_config):
     assert r.status_code in {204, 200}, r.text
 
     # verify the server is stopped via systemctl
-    user_unit = _get_systemdspawner_user_unit(username)
-    print(user_unit)
-    assert not user_unit
+    assert not await systemd.service_running(unit_name)