Merge pull request #916 from consideRatio/pr/refactor-tests

maint: refactor tests, fix upgrade tests (now correctly failing)

Author: minrk

.github/integration-test.py

@@ -59,82 +59,66 @@ jobs:
         with:
           python-version: "3.10"
 
-      - name: Install pytest
-        run: python3 -m pip install pytest
+      # FIXME: The test_bootstrap.py script has duplicated logic to run build
+      #        and start images and run things in them. This makes tests slower,
+      #        and adds code to maintain. Let's try to remove it.
+      #
+      #        - bootstrap.py's failure detections, put in unit tests?
+      #        - bootstrap.py's --show-progress-page test, include as integration test?
+      #
+      - name: Install integration-tests/requirements.txt for test_bootstrap.py
+        run: pip install -r integration-tests/requirements.txt
 
-      # We abort pytest after two failures as a compromise between wanting to
-      # avoid a flood of logs while still understanding if multiple tests would
-      # fail.
       - name: Run bootstrap tests (Runs in/Builds ${{ matrix.distro_image }} derived image)
         run: |
-          pytest --verbose --maxfail=2 --color=yes --durations=10 --capture=no \
-              integration-tests/test_bootstrap.py
-        timeout-minutes: 20
+          pytest integration-tests/test_bootstrap.py
+        timeout-minutes: 10
         env:
           # integration-tests/test_bootstrap.py will build and start containers
           # based on this environment variable. This is similar to how
           # .github/integration-test.py build-image can take a --build-arg
           # setting the base image via a Dockerfile ARG.
           BASE_IMAGE: ${{ matrix.distro_image }}
 
-      # We build a docker image from wherein we will work
-      - name: Build systemd image (Builds ${{ matrix.distro_image }} derived image)
+      - name: Build systemd image, derived from ${{ matrix.distro_image }}
         run: |
           .github/integration-test.py build-image \
               --build-arg "BASE_IMAGE=${{ matrix.distro_image }}"
 
-      # FIXME: Make the logic below easier to follow.
-      # - In short, setting BOOTSTRAP_PIP_SPEC here, specifies from what
-      #   location the tljh python package should be installed from. In this
-      #   GitHub Workflow's test job, we provide a remote reference to itself as
-      #   found on GitHub - this could be the HEAD of a PR branch or the default
-      #   branch on merge.
-      #
       # Overview of how this logic influences the end result.
       # - integration-test.yaml:
-      #   Runs integration-test.py by passing --bootstrap-pip-spec flag with a
-      #   reference to the pull request on GitHub.
-      # - integration-test.py:
-      #   Starts a pre-build systemd container, setting the
-      #   TLJH_BOOTSTRAP_PIP_SPEC based on its passed --bootstrap-pip-spec value.
-      # - systemd container:
-      #   Runs bootstrap.py
-      # - bootstrap.py
-      #   Makes use of TLJH_BOOTSTRAP_PIP_SPEC environment variable to install
-      #   the tljh package from a given location, which could be a local git
-      #   clone of this repo where setup.py resides, or a reference to some
-      #   GitHub branch for example.
-      - name: Set BOOTSTRAP_PIP_SPEC value
-        run: |
-          BOOTSTRAP_PIP_SPEC="git+https://github.com/$GITHUB_REPOSITORY.git@$GITHUB_REF"
-          echo "BOOTSTRAP_PIP_SPEC=$BOOTSTRAP_PIP_SPEC" >> $GITHUB_ENV
-          echo $BOOTSTRAP_PIP_SPEC
-
-      - name: Run basic tests (Runs in ${{ matrix.distro_image }} derived image)
+      #
+      #   - Runs integration-test.py build-image, to build a systemd based image
+      #     to use later.
+      #
+      #   - Runs integration-test.py run-tests, to start a systemd based
+      #     container, run the bootstrap.py script inside it, and then run
+      #     pytest from the hub python environment setup by the bootstrap
+      #     script.
+      #
+      # About passed --installer-args:
+      #
+      #   - --admin admin:admin
+      #     Required for test_admin_installer.py
+      #
+      #   - --plugin /srv/src/integration-tests/plugins/simplest
+      #     Required for test_simplest_plugin.py
+      #
+      - name: pytest integration-tests/
+        id: integration-tests
         run: |
-          .github/integration-test.py run-test basic-tests \
-              --bootstrap-pip-spec "$BOOTSTRAP_PIP_SPEC" \
+          .github/integration-test.py run-test integration-tests \
+              --installer-args "--admin test-admin-username:test-admin-password" \
+              --installer-args "--plugin /srv/src/integration-tests/plugins/simplest" \
               ${{ matrix.extra_flags }} \
               test_hub.py \
               test_proxy.py \
               test_install.py \
-              test_extensions.py
-        timeout-minutes: 15
-
-      - name: Run admin tests (Runs in ${{ matrix.distro_image }} derived image)
-        run: |
-          .github/integration-test.py run-test admin-tests \
-              --installer-args "--admin admin:admin" \
-              --bootstrap-pip-spec "$BOOTSTRAP_PIP_SPEC" \
-              ${{ matrix.extra_flags }} \
-              test_admin_installer.py
-        timeout-minutes: 15
-
-      - name: Run plugin tests (Runs in ${{ matrix.distro_image }} derived image)
-        run: |
-          .github/integration-test.py run-test plugin-tests \
-              --bootstrap-pip-spec "$BOOTSTRAP_PIP_SPEC" \
-              --installer-args "--plugin /srv/src/integration-tests/plugins/simplest" \
-              ${{ matrix.extra_flags }} \
+              test_extensions.py \
+              test_admin_installer.py \
               test_simplest_plugin.py
         timeout-minutes: 15
+      - name: show logs
+        if: always() && steps.integration-tests.outcome != 'skipped'
+        run: |
+          .github/integration-test.py show-logs integration-tests

.github/workflows/integration-test.yaml

@@ -82,15 +82,15 @@ jobs:
 
       - name: Install Python dependencies
         run: |
-          python3 -m pip install -r dev-requirements.txt
-          python3 -m pip install -e .
+          pip install -r dev-requirements.txt
+          pip install -e .
+
+      - name: List Python dependencies
+        run: |
           pip freeze
 
-      # We abort pytest after two failures as a compromise between wanting to
-      # avoid a flood of logs while still understanding if multiple tests would
-      # fail.
       - name: Run unit tests
-        run: pytest --verbose --maxfail=2 --color=yes --durations=10 --cov=tljh tests/
+        run: pytest tests
         timeout-minutes: 15
 
       - uses: codecov/codecov-action@v3

.github/workflows/unit-test.yaml

@@ -36,7 +36,7 @@
                             logs can be accessed during installation. If this is
                             passed, it will pass --progress-page-server-pid=<pid>
                             to the tljh installer for later termination.
-    --version               TLJH version or Git reference. Default 'latest' is
+    --version VERSION       TLJH version or Git reference. Default 'latest' is
                             the most recent release. Partial versions can be
                             specified, for example '1', '1.0' or '1.0.0'. You
                             can also pass a branch name such as 'main' or a
@@ -183,32 +183,32 @@ def run_subprocess(cmd, *args, **kwargs):
         return output
 
 
+def get_os_release_variable(key):
+    """
+    Return value for key from /etc/os-release
+
+    /etc/os-release is a bash file, so should use bash to parse it.
+
+    Returns empty string if key is not found.
+    """
+    return (
+        subprocess.check_output(
+            [
+                "/bin/bash",
+                "-c",
+                "source /etc/os-release && echo ${{{key}}}".format(key=key),
+            ]
+        )
+        .decode()
+        .strip()
+    )
+
+
 def ensure_host_system_can_install_tljh():
     """
     Check if TLJH is installable in current host system and exit with a clear
     error message otherwise.
     """
-
-    def get_os_release_variable(key):
-        """
-        Return value for key from /etc/os-release
-
-        /etc/os-release is a bash file, so should use bash to parse it.
-
-        Returns empty string if key is not found.
-        """
-        return (
-            subprocess.check_output(
-                [
-                    "/bin/bash",
-                    "-c",
-                    "source /etc/os-release && echo ${{{key}}}".format(key=key),
-                ]
-            )
-            .decode()
-            .strip()
-        )
-
     # Require Ubuntu 20.04+ or Debian 11+
     distro = get_os_release_variable("ID")
     version = get_os_release_variable("VERSION_ID")
@@ -364,7 +364,7 @@ def main():
     )
     parser.add_argument(
         "--version",
-        default="latest",
+        default="",
         help=(
             "TLJH version or Git reference. "
             "Default 'latest' is the most recent release. "
@@ -478,21 +478,26 @@ def serve_forever(server):
     logger.info("Upgrading pip...")
     run_subprocess([hub_env_pip, "install", "--upgrade", "pip"])
 
-    # Install/upgrade TLJH installer
+    # pip install TLJH installer based on
+    #
+    #   1. --version, _resolve_git_version is used
+    #   2. TLJH_BOOTSTRAP_PIP_SPEC (then also respect TLJH_BOOTSTRAP_DEV)
+    #   3. latest, _resolve_git_version is used
+    #
     tljh_install_cmd = [hub_env_pip, "install", "--upgrade"]
-    if os.environ.get("TLJH_BOOTSTRAP_DEV", "no") == "yes":
-        logger.info("Selected TLJH_BOOTSTRAP_DEV=yes...")
-        tljh_install_cmd.append("--editable")
-
     bootstrap_pip_spec = os.environ.get("TLJH_BOOTSTRAP_PIP_SPEC")
-    if not bootstrap_pip_spec:
+    if args.version or not bootstrap_pip_spec:
+        version_to_resolve = args.version or "latest"
         bootstrap_pip_spec = (
             "git+https://github.com/jupyterhub/the-littlest-jupyterhub.git@{}".format(
-                _resolve_git_version(args.version)
+                _resolve_git_version(version_to_resolve)
             )
         )
-
+    elif os.environ.get("TLJH_BOOTSTRAP_DEV", "no") == "yes":
+        logger.info("Selected TLJH_BOOTSTRAP_DEV=yes...")
+        tljh_install_cmd.append("--editable")
     tljh_install_cmd.append(bootstrap_pip_spec)
+
     if initial_setup:
         logger.info("Installing TLJH installer...")
     else:

bootstrap/bootstrap.py

@@ -1,4 +1,5 @@
 packaging
 pytest
 pytest-cov
+pytest-asyncio
 pytest-mock

dev-requirements.txt

@@ -24,8 +24,6 @@ RUN find /etc/systemd/system \
     -not -name '*systemd-user-sessions*' \
     -exec rm \{} \;
 
-RUN mkdir -p /etc/sudoers.d
-
 RUN systemctl set-default multi-user.target
 
 STOPSIGNAL SIGRTMIN+3

integration-tests/Dockerfile

@@ -1,55 +1,47 @@
 """
-Simplest plugin that exercises all the hooks
+Simplest plugin that exercises all the hooks defined in tljh/hooks.py.
 """
 from tljh.hooks import hookimpl
 
 
 @hookimpl
 def tljh_extra_user_conda_packages():
-    return [
-        "hypothesis",
-    ]
+    return ["tqdm"]
 
 
 @hookimpl
 def tljh_extra_user_pip_packages():
-    return [
-        "django",
-    ]
+    return ["django"]
 
 
 @hookimpl
 def tljh_extra_hub_pip_packages():
-    return [
-        "there",
-    ]
+    return ["there"]
 
 
 @hookimpl
 def tljh_extra_apt_packages():
-    return [
-        "sl",
-    ]
+    return ["sl"]
 
 
 @hookimpl
-def tljh_config_post_install(config):
-    # Put an arbitrary marker we can test for
-    config["simplest_plugin"] = {"present": True}
+def tljh_custom_jupyterhub_config(c):
+    c.Test.jupyterhub_config_set_by_simplest_plugin = True
 
 
 @hookimpl
-def tljh_custom_jupyterhub_config(c):
-    c.JupyterHub.authenticator_class = "tmpauthenticator.TmpAuthenticator"
+def tljh_config_post_install(config):
+    config["Test"] = {"tljh_config_set_by_simplest_plugin": True}
 
 
 @hookimpl
 def tljh_post_install():
-    with open("test_post_install", "w") as f:
-        f.write("123456789")
+    with open("test_tljh_post_install", "w") as f:
+        f.write("file_written_by_simplest_plugin")
 
 
 @hookimpl
 def tljh_new_user_create(username):
     with open("test_new_user_create", "w") as f:
+        f.write("file_written_by_simplest_plugin")
         f.write(username)

integration-tests/plugins/simplest/tljh_simplest.py

@@ -1,3 +1,4 @@
 pytest
+pytest-cov
 pytest-asyncio
 git+https://github.com/yuvipanda/hubtraf.git