Merge branch 'main' into feat/e2e-test-completion

Author: patchmemory

.gitmodules

@@ -1,4 +1,4 @@
 [submodule "dev"]
 	path = dev
-	url = git@github.com:patchmemory/scidk-dev.git
+	url = https://github.com/patchmemory/scidk-dev.git
 	branch = main

Makefile

@@ -36,7 +36,7 @@ e2e-install-browsers:
 	.venv/bin/python -m playwright install chromium
 
 # Run headless E2E tests
-# Ensures port 5001 is used by tests; app is auto-started by tests/e2e/conftest.py
+# Requires a running server and BASE_URL to be set (e.g., http://localhost:5000)
 e2e:
 	@mkdir -p dev/test-runs/{tmp,pytest-tmp,artifacts,downloads,pw-browsers}
 	 SCIDK_E2E=1 TMPDIR=$$(pwd)/dev/test-runs/tmp TMP=$$(pwd)/dev/test-runs/tmp TEMP=$$(pwd)/dev/test-runs/tmp PYTEST_ADDOPTS="--basetemp=$$(pwd)/dev/test-runs/pytest-tmp" PLAYWRIGHT_BROWSERS_PATH=$$(pwd)/dev/test-runs/pw-browsers pytest -m e2e tests/e2e -v --maxfail=1

README.md

@@ -62,7 +62,9 @@ To keep CI reliable and PRs easy to review, we follow a simple workflow: one act
 
 ## End-to-End (E2E) tests
 
-These tests run in a real browser using Playwright and pytest. The test suite automatically starts the Flask app on port 5001 with safe defaults and no external Neo4j connection.
+These tests run in a real browser using Playwright and pytest.
+
+Important: For the initial smoke baseline, you must start the server yourself (for example, python -m scidk.app) and set the BASE_URL environment variable for the tests to know where to connect (e.g., http://localhost:5000). The tests are headless by default and designed to be fast and deterministic.
 
 Prereqs (once per machine):
 - Python virtual environment activated.

dev

@@ -31,6 +31,7 @@ Goal: Keep the development flow simple and reliable by working on one active bra
 - Unit tests and smoke checks run on every PR.
 - E2E smoke (where applicable) runs within a few minutes (<5s/spec target).
 - Required checks must be green before merge.
+- Dev submodule freshness: PRs to main must keep dev/ submodule at the latest commit of its configured branch (see .gitmodules). A CI check enforces this, and main auto-syncs dev/ after merge.
 
 ## Tips
 - Use `python -m dev.cli ready-queue` to confirm current priorities.

docs/branching-and-ci.md

@@ -1,120 +1,54 @@
-"""
-Playwright E2E test configuration - manages Flask app startup
-"""
 import os
-import subprocess
+import sys
 import time
-from pathlib import Path
+from typing import Optional, List
 
 import pytest
-import requests
-import sys
-
-FLASK_PORT = 5001
-FLASK_HOST = "127.0.0.1"
-TEST_DB = os.getenv("SCIDK_TEST_DB", "sqlite:///:memory:")
 
+try:
+    import requests  # lightweight reachability check
+except Exception:  # pragma: no cover
+    requests = None  # type: ignore
 
-@pytest.fixture(scope="session", autouse=True)
-def flask_app():
-    """Start Flask app in test mode for the whole test session.
-    Skips E2E entirely unless running in CI or SCIDK_E2E=1 is set, to avoid local Playwright browser issues.
-    """
-    if not (os.environ.get("CI") or os.environ.get("SCIDK_E2E") == "1"):
-        pytest.skip("Skipping E2E: set SCIDK_E2E=1 or run in CI to enable Playwright tests")
-    env = os.environ.copy()
-    env.update({
-        "FLASK_DEBUG": "0",
-        # Make the app listen on the port our tests will hit
-        "SCIDK_PORT": str(FLASK_PORT),
-        # Use in-memory/throwaway DB by default
-        "SCIDK_DB_PATH": TEST_DB,
-        # Prefer sqlite-backed state when supported
-        "SCIDK_STATE_BACKEND": os.environ.get("SCIDK_STATE_BACKEND", "sqlite"),
-        # Ensure no real Neo4j connection attempt occurs
-        "NEO4J_AUTH": "none",
-        # Keep providers simple and reliable for E2E
-        "SCIDK_PROVIDERS": "local_fs",
-        # Feature flags with safe defaults
-        "SCIDK_FEATURE_FILE_INDEX": os.environ.get("SCIDK_FEATURE_FILE_INDEX", "1"),
-        "SCIDK_COMMIT_FROM_INDEX": os.environ.get("SCIDK_COMMIT_FROM_INDEX", "1"),
-    })
 
-    repo_root = Path(__file__).resolve().parents[2]
-    flask_process = subprocess.Popen(
-        [sys.executable, "-m", "scidk.app"],
-        env=env,
-        stdout=subprocess.PIPE,
-        stderr=subprocess.PIPE,
-        cwd=repo_root,
-    )
+@pytest.fixture(scope="session")
+def base_url() -> str:
+    """Resolve BASE_URL from env and verify it is reachable.
 
-    # Wait for Flask to start
-    max_retries = 30
-    for _ in range(max_retries):
-        try:
-            r = requests.get(f"http://{FLASK_HOST}:{FLASK_PORT}/", timeout=0.5)
-            if r.status_code < 500:
-                break
-        except Exception:
-            time.sleep(0.5)
-    else:
+    Skips the E2E session if not set or unreachable to keep CI green until the
+    server orchestration is added. This matches the smoke-baseline plan where
+    the server must be running separately.
+    """
+    url = os.environ.get("BASE_URL") or ""
+    if not url:
+        pytest.skip("BASE_URL is not set; start the server locally and export BASE_URL to run E2E smoke.")
+    # Quick reachability check
+    if requests is not None:
         try:
-            out, err = flask_process.communicate(timeout=1)
+            r = requests.get(url, timeout=2)
+            if r.status_code >= 500:
+                pytest.skip(f"BASE_URL responded with {r.status_code}; skipping E2E smoke")
         except Exception:
-            out = err = b""
-        raise RuntimeError(
-            "Flask app failed to start on E2E bootstrap.\n"
-            f"stdout: {out.decode(errors='ignore')}\n"
-            f"stderr: {err.decode(errors='ignore')}"
-        )
-
-    yield flask_process
-
-    flask_process.terminate()
-    try:
-        flask_process.wait(timeout=10)
-    except Exception:
-        flask_process.kill()
-
-
-@pytest.fixture(scope="session")
-def base_url():
-    return f"http://{FLASK_HOST}:{FLASK_PORT}"
-
-
-@pytest.fixture(scope="session")
-def context_kwargs():
-    """Ensure Playwright downloads and artifacts land under the repo, not system /tmp."""
-    repo_root = Path(__file__).resolve().parents[2]
-    downloads_dir = repo_root / "dev/test-runs/downloads"
-    downloads_dir.mkdir(parents=True, exist_ok=True)
-    return {"acceptDownloads": True, "downloadsPath": str(downloads_dir)}
-
-
-class PageHelpers:
-    """Reusable helpers for common page interactions (sync API)."""
-    def __init__(self, page, base_url):
-        self.page = page
-        self.base_url = base_url
+            pytest.skip("BASE_URL is not reachable; ensure the server is running and accessible")
+    return url.rstrip('/')
 
-    def goto_page(self, path: str):
-        self.page.goto(f"{self.base_url}{path}")
-        self.page.wait_for_load_state("networkidle")
 
-    def fill_and_submit_form(self, field_selectors: dict, submit_button="button[type='submit']"):
-        for selector, value in field_selectors.items():
-            self.page.fill(selector, value)
-        self.page.click(submit_button)
-        self.page.wait_for_load_state("networkidle")
-
-    def wait_for_element(self, selector: str, timeout=5000):
-        self.page.locator(selector).first.wait_for(state="visible", timeout=timeout)
+@pytest.fixture
+def no_console_errors(page):
+    """Ensure the page does not emit console errors during a test.
 
-    def expect_notification(self, message: str, timeout=5000):
-        self.page.get_by_text(message, exact=False).first.wait_for(timeout=timeout)
+    Attaches a listener that records console messages of type 'error'; asserts none at teardown.
+    """
+    errors: List[str] = []
 
+    def _on_console_message(msg):  # type: ignore
+        try:
+            if getattr(msg, 'type', lambda: None)() == 'error':
+                errors.append(str(getattr(msg, 'text', lambda: '')()))
+        except Exception:
+            # Be defensive; do not crash on adapter differences
+            errors.append("<console error (unparsed)>")
 
-@pytest.fixture
-def page_helpers(page, base_url):
-    return PageHelpers(page, base_url)
+    page.on("console", _on_console_message)
+    yield
+    assert not errors, f"Console errors detected: {errors}"

tests/e2e/conftest.py

@@ -0,0 +1,13 @@
+import pytest
+
+pytestmark = pytest.mark.e2e
+
+
+def test_homepage_loads(page, base_url, no_console_errors):
+    # Navigate to home and ensure the request succeeds
+    resp = page.goto(base_url, wait_until="domcontentloaded")
+    assert resp is not None, "No response when navigating to BASE_URL"
+    assert resp.ok, f"Homepage request failed: {resp.status}"
+    # Basic sanity checks on content
+    body_text = page.text_content("body") or ""
+    assert len(body_text) > 0