From 1ad07346f1b8b01870d9e695472deab1e7a1ea30 Mon Sep 17 00:00:00 2001 From: Federico Capoano Date: Sat, 4 Jul 2026 14:17:05 -0300 Subject: [PATCH 1/3] [tests] Separated base test classes (base vs selenium) SeleniumTestMixin brings a lot of logic which is not needed for the precondition checks. It recently introduced logic for dealing with flaky tests caused by sqlite, and depends on django SETTINGS being loaded, which is not the case for the precondition checks. --- tests/runtests.py | 11 +++++----- tests/utils.py | 54 +++++++++++++++++++++++------------------------ 2 files changed, 33 insertions(+), 32 deletions(-) diff --git a/tests/runtests.py b/tests/runtests.py index ffa47ede..d9d73cca 100644 --- a/tests/runtests.py +++ b/tests/runtests.py @@ -12,10 +12,11 @@ from selenium.webdriver.common.by import By from selenium.webdriver.support import expected_conditions as EC from selenium.webdriver.support.ui import WebDriverWait -from utils import TestUtilities +from utils import BaseTestUtils, SeleniumTestUtils -class Pretest(TestUtilities, unittest.TestCase): +# 0 in the name is on purpose for alphabetical discovery +class Test0Preconditions(BaseTestUtils, unittest.TestCase): """Checks to perform before tests""" def test_wait_for_services(self): @@ -60,7 +61,7 @@ def test_wait_for_services(self): self.fail(f"All celery workers are not online: {online_workers}") -class TestServices(TestUtilities, unittest.TestCase): +class TestServices(SeleniumTestUtils, unittest.TestCase): custom_static_token = None @property @@ -532,8 +533,8 @@ def test_containers_down(self): ) -class TestUtils(TestUtilities, unittest.TestCase): - """Other tests""" +class TestLocalUtils(BaseTestUtils, unittest.TestCase): + """Tests for local utilities""" def test_update_version_updates_only_version_file(self): repository_root = Path(__file__).resolve().parents[1] diff --git a/tests/utils.py b/tests/utils.py index c7def8c8..155d1982 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -11,12 +11,8 @@ from selenium.webdriver.common.by import By -class TestConfig: - """Configuration class for setting up test parameters and utilities.""" - - def shortDescription(self): - """Return a short description for the test.""" - return None +class BaseTestUtils: + """Base class for setting up test parameters and utilities.""" docker_client = docker.from_env() ctx = ssl.create_default_context() @@ -28,9 +24,32 @@ def shortDescription(self): with open(config_file) as json_file: config = json.load(json_file) + def shortDescription(self): + """Keep verbose unittest output focused on test names, not docstrings.""" + return None + + def docker_compose_get_container_id(self, container_name): + """Get the Docker container ID for a specific container. + + Parameters: + + - container_name (str): The name of the Docker container. + + Returns: + str: The ID of the Docker container. + """ + services_output = subprocess.Popen( + ["docker", "compose", "ps", "--quiet", container_name], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + cwd=self.root_location, + ) + output, _ = services_output.communicate() + return output.rstrip().decode("utf-8") + -class TestUtilities(SeleniumTestMixin, TestConfig): - """Utility functions for testing.""" +class SeleniumTestUtils(SeleniumTestMixin, BaseTestUtils): + """Utilities for functional testing.""" objects_to_delete = [] browser = "chrome" @@ -254,25 +273,6 @@ def add_mobile_location_point(self, location_name, driver=None): self._click_save_btn(driver) self.get_resource(location_name, "/admin/geo/location/", driver=driver) - def docker_compose_get_container_id(self, container_name): - """Get the Docker container ID for a specific container. - - Parameters: - - - container_name (str): The name of the Docker container. - - Returns: - str: The ID of the Docker container. - """ - services_output = subprocess.Popen( - ["docker", "compose", "ps", "--quiet", container_name], - stdout=subprocess.PIPE, - stderr=subprocess.PIPE, - cwd=self.root_location, - ) - output, _ = services_output.communicate() - return output.rstrip().decode("utf-8") - def create_network_topology( self, label="automated-selenium-test-01", From c91dda3eaeac7d5d9dd94d45113cacdd41a339a5 Mon Sep 17 00:00:00 2001 From: Federico Capoano Date: Sat, 4 Jul 2026 14:17:33 -0300 Subject: [PATCH 2/3] [tests] Updated custom theme logic: favicon is now an SVG --- tests/runtests.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/runtests.py b/tests/runtests.py index d9d73cca..359bb5ee 100644 --- a/tests/runtests.py +++ b/tests/runtests.py @@ -126,8 +126,8 @@ def _setup_admin_theme_links(cls): f"body{{--openwisp-test: {cls.custom_static_token};}}" ) script = rf""" - grep -q OPENWISP_ADMIN_THEME_LINKS /opt/openwisp/openwisp/settings.py || \ - printf "\nOPENWISP_ADMIN_THEME_LINKS=[{{\"type\":\"text/css\",\"href\":\"/static/admin/css/openwisp.css\",\"rel\":\"stylesheet\",\"media\":\"all\"}},{{\"type\":\"text/css\",\"href\":\"/static/{cls.config["custom_css_filename"]}\",\"rel\":\"stylesheet\",\"media\":\"all\"}},{{\"type\":\"image/x-icon\",\"href\":\"ui/openwisp/images/favicon.png\",\"rel\":\"icon\"}}]\n" >> /opt/openwisp/openwisp/settings.py && + sed -i '/^OPENWISP_ADMIN_THEME_LINKS[[:space:]]*=/d' /opt/openwisp/openwisp/settings.py && + printf "\nOPENWISP_ADMIN_THEME_LINKS=[{{\"type\":\"text/css\",\"href\":\"/static/admin/css/openwisp.css\",\"rel\":\"stylesheet\",\"media\":\"all\"}},{{\"type\":\"text/css\",\"href\":\"/static/{cls.config["custom_css_filename"]}\",\"rel\":\"stylesheet\",\"media\":\"all\"}},{{\"type\":\"image/svg+xml\",\"href\":\"ui/openwisp/images/favicon.svg\",\"rel\":\"icon\"}}]\n" >> /opt/openwisp/openwisp/settings.py && python collectstatic.py && uwsgi --reload uwsgi.pid """ # noqa: E501 From 3d6f69c58edeff3b21e82896e4409af08ac0da4c Mon Sep 17 00:00:00 2001 From: Federico Capoano Date: Sat, 4 Jul 2026 14:47:11 -0300 Subject: [PATCH 3/3] [tests] Fixed isolation bugs in functional tests --- tests/runtests.py | 110 ++++++++++++++-------------------------------- tests/utils.py | 87 ++++++++++++++++++++++++++++++++---- 2 files changed, 112 insertions(+), 85 deletions(-) diff --git a/tests/runtests.py b/tests/runtests.py index 359bb5ee..c78f9e92 100644 --- a/tests/runtests.py +++ b/tests/runtests.py @@ -12,7 +12,7 @@ from selenium.webdriver.common.by import By from selenium.webdriver.support import expected_conditions as EC from selenium.webdriver.support.ui import WebDriverWait -from utils import BaseTestUtils, SeleniumTestUtils +from utils import BaseTestUtils, FunctionalTestUtils # 0 in the name is on purpose for alphabetical discovery @@ -61,7 +61,7 @@ def test_wait_for_services(self): self.fail(f"All celery workers are not online: {online_workers}") -class TestServices(SeleniumTestUtils, unittest.TestCase): +class TestServices(FunctionalTestUtils, unittest.TestCase): custom_static_token = None @property @@ -69,41 +69,6 @@ def failureException(self): TestServices.failed_test = True return super().failureException - @classmethod - def _execute_docker_compose_command(cls, cmd_args, use_text_mode=False): - """Execute a docker compose command and log output. - - Args: - cmd_args: List of command arguments for subprocess.Popen - use_text_mode: If True, use text mode for subprocess output - - Returns: - Tuple of (output, error) from command execution - """ - kwargs = { - "stdout": subprocess.PIPE, - "stderr": subprocess.PIPE, - "cwd": cls.root_location, - } - if use_text_mode: - kwargs["text"] = True - cmd = subprocess.run(cmd_args, check=False, **kwargs) - if use_text_mode: - output, error = cmd.stdout, cmd.stderr - else: - output = cmd.stdout.decode("utf-8", errors="replace") if cmd.stdout else "" - error = cmd.stderr.decode("utf-8", errors="replace") if cmd.stderr else "" - output, error = map(str, (cmd.stdout, cmd.stderr)) - with open(cls.config["logs_file"], "a") as logs_file: - logs_file.write(output) - logs_file.write(error) - if cmd.returncode != 0: - raise RuntimeError( - f"docker compose command failed " - f"({cmd.returncode}): {' '.join(cmd_args)}" - ) - return output, error - @classmethod def _setup_admin_theme_links(cls): """Configure admin theme links during tests. @@ -127,7 +92,7 @@ def _setup_admin_theme_links(cls): ) script = rf""" sed -i '/^OPENWISP_ADMIN_THEME_LINKS[[:space:]]*=/d' /opt/openwisp/openwisp/settings.py && - printf "\nOPENWISP_ADMIN_THEME_LINKS=[{{\"type\":\"text/css\",\"href\":\"/static/admin/css/openwisp.css\",\"rel\":\"stylesheet\",\"media\":\"all\"}},{{\"type\":\"text/css\",\"href\":\"/static/{cls.config["custom_css_filename"]}\",\"rel\":\"stylesheet\",\"media\":\"all\"}},{{\"type\":\"image/svg+xml\",\"href\":\"ui/openwisp/images/favicon.svg\",\"rel\":\"icon\"}}]\n" >> /opt/openwisp/openwisp/settings.py && + printf "\nOPENWISP_ADMIN_THEME_LINKS=[{{\"type\":\"text/css\",\"href\":\"/static/admin/css/openwisp.css\",\"rel\":\"stylesheet\",\"media\":\"all\"}},{{\"type\":\"text/css\",\"href\":\"/static/{cls.config["custom_css_filename"]}\",\"rel\":\"stylesheet\",\"media\":\"all\"}},{{\"type\":\"image/svg+xml\",\"href\":\"/static/ui/openwisp/images/favicon.svg\",\"rel\":\"icon\"}}]\n" >> /opt/openwisp/openwisp/settings.py && python collectstatic.py && uwsgi --reload uwsgi.pid """ # noqa: E501 @@ -154,27 +119,7 @@ def _cleanup_stale_test_data(cls): avoid any Selenium dependency during setup. """ try: - cls._execute_docker_compose_command( - [ - "docker", - "compose", - "exec", - "-T", - "dashboard", - "python", - "manage.py", - "shell", - "-c", - ( - "from openwisp_users.models import User; " - "User.objects.filter(" - "username__in=['signup-user', 'test_superuser', " - "'test_superuser2']" - ").delete()" - ), - ], - use_text_mode=True, - ) + cls.delete_test_users("signup-user", "test_superuser", "test_superuser2") except Exception as e: exc_type = type(e).__name__ print( @@ -224,6 +169,12 @@ def setUpClass(cls): @classmethod def tearDownClass(cls): + try: + cls.delete_test_users(*cls.test_usernames_to_delete) + cls.test_usernames_to_delete.clear() + except Exception as e: + exc_type = type(e).__name__ + print(f"Unable to delete test users: {exc_type}: {e}") for resource_link in cls.objects_to_delete: try: cls._delete_object(resource_link) @@ -278,6 +229,13 @@ def test_admin_login(self): def test_custom_static_files_loaded(self): self.login() self.open("/admin/") + favicon_href = self.web_driver.find_element( + By.CSS_SELECTOR, 'link[rel="icon"]' + ).get_attribute("href") + self.assertRegex( + favicon_href, + r"/static/ui/openwisp/images/favicon(\.[0-9a-f]+)?\.svg$", + ) # Check if the custom CSS variable is applied value = self.web_driver.execute_script( "return getComputedStyle(document.body)" @@ -363,6 +321,7 @@ def test_add_superuser(self): def test_forgot_password(self): """Test forgot password to ensure that postfix is working properly.""" + self.login() self.logout() try: WebDriverWait(self.base_driver, 3).until( @@ -464,23 +423,22 @@ def _test_celery_task_registered(container_name): def test_radius_user_registration(self): """Ensure users can register using the RADIUS API.""" url = f"{self.config['api_url']}/api/v1/radius/organization/default/account/" - response = requests.post( - url, - json={ - "username": "signup-user", - "email": "user@signup.com", - "password1": "rLx6OH%[", - "password2": "rLx6OH%[", - }, - verify=False, - ) - self.assertEqual(response.status_code, 201) - # Delete the created user - self.login() - self.get_resource( - "signup-user", "/admin/openwisp_users/user/", "field-username" - ) - self.objects_to_delete.append(self.base_driver.current_url) + username = "signup-user" + try: + response = requests.post( + url, + json={ + "username": username, + "email": "user@signup.com", + "password1": "rLx6OH%[", + "password2": "rLx6OH%[", + }, + verify=False, + timeout=10, + ) + self.assertEqual(response.status_code, 201, response.text) + finally: + self.delete_test_users(username) def test_freeradius(self): """Ensure freeradius service is working correctly.""" diff --git a/tests/utils.py b/tests/utils.py index 155d1982..bd3f3ef5 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -14,6 +14,7 @@ class BaseTestUtils: """Base class for setting up test parameters and utilities.""" + docker_compose_timeout = 120 docker_client = docker.from_env() ctx = ssl.create_default_context() ctx.check_hostname = False @@ -28,6 +29,52 @@ def shortDescription(self): """Keep verbose unittest output focused on test names, not docstrings.""" return None + @classmethod + def _execute_docker_compose_command(cls, cmd_args, use_text_mode=False): + """Execute a docker compose command and log output.""" + kwargs = { + "stdout": subprocess.PIPE, + "stderr": subprocess.PIPE, + "cwd": cls.root_location, + } + if use_text_mode: + kwargs["text"] = True + try: + cmd = subprocess.run( + cmd_args, + check=False, + timeout=cls.docker_compose_timeout, + **kwargs, + ) + except subprocess.TimeoutExpired as e: + output = e.stdout or "" + error = e.stderr or "" + if isinstance(output, bytes): + output = output.decode("utf-8", errors="replace") if output else "" + if isinstance(error, bytes): + error = error.decode("utf-8", errors="replace") if error else "" + with open(cls.config["logs_file"], "a") as logs_file: + logs_file.write(output) + logs_file.write(error) + raise RuntimeError( + "docker compose command timed out " + f"after {cls.docker_compose_timeout}s: {' '.join(cmd_args)}" + ) + if use_text_mode: + output, error = cmd.stdout, cmd.stderr + else: + output = cmd.stdout.decode("utf-8", errors="replace") if cmd.stdout else "" + error = cmd.stderr.decode("utf-8", errors="replace") if cmd.stderr else "" + with open(cls.config["logs_file"], "a") as logs_file: + logs_file.write(output) + logs_file.write(error) + if cmd.returncode != 0: + raise RuntimeError( + f"docker compose command failed " + f"({cmd.returncode}): {' '.join(cmd_args)}" + ) + return output, error + def docker_compose_get_container_id(self, container_name): """Get the Docker container ID for a specific container. @@ -38,22 +85,44 @@ def docker_compose_get_container_id(self, container_name): Returns: str: The ID of the Docker container. """ - services_output = subprocess.Popen( - ["docker", "compose", "ps", "--quiet", container_name], - stdout=subprocess.PIPE, - stderr=subprocess.PIPE, - cwd=self.root_location, + output, _ = self._execute_docker_compose_command( + ["docker", "compose", "ps", "--quiet", container_name] ) - output, _ = services_output.communicate() - return output.rstrip().decode("utf-8") + return output.rstrip() -class SeleniumTestUtils(SeleniumTestMixin, BaseTestUtils): +class FunctionalTestUtils(SeleniumTestMixin, BaseTestUtils): """Utilities for functional testing.""" objects_to_delete = [] + test_usernames_to_delete = set() browser = "chrome" + @classmethod + def delete_test_users(cls, *usernames): + """Delete test-created users without relying on browser state.""" + usernames = sorted(set(usernames)) + if not usernames: + return + cls._execute_docker_compose_command( + [ + "docker", + "compose", + "exec", + "-T", + "dashboard", + "python", + "manage.py", + "shell", + "-c", + ( + "from openwisp_users.models import User; " + f"User.objects.filter(username__in={usernames!r}).delete()" + ), + ], + use_text_mode=True, + ) + def setUp(self): # Override TestSeleniumMixin setUp which uses # Django methods to create superuser @@ -130,7 +199,7 @@ def create_superuser( self.find_element(By.NAME, "password2", driver=driver).send_keys(password) self.find_element(By.NAME, "is_superuser", driver=driver).click() self._click_save_btn(driver) - self.objects_to_delete.append(driver.current_url) + self.test_usernames_to_delete.add(username) self._click_save_btn(driver) self._wait_until_page_ready() self.wait_for_visibility(By.ID, "content", driver=driver, timeout=10)