From 84323bd2aaf47602878cb072e8d7c9ecd6fe74e1 Mon Sep 17 00:00:00 2001 From: Kevin Veen-Birkenbach Date: Wed, 24 Dec 2025 17:22:50 +0100 Subject: [PATCH] test: add integration tests for installer warning detection - add make target test-integration and run it in reusable CI workflow - add integration unittest covering _page_warnings stderr output + deduplication - surface Matomo installer warnings during Playwright flow (stderr only) https://chatgpt.com/share/694c1371-365c-800f-bdf8-ede2e850e648 --- .github/workflows/reusable-test.yml | 15 +++ Makefile | 11 +- src/matomo_bootstrap/installers/web.py | 121 +++++++++++++++-- tests/integration/__init__.py | 0 .../test_web_installer_warnings.py | 126 ++++++++++++++++++ 5 files changed, 262 insertions(+), 11 deletions(-) create mode 100644 tests/integration/__init__.py create mode 100644 tests/integration/test_web_installer_warnings.py diff --git a/.github/workflows/reusable-test.yml b/.github/workflows/reusable-test.yml index 8cf6173..5122af9 100644 --- a/.github/workflows/reusable-test.yml +++ b/.github/workflows/reusable-test.yml @@ -52,6 +52,21 @@ jobs: ruff check . ruff format --check . + integration: + runs-on: ubuntu-latest + needs: lint + steps: + - name: Checkout + uses: actions/checkout@v4 + + - name: Set up Python + uses: actions/setup-python@v5 + with: + python-version: ${{ inputs.python-version }} + + - name: Integration tests + run: make test-integration + e2e: runs-on: ubuntu-latest timeout-minutes: 30 diff --git a/Makefile b/Makefile index 5a805f1..cee7fc1 100644 --- a/Makefile +++ b/Makefile @@ -34,12 +34,13 @@ COMPOSE_STACK := docker compose -f $(COMPOSE_STACK_FILE) .PHONY: help \ venv deps-e2e playwright-install e2e-up e2e-install e2e-test e2e-down e2e logs clean \ + test-integration \ image-build image-run image-shell image-push image-clean \ stack-up stack-down stack-logs stack-ps stack-bootstrap stack-rebootstrap stack-clean stack-reset help: @echo "Targets:" - @echo " venv Create local venv in $(VENV_DIR)" + @echo " venv Create local venv in $(VENV_DIR)" @echo " deps-e2e Install package + E2E deps into venv" @echo " playwright-install Install Chromium for Playwright (inside venv)" @echo " e2e-up Start Matomo + DB for E2E tests" @@ -49,6 +50,7 @@ help: @echo " e2e Full cycle: up → install → test → down" @echo " logs Show Matomo logs (E2E compose)" @echo " clean Stop E2E containers + remove venv" + @echo " test-integration Run integration tests (unittest)" @echo "" @echo "Container image targets:" @echo " image-build Build matomo-bootstrap container image" @@ -129,6 +131,13 @@ logs: clean: e2e-down rm -rf $(VENV_DIR) +# ---------------------------- +# Integration tests +# ---------------------------- + +test-integration: + PYTHONPATH=src $(PYTHON) -m unittest discover -s tests/integration -v + # ---------------------------- # Container image workflow # ---------------------------- diff --git a/src/matomo_bootstrap/installers/web.py b/src/matomo_bootstrap/installers/web.py index 6580dc8..b5b2b54 100644 --- a/src/matomo_bootstrap/installers/web.py +++ b/src/matomo_bootstrap/installers/web.py @@ -33,6 +33,95 @@ def _log(msg: str) -> None: print(msg, file=sys.stderr) +def _page_warnings(page, *, prefix: str = "[install]") -> list[str]: + """ + Detect Matomo installer warnings/errors on the current page. + + - Does NOT change any click logic. + - Prints found warnings/errors to stderr (stdout stays clean). + - Returns a de-duplicated list of warning/error texts (empty if none found). + """ + def _safe(s: str | None) -> str: + return (s or "").strip() + + # Helpful context (doesn't spam much, but makes failures traceable) + try: + url = page.url + except Exception: + url = "" + try: + title = page.title() + except Exception: + title = "" + + selectors = [ + # your originals + ".warning", + ".alert.alert-danger", + ".alert.alert-warning", + ".notification", + ".message_container", + # common Matomo / UI patterns seen across versions + "#notificationContainer", + ".system-check-error", + ".system-check-warning", + ".form-errors", + ".error", + ".errorMessage", + ".invalid-feedback", + ".help-block.error", + ".ui-state-error", + ".alert-danger", + ".alert-warning", + "[role='alert']", + ] + + texts: list[str] = [] + + for sel in selectors: + loc = page.locator(sel) + try: + n = loc.count() + except Exception: + n = 0 + if n <= 0: + continue + + # collect all matches (not only .first) + for i in range(min(n, 50)): # avoid insane spam if page is weird + try: + t = _safe(loc.nth(i).inner_text()) + except Exception: + t = "" + if t: + texts.append(t) + + # Also catch HTML5 validation bubbles / inline field errors + # (Sometimes Matomo marks invalid inputs with aria-invalid + sibling text) + try: + invalid = page.locator("[aria-invalid='true']") + n_invalid = invalid.count() + except Exception: + n_invalid = 0 + + if n_invalid > 0: + texts.append(f"{n_invalid} field(s) marked aria-invalid=true.") + + # De-duplicate while preserving order + seen: set[str] = set() + out: list[str] = [] + for t in texts: + if t not in seen: + seen.add(t) + out.append(t) + + if out: + print(f"{prefix} page warnings/errors detected @ {url} ({title}):", file=sys.stderr) + for idx, t in enumerate(out, 1): + print(f"{prefix} {idx}) {t}", file=sys.stderr) + + return out + def wait_http(url: str, timeout: int = 180) -> None: """ Consider Matomo 'reachable' as soon as the HTTP server answers - even with 500. @@ -102,8 +191,7 @@ class WebInstaller(Installer): wait_http(base_url) if is_installed(base_url): - if config.debug: - _log("[install] Matomo already looks installed. Skipping installer.") + _log("[install] Matomo already looks installed. Skipping installer.") return from playwright.sync_api import sync_playwright @@ -120,10 +208,6 @@ class WebInstaller(Installer): page.set_default_navigation_timeout(PLAYWRIGHT_NAV_TIMEOUT_MS) page.set_default_timeout(PLAYWRIGHT_NAV_TIMEOUT_MS) - def _dbg(msg: str) -> None: - if config.debug: - _log(f"[install] {msg}") - def click_next() -> None: """ Matomo installer mixes link/button variants and sometimes includes '»'. @@ -149,13 +233,11 @@ class WebInstaller(Installer): for role, name in candidates: loc = page.get_by_role(role, name=name) if loc.count() > 0: - _dbg(f"click_next(): {role} '{name}'") loc.first.click() return loc = page.get_by_text("Next", exact=False) if loc.count() > 0: - _dbg("click_next(): fallback text 'Next'") loc.first.click() return @@ -164,6 +246,7 @@ class WebInstaller(Installer): ) page.goto(base_url, wait_until="domcontentloaded") + _page_warnings(page) def superuser_form_visible() -> bool: return page.locator("#login-0").count() > 0 @@ -174,6 +257,7 @@ class WebInstaller(Installer): click_next() page.wait_for_load_state("domcontentloaded") page.wait_for_timeout(200) + _page_warnings(page) else: raise RuntimeError( "Installer did not reach superuser step (login-0 not found)." @@ -191,12 +275,17 @@ class WebInstaller(Installer): page.locator("#email-0").click() page.locator("#email-0").fill(config.admin_email) + _page_warnings(page) if page.get_by_role("button", name="Next »").count() > 0: page.get_by_role("button", name="Next »").click() else: click_next() + page.wait_for_load_state("domcontentloaded") + page.wait_for_timeout(200) + _page_warnings(page) + if page.locator("#siteName-0").count() > 0: page.locator("#siteName-0").click() page.locator("#siteName-0").fill(DEFAULT_SITE_NAME) @@ -205,26 +294,38 @@ class WebInstaller(Installer): page.locator("#url-0").click() page.locator("#url-0").fill(DEFAULT_SITE_URL) + _page_warnings(page) + try: page.get_by_role("combobox").first.click() page.get_by_role("listbox").get_by_text(DEFAULT_TIMEZONE).click() except Exception: - _dbg("Timezone selection skipped (not found / changed UI).") + _log("Timezone selection skipped (not found / changed UI).") try: page.get_by_role("combobox").nth(2).click() page.get_by_role("listbox").get_by_text(DEFAULT_ECOMMERCE).click() except Exception: - _dbg("Ecommerce selection skipped (not found / changed UI).") + _log("Ecommerce selection skipped (not found / changed UI).") + + _page_warnings(page) click_next() page.wait_for_load_state("domcontentloaded") + page.wait_for_timeout(200) + _page_warnings(page) if page.get_by_role("link", name="Next »").count() > 0: page.get_by_role("link", name="Next »").click() + page.wait_for_load_state("domcontentloaded") + page.wait_for_timeout(200) + _page_warnings(page) if page.get_by_role("button", name="Continue to Matomo »").count() > 0: page.get_by_role("button", name="Continue to Matomo »").click() + page.wait_for_load_state("domcontentloaded") + page.wait_for_timeout(200) + _page_warnings(page) context.close() browser.close() diff --git a/tests/integration/__init__.py b/tests/integration/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/integration/test_web_installer_warnings.py b/tests/integration/test_web_installer_warnings.py new file mode 100644 index 0000000..196af10 --- /dev/null +++ b/tests/integration/test_web_installer_warnings.py @@ -0,0 +1,126 @@ +import io +import unittest +from contextlib import redirect_stderr + + +# Import the function under test. +# This keeps the test close to real integration behavior without requiring Playwright. +from matomo_bootstrap.installers.web import _page_warnings + + +class _FakeLocatorNth: + def __init__(self, text: str): + self._text = text + + def inner_text(self) -> str: + return self._text + + +class _FakeLocator: + def __init__(self, texts: list[str]): + self._texts = texts + + def count(self) -> int: + return len(self._texts) + + def nth(self, i: int) -> _FakeLocatorNth: + return _FakeLocatorNth(self._texts[i]) + + +class _FakePage: + """ + Minimal Playwright-like page stub: + - locator(selector) -> object with count() / nth(i).inner_text() + - url, title() + """ + + def __init__(self, *, url: str, title: str, selector_texts: dict[str, list[str]]): + self.url = url + self._title = title + self._selector_texts = selector_texts + + def title(self) -> str: + return self._title + + def locator(self, selector: str) -> _FakeLocator: + return _FakeLocator(self._selector_texts.get(selector, [])) + + +class TestWebInstallerWarningsIntegration(unittest.TestCase): + def test_detects_bootstrap_alert_warning_block(self) -> None: + """ + Matomo installer commonly renders validation errors like: +
...
  • ...
...
+ We must detect and print those messages to stderr. + """ + page = _FakePage( + url="http://matomo/index.php?action=setupSuperUser&module=Installation", + title="Superuser", + selector_texts={ + # The key selector from the observed DOM + ".alert.alert-warning": [ + "Please fix the following errors:\n" + "Password required\n" + "Password (repeat) required\n" + "The email doesn't have a valid format." + ], + }, + ) + + buf = io.StringIO() + with redirect_stderr(buf): + warnings = _page_warnings(page, prefix="[install]") + + # Function must return the warning text + self.assertEqual(len(warnings), 1) + self.assertIn("Please fix the following errors:", warnings[0]) + self.assertIn("The email doesn't have a valid format.", warnings[0]) + + # And it must print it to stderr (stdout must remain token-only in the app) + out = buf.getvalue() + self.assertIn("[install] page warnings/errors detected", out) + self.assertIn("Superuser", out) + self.assertIn("The email doesn't have a valid format.", out) + + def test_deduplicates_repeated_warning_blocks(self) -> None: + """ + Some Matomo versions repeat the same alert in multiple containers. + We must return/log each unique text only once. + """ + repeated = "Please fix the following errors:\nThe email doesn't have a valid format." + page = _FakePage( + url="http://matomo/index.php?action=setupSuperUser&module=Installation", + title="Superuser", + selector_texts={ + ".alert.alert-warning": [repeated, repeated], + }, + ) + + buf = io.StringIO() + with redirect_stderr(buf): + warnings = _page_warnings(page, prefix="[install]") + + self.assertEqual(warnings, [repeated]) + + out = buf.getvalue() + # Only a single numbered entry should be printed + self.assertIn("[install] 1) ", out) + self.assertNotIn("[install] 2) ", out) + + def test_no_output_when_no_warnings(self) -> None: + page = _FakePage( + url="http://matomo/", + title="Welcome", + selector_texts={}, + ) + + buf = io.StringIO() + with redirect_stderr(buf): + warnings = _page_warnings(page, prefix="[install]") + + self.assertEqual(warnings, []) + self.assertEqual(buf.getvalue(), "") + + +if __name__ == "__main__": + unittest.main()