From abab635a01e5886086a7fcda7fd9e8a27aa151ad Mon Sep 17 00:00:00 2001 From: Jens L Date: Thu, 12 Oct 2023 13:57:29 +0200 Subject: [PATCH] tests: fix potential infinite wait in tests spinning up a container (#7153) Signed-off-by: Jens Langhammer --- authentik/root/test_runner.py | 8 +++---- tests/e2e/test_provider_oidc.py | 9 ++------ tests/e2e/test_provider_oidc_implicit.py | 9 ++------ tests/e2e/test_provider_saml.py | 9 ++------ tests/e2e/utils.py | 28 +++++++++++++++++------- tests/integration/test_outpost_docker.py | 13 ++++------- tests/integration/test_proxy_docker.py | 13 ++++------- 7 files changed, 37 insertions(+), 52 deletions(-) diff --git a/authentik/root/test_runner.py b/authentik/root/test_runner.py index 02425f0169..b2bf7a3d72 100644 --- a/authentik/root/test_runner.py +++ b/authentik/root/test_runner.py @@ -17,8 +17,8 @@ TestCase.maxDiff = None class PytestTestRunner(DiscoverRunner): # pragma: no cover """Runs pytest to discover and run tests.""" - def __init__(self, verbosity=1, failfast=False, keepdb=False, **kwargs): - super().__init__(verbosity, failfast, keepdb, **kwargs) + def __init__(self, **kwargs): + super().__init__(**kwargs) self.args = [] if self.failfast: @@ -47,6 +47,7 @@ class PytestTestRunner(DiscoverRunner): # pragma: no cover @classmethod def add_arguments(cls, parser: ArgumentParser): """Add more pytest-specific arguments""" + DiscoverRunner.add_arguments(parser) parser.add_argument( "--randomly-seed", type=int, @@ -55,9 +56,6 @@ class PytestTestRunner(DiscoverRunner): # pragma: no cover "Default behaviour: use random.Random().getrandbits(32), so the seed is" "different on each run.", ) - parser.add_argument( - "--keepdb", action="store_true", help="Preserves the test DB between runs." - ) def run_tests(self, test_labels, extra_tests=None, **kwargs): """Run pytest and return the exitcode. diff --git a/tests/e2e/test_provider_oidc.py b/tests/e2e/test_provider_oidc.py index e568178369..23cbd14124 100644 --- a/tests/e2e/test_provider_oidc.py +++ b/tests/e2e/test_provider_oidc.py @@ -49,13 +49,8 @@ class TestProviderOAuth2OIDC(SeleniumTestCase): "OIDC_PROVIDER": f"{self.live_server_url}/application/o/{self.application_slug}/", }, ) - while True: - container.reload() - status = container.attrs.get("State", {}).get("Health", {}).get("Status") - if status == "healthy": - return container - self.logger.info("Container failed healthcheck") - sleep(1) + self.wait_for_container(container) + return container @retry() @apply_blueprint( diff --git a/tests/e2e/test_provider_oidc_implicit.py b/tests/e2e/test_provider_oidc_implicit.py index 71e1c15f49..e952d4e185 100644 --- a/tests/e2e/test_provider_oidc_implicit.py +++ b/tests/e2e/test_provider_oidc_implicit.py @@ -49,13 +49,8 @@ class TestProviderOAuth2OIDCImplicit(SeleniumTestCase): "OIDC_PROVIDER": f"{self.live_server_url}/application/o/{self.application_slug}/", }, ) - while True: - container.reload() - status = container.attrs.get("State", {}).get("Health", {}).get("Status") - if status == "healthy": - return container - self.logger.info("Container failed healthcheck") - sleep(1) + self.wait_for_container(container) + return container @retry() @apply_blueprint( diff --git a/tests/e2e/test_provider_saml.py b/tests/e2e/test_provider_saml.py index ef7baf8cc3..9252ab0c0f 100644 --- a/tests/e2e/test_provider_saml.py +++ b/tests/e2e/test_provider_saml.py @@ -48,13 +48,8 @@ class TestProviderSAML(SeleniumTestCase): "SP_METADATA_URL": metadata_url, }, ) - while True: - container.reload() - status = container.attrs.get("State", {}).get("Health", {}).get("Status") - if status == "healthy": - return container - self.logger.info("Container failed healthcheck") - sleep(1) + self.wait_for_container(container) + return container @retry() @apply_blueprint( diff --git a/tests/e2e/utils.py b/tests/e2e/utils.py index 90314465fd..7092ac4e91 100644 --- a/tests/e2e/utils.py +++ b/tests/e2e/utils.py @@ -43,7 +43,24 @@ def get_docker_tag() -> str: return f"gh-{branch_name}" -class SeleniumTestCase(StaticLiveServerTestCase): +class DockerTestCase: + """Mixin for dealing with containers""" + + def wait_for_container(self, container: Container): + """Check that container is health""" + attempt = 0 + while True: + container.reload() + status = container.attrs.get("State", {}).get("Health", {}).get("Status") + if status == "healthy": + return container + sleep(1) + attempt += 1 + if attempt >= 30: + self.failureException("Container failed to start") + + +class SeleniumTestCase(DockerTestCase, StaticLiveServerTestCase): """StaticLiveServerTestCase which automatically creates a Webdriver instance""" container: Optional[Container] = None @@ -82,13 +99,8 @@ class SeleniumTestCase(StaticLiveServerTestCase): state = container.attrs.get("State", {}) if "Health" not in state: return container - while True: - container.reload() - status = container.attrs.get("State", {}).get("Health", {}).get("Status") - if status == "healthy": - return container - self.logger.info("Container failed healthcheck") - sleep(1) + self.wait_for_container(container) + return container def output_container_logs(self, container: Optional[Container] = None): """Output the container logs to our STDOUT""" diff --git a/tests/integration/test_outpost_docker.py b/tests/integration/test_outpost_docker.py index 98e4ec48ac..4e12cbbca8 100644 --- a/tests/integration/test_outpost_docker.py +++ b/tests/integration/test_outpost_docker.py @@ -1,7 +1,6 @@ """outpost tests""" from shutil import rmtree from tempfile import mkdtemp -from time import sleep import yaml from channels.testing import ChannelsLiveServerTestCase @@ -20,10 +19,10 @@ from authentik.outposts.models import ( ) from authentik.outposts.tasks import outpost_connection_discovery from authentik.providers.proxy.models import ProxyProvider -from tests.e2e.utils import get_docker_tag +from tests.e2e.utils import DockerTestCase, get_docker_tag -class OutpostDockerTests(ChannelsLiveServerTestCase): +class OutpostDockerTests(DockerTestCase, ChannelsLiveServerTestCase): """Test Docker Controllers""" def _start_container(self, ssl_folder: str) -> Container: @@ -45,12 +44,8 @@ class OutpostDockerTests(ChannelsLiveServerTestCase): } }, ) - while True: - container.reload() - status = container.attrs.get("State", {}).get("Health", {}).get("Status") - if status == "healthy": - return container - sleep(1) + self.wait_for_container(container) + return container def setUp(self): super().setUp() diff --git a/tests/integration/test_proxy_docker.py b/tests/integration/test_proxy_docker.py index 147088ee6f..8cbeb0bddd 100644 --- a/tests/integration/test_proxy_docker.py +++ b/tests/integration/test_proxy_docker.py @@ -1,7 +1,6 @@ """outpost tests""" from shutil import rmtree from tempfile import mkdtemp -from time import sleep import yaml from channels.testing.live import ChannelsLiveServerTestCase @@ -20,10 +19,10 @@ from authentik.outposts.models import ( from authentik.outposts.tasks import outpost_connection_discovery from authentik.providers.proxy.controllers.docker import DockerController from authentik.providers.proxy.models import ProxyProvider -from tests.e2e.utils import get_docker_tag +from tests.e2e.utils import DockerTestCase, get_docker_tag -class TestProxyDocker(ChannelsLiveServerTestCase): +class TestProxyDocker(DockerTestCase, ChannelsLiveServerTestCase): """Test Docker Controllers""" def _start_container(self, ssl_folder: str) -> Container: @@ -45,12 +44,8 @@ class TestProxyDocker(ChannelsLiveServerTestCase): } }, ) - while True: - container.reload() - status = container.attrs.get("State", {}).get("Health", {}).get("Status") - if status == "healthy": - return container - sleep(1) + self.wait_for_container(container) + return container def setUp(self): super().setUp()