Merge pull request #129 from yuvipanda/better-integration

Make it easier to run multiple independent integration tests

.circleci/config.yml

@@ -65,41 +65,14 @@ jobs:
             - run:
                 name: build systemd image
                 command: |
-                    python3 .circleci/integration-test.py build-image
+                    .circleci/integration-test.py build-image
 
             - run:
-                name: start systemd image
+                name: Run basic tests tests
                 command: |
-                    python3 .circleci/integration-test.py start-container
+                    .circleci/integration-test.py run-test basic-tests test_hub.py test_install.py test_extensions.py
-
-            - run:
-                name: run tljh installer
-                command: |
-                    python3 .circleci/integration-test.py copy . /srv/src
-                    python3 .circleci/integration-test.py run 'python3 /srv/src/bootstrap/bootstrap.py'
 
 
-            - run:
-                name: switch to dummyauthenticator
-                command: |
-                    python3 .circleci/integration-test.py run '/opt/tljh/hub/bin/tljh-config set auth.type dummyauthenticator.DummyAuthenticator'
-                    python3 .circleci/integration-test.py run '/opt/tljh/hub/bin/tljh-config reload'
-
-            - run:
-                name: print systemd status + logs
-                command: |
-                    python3 .circleci/integration-test.py run 'journalctl --no-pager'
-                    python3 .circleci/integration-test.py run 'systemctl --no-pager status jupyterhub configurable-http-proxy'
-
-            - run:
-                name: install integration test requirements
-                command: |
-                    python3 .circleci/integration-test.py run 'python3 -m pip install -r /srv/src/integration-tests/requirements.txt'
-
-            - run:
-                name: run integration tests
-                command: |
-                    python3 .circleci/integration-test.py run 'python3 -m pytest -v /srv/src/integration-tests'
 
     documentation:
         docker:

.circleci/integration-test.py

@@ -1,3 +1,4 @@
+#!/usr/bin/env python3
 import argparse
 import subprocess
 import os
@@ -32,14 +33,14 @@ def run_systemd_image(image_name, container_name):
     ])
 
 
-def remove_systemd_container(container_name):
+def stop_container(container_name):
     """
     Stop & remove docker container if it exists.
     """
     try:
         subprocess.check_output([
             'docker', 'inspect', container_name
-        ])
+        ], stderr=subprocess.STDOUT)
     except subprocess.CalledProcessError:
         # No such container exists, nothing to do
         return
@@ -52,12 +53,17 @@ def run_container_command(container_name, cmd):
     """
     Run cmd in a running container with a bash shell
     """
-    subprocess.check_call([
+    proc = subprocess.run([
         'docker', 'exec',
         '-it', container_name,
         '/bin/bash', '-c', cmd
     ])
 
+    if proc.returncode != 0:
+        # Don't throw if command fails. This lets us continue next parts
+        # of tests. Not entirely sure this is the right thing to do though!
+        print(f'command {cmd} exited with return code {proc.returncode}')
+
 
 def copy_to_container(container_name, src_path, dest_path):
     """
@@ -69,13 +75,55 @@ def copy_to_container(container_name, src_path, dest_path):
     ])
 
 
+def run_test(image_name, test_name, test_files, installer_args):
+    """
+    Wrapper that sets up tljh with installer_args & runs test_name
+    """
+    stop_container(test_name)
+    run_systemd_image(image_name, test_name)
+
+    source_path = os.path.abspath(
+        os.path.join(os.path.dirname(__file__), os.pardir)
+    )
+
+    copy_to_container(test_name, source_path, '/srv/src')
+    run_container_command(
+        test_name,
+        f'python3 /srv/src/bootstrap/bootstrap.py {installer_args}'
+    )
+    run_container_command(
+        test_name,
+        'python3 -m pip install -r /srv/src/integration-tests/requirements.txt'
+    )
+    run_container_command(
+        test_name,
+        'python3 -m pytest -v {}'.format(
+            ' '.join([os.path.join('/srv/src/integration-tests/', f) for f in test_files])
+        )
+    )
+
+
+def show_logs(container_name):
+    """
+    Print logs from inside container to stdout
+    """
+    run_container_command(
+        container_name,
+        'journalctl --no-pager'
+    )
+    run_container_command(
+        container_name,
+        'systemctl --no-pager status jupyterhub configurable-http-proxy'
+    )
+
 def main():
     argparser = argparse.ArgumentParser()
     subparsers = argparser.add_subparsers(dest='action')
 
     subparsers.add_parser('build-image')
-    subparsers.add_parser('start-container')
+    subparsers.add_parser('stop-container').add_argument(
-    subparsers.add_parser('stop-container')
+        'container_name'
+    )
     subparsers.add_parser('run').add_argument(
         'command',
     )
@@ -83,24 +131,30 @@ def main():
     copy_parser.add_argument('src')
     copy_parser.add_argument('dest')
 
+    run_test_parser = subparsers.add_parser('run-test')
+    run_test_parser.add_argument('--installer-args', default='')
+    run_test_parser.add_argument('test_name')
+    run_test_parser.add_argument('test_files', nargs='+')
+
+    show_logs_parser = subparsers.add_parser('show-logs')
+    show_logs_parser.add_argument('container_name')
+
     args = argparser.parse_args()
 
     image_name = 'tljh-systemd'
-    container_name = 'tljh-ci-run'
-    source_path = os.path.abspath(
-        os.path.join(os.path.dirname(__file__), os.pardir, 'integration-tests')
-    )
 
-    if args.action == 'build-image':
+    if args.action == 'run-test':
-        build_systemd_image(image_name, source_path)
+        run_test(image_name, args.test_name, args.test_files, args.installer_args)
-    elif args.action == 'start-container':
+    elif args.action == 'show-logs':
-        run_systemd_image(image_name, container_name)
+        show_logs(args.container_name)
-    elif args.action == 'stop-container':
-        remove_systemd_container(container_name)
     elif args.action == 'run':
-        run_container_command(container_name, args.command)
+        run_container_command(args.container_name, args.command)
     elif args.action == 'copy':
-        copy_to_container(container_name, args.src, args.dest)
+        copy_to_container(args.container_name, args.src, args.dest)
+    elif args.action == 'stop-container':
+        stop_container(args.container_name)
+    elif args.action == 'build-image':
+        build_systemd_image(image_name, 'integration-tests')
 
 
 if __name__ == '__main__':

docs/contributing/code-review.rst

@@ -46,4 +46,6 @@ feels exhaustively unit-testable, write unit tests too. When in doubt,
 add more tests.
 
 If you are unsure what kind of tests to add for your pull request, other
-contributors to the repo will be happy to help guide you!
+contributors to the repo will be happy to help guide you!
+
+See :ref:`contributing/tests` for guidelines on writing tests.

docs/contributing/tests.rst

@@ -0,0 +1,59 @@
+.. _contributing/tests:
+
+============
+Testing TLJH
+============
+
+Unit and integration tests are a core part of TLJH, as important as
+the code & documentation. They help validate that the code works as
+we think it does, and continues to do so when changes occur. They
+also help communicate in precise terms what we expect our code
+to do. 
+
+Integration tests
+=================
+
+TLJH is a *distribution* where the primary value is the many 
+opinionated choices we have made on components to use and how
+they fit together. Integration tests are perfect for testing
+that the various components fit together and work as they should.
+So we write a lot of integration tests, and put in more effort
+towards them than unit tests. 
+
+All integration tests are run on `CircleCI <https://circleci.com>`_
+for each PR and merge, making sure we don't have broken tests
+for too long.
+
+The integration tests are in the ``integration-tests`` directory
+in the git repository. ``py.test`` is used to write the integration
+tests. Each file should contain tests that can be run in any order
+against the same installation of TLJH.
+
+Running integration tests locally
+---------------------------------
+
+You need ``docker`` installed and callable by the user running
+the integration tests without needing sudo. 
+
+You can then run the tests with:
+
+.. code-block:: bash
+
+   .circleci/integration-test run-test <name-of-run> <test-file-names>
+
+
+``<name-of-run>`` is an identifier for the tests - you can choose
+anything you want. ``<test-file-names>>`` is list of test files
+(under ``integration-tests``) that should be run in one go.
+
+For example, to run all the basic tests, you would write:
+
+.. code-block:: bash
+
+   .circleci/integration-test.py run-test basic-tests \
+                                 test_hub.py \
+                                 test_install.py \
+                                 test_extensions.py
+
+This will run the tests in the three files against the same installation
+of TLJH and report errors.

docs/index.rst

@@ -126,3 +126,4 @@ to people contributing in various ways.
    contributing/docs
    contributing/code-review
    contributing/dev-setup
+   contributing/tests

integration-tests/test_hub.py

@@ -29,6 +29,13 @@ async def test_user_code_execute():
     hub_url = 'http://localhost'
     username = secrets.token_hex(8)
 
+    assert 0 == await (await asyncio.create_subprocess_exec(*TLJH_CONFIG_PATH, 'set', 'auth.type', 'dummyauthenticator.DummyAuthenticator')).wait()
+    assert 0 == await (await asyncio.create_subprocess_exec(*TLJH_CONFIG_PATH, 'reload')).wait()
+
+    # FIXME: wait for reload to finish & hub to come up
+    # Should be part of tljh-config reload
+    await asyncio.sleep(1)
+
     async with User(username, hub_url, partial(login_dummy, password='')) as u:
             await u.login()
             await u.ensure_server()
@@ -49,7 +56,7 @@ async def test_user_admin_add():
     hub_url = 'http://localhost'
     username = secrets.token_hex(8)
 
-
+    assert 0 == await (await asyncio.create_subprocess_exec(*TLJH_CONFIG_PATH, 'set', 'auth.type', 'dummyauthenticator.DummyAuthenticator')).wait()
     assert 0 == await (await asyncio.create_subprocess_exec(*TLJH_CONFIG_PATH, 'add-item', 'users.admin', username)).wait()
     assert 0 == await (await asyncio.create_subprocess_exec(*TLJH_CONFIG_PATH, 'reload')).wait()
 
@@ -79,6 +86,7 @@ async def test_user_admin_remove():
     hub_url = 'http://localhost'
     username = secrets.token_hex(8)
 
+    assert 0 == await (await asyncio.create_subprocess_exec(*TLJH_CONFIG_PATH, 'set', 'auth.type', 'dummyauthenticator.DummyAuthenticator')).wait()
     assert 0 == await (await asyncio.create_subprocess_exec(*TLJH_CONFIG_PATH, 'add-item', 'users.admin', username)).wait()
     assert 0 == await (await asyncio.create_subprocess_exec(*TLJH_CONFIG_PATH, 'reload')).wait()