From 9e700dbf02676258e3cfa38821f26ee30677c813 Mon Sep 17 00:00:00 2001 From: Ricardo Boni Date: Wed, 2 Sep 2026 17:16:42 -0400 Subject: [PATCH 1/8] ci(release): smoke-test the built exe before publishing it The exe is the entry point most Windows users take, and it is the one artifact this repo can still gate: main is production for dk-installer.py the moment a merge lands, but the exe is only published after CI has had its say. So the smoke test sits between pyinstaller and the release step -- a failure leaves the previous release in place instead of replacing it with a broken build. tests/e2e/smoke_exe.py installs TestGen for real in pip mode, checks the app serves and the embedded Postgres is up, kills the installer to orphan the app tree, then uninstalls and checks nothing was left behind. That last part is the shape of the bugs found by hand in August: the orphan sweep, and a "TestGen uninstalled." message printed while the tool environment was still on disk. The script asserts the pairing, not just the message. Readiness comes from the install marker plus the UI port rather than the installer's stdout, which is block-buffered once redirected to a file and can withhold the line announcing the app for as long as the app runs. The script is destructive -- it kills every TestGen process on the machine and removes ~/.testgen -- so it refuses to run outside CI without --force. release_exe.yml is now explicit steps rather than python-job.yml: the release step needs an `if` guard, and the smoke logs need uploading on failure. Two knock-on changes from the job going from ~3 to ~25 minutes: a 45 minute timeout, and a concurrency group so two quick merges queue instead of racing to move the `latest` tag. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/_temp_smoke_dryrun.yml | 46 ++++ .github/workflows/release_exe.yml | 77 +++++- tests/e2e/smoke_exe.py | 331 +++++++++++++++++++++++ 3 files changed, 440 insertions(+), 14 deletions(-) create mode 100644 .github/workflows/_temp_smoke_dryrun.yml create mode 100644 tests/e2e/smoke_exe.py diff --git a/.github/workflows/_temp_smoke_dryrun.yml b/.github/workflows/_temp_smoke_dryrun.yml new file mode 100644 index 0000000..ec324c8 --- /dev/null +++ b/.github/workflows/_temp_smoke_dryrun.yml @@ -0,0 +1,46 @@ +# TEMPORARY -- delete before merging this branch. +# +# release_exe.yml runs only on a push to main, and its workflow_dispatch trigger is not +# usable until that file is on main, so there is no other way to exercise the smoke test +# from a branch. This runs the same steps, minus everything that publishes. +name: TEMP exe smoke dry run + +on: + pull_request: + branches: [ main ] + +jobs: + dry-run: + runs-on: windows-latest + timeout-minutes: 45 + defaults: + run: + shell: bash + steps: + - name: Checkout source code + uses: actions/checkout@v4 + + - name: Set up Python + uses: actions/setup-python@v5 + with: + cache: 'pip' + python-version: "3.9" + + - name: Install project dependencies + run: | + python -m pip install --upgrade pip + pip install .[dev,test] + + - name: Build the executable + run: pyinstaller --onefile --exclude demo dk-installer.py + + - name: Smoke-test the executable + run: python tests/e2e/smoke_exe.py dist/dk-installer.exe + + - name: Upload smoke-test logs + if: always() + uses: actions/upload-artifact@v4 + with: + name: smoke-test-logs + path: smoke/ + if-no-files-found: ignore diff --git a/.github/workflows/release_exe.yml b/.github/workflows/release_exe.yml index 4964285..2c1006d 100644 --- a/.github/workflows/release_exe.yml +++ b/.github/workflows/release_exe.yml @@ -4,19 +4,68 @@ on: push: branches: - main + + # Lets the smoke test run against a branch without publishing anything: the release step + # below is gated on a push to main. + workflow_dispatch: + +# The smoke test makes this job ~25 minutes instead of ~3, so two merges in quick +# succession can now overlap. Queue them rather than cancelling: the release step deletes +# the tag before recreating it, and a run cancelled inside that window would leave no +# release at all. +concurrency: + group: release-windows-build + cancel-in-progress: false + jobs: windows-release: - uses: ./.github/workflows/python-job.yml - secrets: inherit - with: - runner: windows-latest - python-version: "3.9" - run: | - pyinstaller --onefile --exclude demo dk-installer.py - export RELEASE_TAG=latest - gh release delete "${RELEASE_TAG}" -y || true - git push origin --delete "${RELEASE_TAG}" || true - git tag "${RELEASE_TAG}" - git push origin "${RELEASE_TAG}" - gh release create "${RELEASE_TAG}" ./dist/dk-installer.exe \ - --title "Latest Release" + runs-on: windows-latest + timeout-minutes: 45 + defaults: + run: + shell: bash + steps: + - name: Checkout source code + uses: actions/checkout@v4 + + - name: Set up Python + uses: actions/setup-python@v5 + with: + cache: 'pip' + python-version: "3.9" + + - name: Install project dependencies + run: | + python -m pip install --upgrade pip + pip install .[dev,test] + + - name: Build the executable + run: pyinstaller --onefile --exclude demo dk-installer.py + + # Installs TestGen for real in pip mode, checks the app serves, kills the installer + # and uninstalls. Deliberately ahead of the release step: the exe is the entry point + # most Windows users take, and this is the one place it can still be gated -- a + # failure here leaves the previous release in place instead of replacing it. + - name: Smoke-test the executable + run: python tests/e2e/smoke_exe.py dist/dk-installer.exe + + - name: Upload smoke-test logs + if: always() + uses: actions/upload-artifact@v4 + with: + name: smoke-test-logs + path: smoke/ + if-no-files-found: ignore + + - name: Publish the release + if: github.event_name == 'push' && github.ref == 'refs/heads/main' + env: + GH_TOKEN: ${{ github.token }} + run: | + export RELEASE_TAG=latest + gh release delete "${RELEASE_TAG}" -y || true + git push origin --delete "${RELEASE_TAG}" || true + git tag "${RELEASE_TAG}" + git push origin "${RELEASE_TAG}" + gh release create "${RELEASE_TAG}" ./dist/dk-installer.exe \ + --title "Latest Release" diff --git a/tests/e2e/smoke_exe.py b/tests/e2e/smoke_exe.py new file mode 100644 index 0000000..f45e62b --- /dev/null +++ b/tests/e2e/smoke_exe.py @@ -0,0 +1,331 @@ +"""Install TestGen for real with a built installer, prove it runs, then uninstall it. + +Run as ``python tests/e2e/smoke_exe.py dist/dk-installer.exe`` (a ``dk-installer.py`` path +works too). Exercises the pip (standalone) path end to end: the uv bootstrap, ``uv tool +install``, the embedded Postgres, ``standalone-setup``, and the orphan sweep in ``tg +delete``. All of that only fails for real -- unit tests reach it through mocks. + +The installer is killed rather than interrupted, on purpose. A clean Ctrl+C never reaches +``force_kill_app_tree``, so it does not exercise the sweep at all; only an orphaned tree +makes ``tg delete`` find the processes by command line, which is where the Windows bugs +were. Delivering a real console Ctrl+C from a CI step is a separate problem, left alone. + +Readiness is taken from the install marker plus the UI port, never from the installer's +stdout: redirected to a file that output is block-buffered, so the line announcing the app +can sit unflushed for as long as the app runs. + +Every check is collected rather than raised, so one run reports everything that is wrong. +Session logs land in ``smoke/`` for the workflow to upload. +""" + +import argparse +import json +import os +import platform +import shutil +import socket +import subprocess +import sys +import time +import urllib.error +import urllib.request +from pathlib import Path + +WINDOWS = platform.system() == "Windows" + +UI_PORT = 8501 +API_PORT = 8530 +# The install pulls uv, a Python 3.13 and TestGen's whole dependency tree, then runs initdb. +# 4~8 minutes is what the installer itself promises; allow well past it before calling it a hang. +INSTALL_TIMEOUT = 20 * 60 +# Mirrors STANDALONE_PROC_PATTERNS in dk-installer.py, with separators already normalised. +# Duplicated rather than imported: the subject here is the built artifact, not the source +# sitting next to it. +PROC_PATTERNS = ("testgen.*run-app", "tools/dataops-testgen") + +OUT_DIR = Path("smoke") + + +class Report: + """Collects checks so a run reports every failure, not just the first.""" + + def __init__(self): + self.failures = [] + + def check(self, ok, label, detail=""): + suffix = f" -- {detail}" if detail else "" + print(f" [{'PASS' if ok else 'FAIL'}] {label}{suffix}", flush=True) + if not ok: + self.failures.append(f"{label}{suffix}") + return ok + + def note(self, label, detail=""): + print(f" [note] {label}{f' -- {detail}' if detail else ''}", flush=True) + + +def step(title): + print(f"\n=== {title} ===", flush=True) + + +def data_folder(installer): + """Where the installer keeps the marker, the credentials and its session logs.""" + if WINDOWS: + return Path(os.environ["LOCALAPPDATA"]) / "DataKitchenApps" + return installer.resolve().parent + + +def logs_folder(installer): + base = data_folder(installer) + return base / "logs" if WINDOWS else base / ".dk-installer" + + +def testgen_home(): + return Path(os.environ.get("TG_TESTGEN_HOME", Path.home() / ".testgen")) + + +def resolve_uv(installer): + """The uv the installer would use: its own bootstrapped copy, else one on PATH.""" + local = data_folder(installer) / "bin" / ("uv.exe" if WINDOWS else "uv") + return str(local) if local.exists() else shutil.which("uv") + + +def run(cmd, **kwargs): + kwargs.setdefault("capture_output", True) + kwargs.setdefault("text", True) + return subprocess.run(cmd, check=False, **kwargs) + + +def http_get(url): + """(status, first bytes of body), or (None, reason) when the request could not be made.""" + try: + with urllib.request.urlopen(url, timeout=15) as response: + return response.status, response.read(200).decode("utf-8", "replace") + except urllib.error.HTTPError as e: + return e.code, "" + except Exception as e: # URLError, timeouts, refused connections + return None, str(e) + + +def port_open(port): + try: + with socket.create_connection(("localhost", port), timeout=5): + return True + except OSError: + return False + + +def standalone_pids(): + """PIDs of the processes a standalone install spawns: testgen, streamlit, postgres.""" + if WINDOWS: + clause = " -or ".join(f"($cmd -match '{p}') -or ($exe -match '{p}')" for p in PROC_PATTERNS) + script = ( + "$ErrorActionPreference = 'SilentlyContinue'; " + "Get-CimInstance Win32_Process | Where-Object { " + "$cmd = ($_.CommandLine -replace '\\\\', '/'); " + "$exe = ($_.ExecutablePath -replace '\\\\', '/'); " + clause + " } | ForEach-Object { $_.ProcessId }" + ) + result = run(["powershell", "-NoProfile", "-NonInteractive", "-Command", script]) + else: + result = run(["pgrep", "-f", "|".join(PROC_PATTERNS)]) + return sorted(int(line) for line in result.stdout.split() if line.strip().isdigit()) + + +def wait_until_running(proc, installer, report): + """Wait for the marker the installer writes just before it starts the app, then the port.""" + marker = data_folder(installer) / "dk-tg-install.json" + deadline = time.time() + INSTALL_TIMEOUT + while time.time() < deadline: + if marker.exists() and port_open(UI_PORT): + return True + if proc.poll() is not None: + report.check(False, "installer stayed up until the app was ready", f"exited {proc.returncode}") + return False + time.sleep(5) + report.check(False, "install finished within the timeout", f"{INSTALL_TIMEOUT}s elapsed") + return False + + +def check_running_install(installer, report): + """What must hold while the app is up, checked before anything is torn down.""" + status, _ = http_get(f"http://localhost:{UI_PORT}/") + report.check(status == 200, f"UI answers on port {UI_PORT}", f"status {status}") + + # Streamlit's own liveness endpoint. Its path has moved between releases, so this is + # reported rather than required -- the UI check above is the gate. + for path in ("/_stcore/health", "/healthz"): + health, body = http_get(f"http://localhost:{UI_PORT}{path}") + if health == 200: + report.note(f"health endpoint {path}", body.strip()[:40]) + break + + # TCP only: the API's routes are TestGen's to define, and pinning one here would break + # this gate on an unrelated rename. Tighten once we have seen what it serves. + report.check(port_open(API_PORT), f"API port {API_PORT} accepts connections") + + postmaster = testgen_home() / "pgdata" / "postmaster.pid" + report.check(postmaster.exists(), "embedded Postgres is running", str(postmaster)) + + config = testgen_home() / "config.env" + config_text = config.read_text(encoding="utf-8", errors="replace") if config.exists() else "" + report.check(f"TG_UI_PORT={UI_PORT}" in config_text, "standalone-setup persisted the UI port") + + marker = data_folder(installer) / "dk-tg-install.json" + mode = json.loads(marker.read_text()).get("install_mode") if marker.exists() else None + report.check(mode == "pip", "install marker records pip mode", f"got {mode!r}") + + creds = data_folder(installer) / "dk-tg-credentials.txt" + report.check(creds.exists() and creds.stat().st_size > 0, "credentials file written") + + uv_path = resolve_uv(installer) + listed = run([uv_path, "tool", "list"]).stdout if uv_path else "" + report.check("dataops-testgen" in listed, "uv reports the tool installed", listed.strip()[:80]) + + app_log = testgen_home() / "logs" / "app.log" + app_text = app_log.read_text(encoding="utf-8", errors="replace") if app_log.exists() else "" + report.check("Traceback" not in app_text, "no traceback in the app log") + + +def uv_tool_paths(installer): + """uv's tool env and shim for TestGen, resolved before the delete removes uv itself.""" + uv_path = resolve_uv(installer) + if not uv_path: + return [] + paths = [] + tool_dir = run([uv_path, "tool", "dir"]).stdout.strip() + if tool_dir: + paths.append(Path(tool_dir) / "dataops-testgen") + bin_dir = run([uv_path, "tool", "dir", "--bin"]).stdout.strip() + if bin_dir: + paths.append(Path(bin_dir) / ("testgen.exe" if WINDOWS else "testgen")) + return paths + + +def kill_installer(proc): + """Kill only the installer, orphaning its app tree -- the sweep's whole reason to exist.""" + if WINDOWS: + # Deliberately no /T: killing the tree would do the sweep's job for it. + run(["taskkill", "/F", "/PID", str(proc.pid)]) + else: + os.kill(proc.pid, 9) + proc.wait(timeout=30) + time.sleep(3) + + +def check_delete(installer, output, tool_paths, report): + """The delete's own report has to match what is actually left on disk.""" + survived = [] + pids = standalone_pids() + if pids: + survived.append(f"processes {pids}") + for path in [*tool_paths, testgen_home()]: + if path.exists(): + survived.append(str(path)) + for name in ("dk-tg-install.json", "dk-tg-credentials.txt"): + if (data_folder(installer) / name).exists(): + survived.append(name) + + report.check(not survived, "nothing survived the delete", "; ".join(survived)) + + claimed = "TestGen uninstalled." in output + if survived: + # The bug this guards against: a success message printed while the tool environment + # and shim were still on disk, because a live process held them open. + report.check(not claimed, "delete did not claim a success it cannot back up") + else: + report.check(claimed, "delete reported success") + + +def collect_logs(installer): + OUT_DIR.mkdir(exist_ok=True) + session_logs = logs_folder(installer) + if session_logs.exists(): + for zipped in sorted(session_logs.glob("*.zip")): + shutil.copy2(zipped, OUT_DIR / zipped.name) + app_log = testgen_home() / "logs" / "app.log" + if app_log.exists(): + shutil.copy2(app_log, OUT_DIR / "testgen-app.log") + + +def refuse_outside_ci(force): + """This script is destructive. Make running it by hand on a workstation deliberate. + + It kills every TestGen process on the machine (the installer's own sweep does that + before standalone-setup) and ``tg delete`` then removes the TestGen data directory and + uv's tool environment -- including an install that was already there. + """ + if force or os.environ.get("CI"): + return + sys.exit( + "Refusing to run outside CI. This installs TestGen for real, kills every TestGen\n" + f"process on this machine, and deletes {testgen_home()} along with uv's\n" + "dataops-testgen tool environment -- an existing install included.\n" + "Run it in a throwaway container, or pass --force if you mean it." + ) + + +def main(): + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("installer", type=Path, help="the dk-installer executable (or .py) to test") + parser.add_argument("--force", action="store_true", help="run outside CI, destroying any local install") + args = parser.parse_args() + + refuse_outside_ci(args.force) + + installer = args.installer + if not installer.exists(): + sys.exit(f"No installer at {installer}") + + OUT_DIR.mkdir(exist_ok=True) + install_log = OUT_DIR / "install.log" + report = Report() + + command = [sys.executable, str(installer)] if installer.suffix == ".py" else [str(installer.resolve())] + + step("install (backgrounded: the installer blocks running the app)") + with install_log.open("w", encoding="utf-8") as log_file: + proc = subprocess.Popen( + [*command, "tg", "install", "--pip", "--no-demo", "--no-analytics"], + stdout=log_file, + stderr=subprocess.STDOUT, + stdin=subprocess.DEVNULL, + # Unbuffered so the log is useful while the run is still going; readiness does + # not depend on it either way. + env={**os.environ, "PYTHONUNBUFFERED": "1"}, + ) + running = wait_until_running(proc, installer, report) + + print(install_log.read_text(encoding="utf-8", errors="replace")[-3000:], flush=True) + + if not running: + collect_logs(installer) + sys.exit("\n".join(["the install never reached a running app:", *report.failures])) + + step("what a finished install must look like") + check_running_install(installer, report) + tool_paths = uv_tool_paths(installer) + + step("kill the installer, orphaning the app tree") + kill_installer(proc) + orphans = standalone_pids() + if orphans: + report.note("orphans left by the dirty exit", str(orphans)) + else: + report.note("nothing survived the kill", "the sweep has nothing to prove in this run") + + step("delete") + deleted = run([*command, "tg", "delete", "--no-analytics"]) + print(deleted.stdout[-3000:], flush=True) + (OUT_DIR / "delete.log").write_text(deleted.stdout + deleted.stderr, encoding="utf-8") + report.check(deleted.returncode == 0, "tg delete exited cleanly", f"rc {deleted.returncode}") + check_delete(installer, deleted.stdout, tool_paths, report) + + collect_logs(installer) + + step("result") + if report.failures: + sys.exit("\n".join([f"{len(report.failures)} check(s) failed:", *(f" - {f}" for f in report.failures)])) + print(" all checks passed", flush=True) + + +if __name__ == "__main__": + main() From f0ab87b9d035a49f1551ce80aa4e0610a4d70580 Mon Sep 17 00:00:00 2001 From: Ricardo Boni Date: Wed, 2 Sep 2026 17:20:57 -0400 Subject: [PATCH 2/8] fix(ci): opt out of analytics by env var, and test the smoke driver's argv The first smoke run failed in 5 seconds: --no-analytics is a top-level flag, so it cannot follow the product name. Setting DK_INSTALLER_ANALYTICS=no in the child environment is what the flag reads its default from anyway, and it applies to every invocation with no ordering to get wrong. The argv now lives in module constants so a unit test can parse it against the installer's real parser. That job only runs on a merge to main, so without this a renamed flag surfaces 25 minutes into a release build. Co-Authored-By: Claude Opus 5 (1M context) --- tests/e2e/__init__.py | 0 tests/e2e/smoke_exe.py | 14 ++++++++++++-- tests/test_smoke_exe_args.py | 31 +++++++++++++++++++++++++++++++ 3 files changed, 43 insertions(+), 2 deletions(-) create mode 100644 tests/e2e/__init__.py create mode 100644 tests/test_smoke_exe_args.py diff --git a/tests/e2e/__init__.py b/tests/e2e/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/e2e/smoke_exe.py b/tests/e2e/smoke_exe.py index f45e62b..17e72a4 100644 --- a/tests/e2e/smoke_exe.py +++ b/tests/e2e/smoke_exe.py @@ -45,6 +45,12 @@ OUT_DIR = Path("smoke") +# Module level so a unit test can parse them against the installer's own parser. This +# script runs only on a merge to main, so a rename here (or of a flag it passes) would +# otherwise surface 25 minutes into a release build. +INSTALL_ARGS = ("tg", "install", "--pip", "--no-demo") +DELETE_ARGS = ("tg", "delete") + class Report: """Collects checks so a run reports every failure, not just the first.""" @@ -275,6 +281,10 @@ def main(): if not installer.exists(): sys.exit(f"No installer at {installer}") + # Opt out of analytics for every child: this is the default source for the top-level + # --no-analytics flag, and unlike the flag it cannot be passed in the wrong position. + os.environ["DK_INSTALLER_ANALYTICS"] = "no" + OUT_DIR.mkdir(exist_ok=True) install_log = OUT_DIR / "install.log" report = Report() @@ -284,7 +294,7 @@ def main(): step("install (backgrounded: the installer blocks running the app)") with install_log.open("w", encoding="utf-8") as log_file: proc = subprocess.Popen( - [*command, "tg", "install", "--pip", "--no-demo", "--no-analytics"], + [*command, *INSTALL_ARGS], stdout=log_file, stderr=subprocess.STDOUT, stdin=subprocess.DEVNULL, @@ -313,7 +323,7 @@ def main(): report.note("nothing survived the kill", "the sweep has nothing to prove in this run") step("delete") - deleted = run([*command, "tg", "delete", "--no-analytics"]) + deleted = run([*command, *DELETE_ARGS]) print(deleted.stdout[-3000:], flush=True) (OUT_DIR / "delete.log").write_text(deleted.stdout + deleted.stderr, encoding="utf-8") report.check(deleted.returncode == 0, "tg delete exited cleanly", f"rc {deleted.returncode}") diff --git a/tests/test_smoke_exe_args.py b/tests/test_smoke_exe_args.py new file mode 100644 index 0000000..fc4087b --- /dev/null +++ b/tests/test_smoke_exe_args.py @@ -0,0 +1,31 @@ +"""The exe smoke test drives the installer through its CLI, and only runs on a merge to +main. Parsing its argv against the real parser here turns a 25-minute round trip into a +2-second one -- the first run of that job failed because ``--no-analytics`` is a top-level +flag that cannot follow the product name. +""" + +import pytest + +from .e2e.smoke_exe import DELETE_ARGS, INSTALL_ARGS +from .installer import get_installer_instance + + +@pytest.mark.unit +def test_smoke_install_args_are_accepted(): + args = get_installer_instance().parser.parse_args(list(INSTALL_ARGS)) + + assert args.prod == "tg" + assert args.install_mode == "pip" + # The demo step is skipped on purpose: it is `required = False`, so it could not gate + # the release anyway, and it is the longest part of the install. + assert args.generate_demo is False + + +@pytest.mark.unit +def test_smoke_delete_args_are_accepted(): + args = get_installer_instance().parser.parse_args(list(DELETE_ARGS)) + + assert args.prod == "tg" + # Nothing is kept: the smoke test asserts the uninstall left nothing behind. + assert args.keep_data is False + assert args.keep_images is False From f3d100d2b572f16db91b888232128558297d95b7 Mon Sep 17 00:00:00 2001 From: Ricardo Boni Date: Wed, 2 Sep 2026 17:26:31 -0400 Subject: [PATCH 3/8] fix(ci): read TestGen's log after the app is gone, not while it holds it open The smoke test got all the way through a real Windows install -- uv, the embedded database, standalone-setup, UI on 8501, API on 8530 -- then crashed on its last check: reading ~/.testgen/logs/app.log raised PermissionError, because Windows denies the read while TestGen has the file open. The log is now saved and scanned between the kill and the delete, the only window where the tree is gone and the directory still exists. Reported rather than failed: the UI, API and Postgres checks are the gate, and a traceback that breaks none of them should not block a release. Worth promoting once we know the log is quiet in practice. Also reports the embedded Postgres major version from pgdata/PG_VERSION. The install passed "Verifying the embedded database" on the runner, which either means TestGen no longer initializes new clusters on PG18 or the runner has libwinpthread-1.dll on PATH via Git for Windows -- TG-1245 either way, and PG_VERSION is what tells the two apart. Every file read the script makes near a live install is now best-effort, so a locked or vanished log cannot mask the result being reported. Co-Authored-By: Claude Opus 5 (1M context) --- tests/e2e/smoke_exe.py | 60 ++++++++++++++++++++++++++++++++++-------- 1 file changed, 49 insertions(+), 11 deletions(-) diff --git a/tests/e2e/smoke_exe.py b/tests/e2e/smoke_exe.py index 17e72a4..004066a 100644 --- a/tests/e2e/smoke_exe.py +++ b/tests/e2e/smoke_exe.py @@ -112,6 +112,18 @@ def http_get(url): return None, str(e) +def read_text_safe(path): + """The file's text, or None when it is absent or cannot be read. + + Windows denies the read outright while another process holds the file open, which is + exactly the state TestGen's own log is in while the app is running. + """ + try: + return path.read_text(encoding="utf-8", errors="replace") + except OSError: + return None + + def port_open(port): try: with socket.create_connection(("localhost", port), timeout=5): @@ -171,8 +183,7 @@ def check_running_install(installer, report): postmaster = testgen_home() / "pgdata" / "postmaster.pid" report.check(postmaster.exists(), "embedded Postgres is running", str(postmaster)) - config = testgen_home() / "config.env" - config_text = config.read_text(encoding="utf-8", errors="replace") if config.exists() else "" + config_text = read_text_safe(testgen_home() / "config.env") or "" report.check(f"TG_UI_PORT={UI_PORT}" in config_text, "standalone-setup persisted the UI port") marker = data_folder(installer) / "dk-tg-install.json" @@ -184,11 +195,12 @@ def check_running_install(installer, report): uv_path = resolve_uv(installer) listed = run([uv_path, "tool", "list"]).stdout if uv_path else "" - report.check("dataops-testgen" in listed, "uv reports the tool installed", listed.strip()[:80]) + report.check("dataops-testgen" in listed, "uv reports the tool installed", listed.splitlines()[0] if listed else "") - app_log = testgen_home() / "logs" / "app.log" - app_text = app_log.read_text(encoding="utf-8", errors="replace") if app_log.exists() else "" - report.check("Traceback" not in app_text, "no traceback in the app log") + # Which major version a new cluster initialized on. Reported because it answers TG-1245: + # pgserver 0.6.0's PostgreSQL 18 build cannot start on a Windows box without + # libwinpthread-1.dll, which the wheel does not ship. + report.note("embedded Postgres version", (read_text_safe(testgen_home() / "pgdata" / "PG_VERSION") or "?").strip()) def uv_tool_paths(installer): @@ -217,6 +229,28 @@ def kill_installer(proc): time.sleep(3) +def save_and_scan_app_log(report): + """Copy TestGen's log aside, then look for a traceback in it. + + Only possible between the kill and the delete: the app holds the file open while it + runs, and ``tg delete`` takes the whole directory. Reported rather than failed -- the + UI, API and Postgres checks are the gate, and a traceback that breaks none of them + should not block a release. Promote it once we know the log is quiet in practice. + """ + app_log = testgen_home() / "logs" / "app.log" + text = read_text_safe(app_log) + if text is None: + report.note("app log unreadable", str(app_log)) + return + OUT_DIR.mkdir(exist_ok=True) + (OUT_DIR / "testgen-app.log").write_text(text, encoding="utf-8") + if "Traceback" in text: + first = next(line for line in text.splitlines() if "Traceback" in line) + report.note("traceback in the app log", first.strip()[:120]) + else: + report.note("app log has no traceback") + + def check_delete(installer, output, tool_paths, report): """The delete's own report has to match what is actually left on disk.""" survived = [] @@ -242,14 +276,17 @@ def check_delete(installer, output, tool_paths, report): def collect_logs(installer): + """Copy the installer's per-command session zips out for the workflow to upload.""" OUT_DIR.mkdir(exist_ok=True) session_logs = logs_folder(installer) - if session_logs.exists(): - for zipped in sorted(session_logs.glob("*.zip")): + if not session_logs.exists(): + return + for zipped in sorted(session_logs.glob("*.zip")): + # Best-effort: a locked or vanished log must not mask the result being reported. + try: shutil.copy2(zipped, OUT_DIR / zipped.name) - app_log = testgen_home() / "logs" / "app.log" - if app_log.exists(): - shutil.copy2(app_log, OUT_DIR / "testgen-app.log") + except OSError as e: + print(f" [note] could not copy {zipped.name} -- {e}", flush=True) def refuse_outside_ci(force): @@ -316,6 +353,7 @@ def main(): step("kill the installer, orphaning the app tree") kill_installer(proc) + save_and_scan_app_log(report) orphans = standalone_pids() if orphans: report.note("orphans left by the dirty exit", str(orphans)) From ffa2bc5baec7d8cc0b219425d5ae57792d617a7a Mon Sep 17 00:00:00 2001 From: Ricardo Boni Date: Wed, 2 Sep 2026 17:35:03 -0400 Subject: [PATCH 4/8] ci(release): name what survives a delete, poll the API, redact the password Three things the second smoke run turned up. The delete left one process behind out of 24 orphans, while printing "TestGen uninstalled." -- the exact pairing this test exists to catch. But the report named only a pid, which is not actionable, and a force-killed process can still be enumerated for a moment after it is gone. So survivors are now listed with their command lines, and the check waits up to 20s for the sweep's kills to settle before calling anything a leak. The API port was closed 4 seconds after the UI answered. The installer advertises "API & MCP: http://localhost:8530" in the credentials it prints and passes TG_API_PORT to standalone-setup, so something should be listening, but whether it binds later than the UI or not at all is TestGen's answer to give. Now polled for 60s and reported rather than gated; polling tells the two apart. The captured stdout carries the password the installer generates, and this artifact is downloadable from a public repo. The install and delete logs are redacted before they are printed or kept. pgdata/PG_VERSION came back 18, so the runner is masking TG-1245: the PG18 build starts there because Git for Windows puts libwinpthread-1.dll on PATH. A user without it still cannot install. Reported, not gated -- this gate cannot see that class of failure. Also: ``pgrep -a`` prints command lines on Linux but does not exist on BSD/macOS, so they are read per pid with ps instead. Co-Authored-By: Claude Opus 5 (1M context) --- tests/e2e/smoke_exe.py | 92 +++++++++++++++++++++++++++++++++--------- 1 file changed, 72 insertions(+), 20 deletions(-) diff --git a/tests/e2e/smoke_exe.py b/tests/e2e/smoke_exe.py index 004066a..6edaa23 100644 --- a/tests/e2e/smoke_exe.py +++ b/tests/e2e/smoke_exe.py @@ -22,6 +22,7 @@ import json import os import platform +import re import shutil import socket import subprocess @@ -52,6 +53,15 @@ DELETE_ARGS = ("tg", "delete") +# The installer prints the generated password once, so the captured stdout carries it. The +# instance is gone with the runner, but the artifact is downloadable from a public repo. +PASSWORD_RE = re.compile(r"(Password:\s*)(\S+)") + + +def redacted(text): + return PASSWORD_RE.sub(r"\1***", text) + + class Report: """Collects checks so a run reports every failure, not just the first.""" @@ -132,20 +142,48 @@ def port_open(port): return False -def standalone_pids(): - """PIDs of the processes a standalone install spawns: testgen, streamlit, postgres.""" +def standalone_procs(): + """``{pid: command line}`` for every process a standalone install spawns. + + The command line matters: a survivor is only actionable if we can say which process it + was, and that is precisely what the sweep matches on. + """ if WINDOWS: clause = " -or ".join(f"($cmd -match '{p}') -or ($exe -match '{p}')" for p in PROC_PATTERNS) script = ( "$ErrorActionPreference = 'SilentlyContinue'; " "Get-CimInstance Win32_Process | Where-Object { " "$cmd = ($_.CommandLine -replace '\\\\', '/'); " - "$exe = ($_.ExecutablePath -replace '\\\\', '/'); " + clause + " } | ForEach-Object { $_.ProcessId }" + "$exe = ($_.ExecutablePath -replace '\\\\', '/'); " + clause + " } | " + 'ForEach-Object { "$($_.ProcessId)|$($_.CommandLine)" }' ) result = run(["powershell", "-NoProfile", "-NonInteractive", "-Command", script]) else: - result = run(["pgrep", "-f", "|".join(PROC_PATTERNS)]) - return sorted(int(line) for line in result.stdout.split() if line.strip().isdigit()) + # ``pgrep -a`` prints command lines on Linux but does not exist on BSD/macOS, so the + # command line is read per pid with ps, which behaves the same on both. + found = run(["pgrep", "-f", "|".join(PROC_PATTERNS)]) + lines = [] + for pid in (p for p in found.stdout.split() if p.strip().isdigit()): + cmdline = run(["ps", "-p", pid, "-o", "command="]).stdout.strip() + lines.append(f"{pid}|{cmdline}") + result = subprocess.CompletedProcess(args=(), returncode=0, stdout="\n".join(lines), stderr="") + + procs = {} + for line in result.stdout.splitlines(): + pid, _, cmdline = line.strip().partition("|") + if pid.isdigit(): + procs[int(pid)] = cmdline.strip() + return procs + + +def wait_for(predicate, timeout): + """Poll until the predicate is true, returning whether it became true in time.""" + deadline = time.time() + timeout + while time.time() < deadline: + if predicate(): + return True + time.sleep(2) + return predicate() def wait_until_running(proc, installer, report): @@ -176,9 +214,14 @@ def check_running_install(installer, report): report.note(f"health endpoint {path}", body.strip()[:40]) break - # TCP only: the API's routes are TestGen's to define, and pinning one here would break - # this gate on an unrelated rename. Tighten once we have seen what it serves. - report.check(port_open(API_PORT), f"API port {API_PORT} accepts connections") + # Reported, not gated. The installer advertises "API & MCP: http://localhost:" in + # the credentials it prints, so this should be listening -- but a first run found it + # closed, and whether it binds later than the UI or not at all is TestGen's answer to + # give. Polling tells the two apart; promote to a check once we know which. + if wait_for(lambda: port_open(API_PORT), timeout=60): + report.note(f"API port {API_PORT} accepts connections") + else: + report.note(f"API port {API_PORT} never opened", "installer advertises it in the credentials") postmaster = testgen_home() / "pgdata" / "postmaster.pid" report.check(postmaster.exists(), "embedded Postgres is running", str(postmaster)) @@ -232,13 +275,15 @@ def kill_installer(proc): def save_and_scan_app_log(report): """Copy TestGen's log aside, then look for a traceback in it. - Only possible between the kill and the delete: the app holds the file open while it - runs, and ``tg delete`` takes the whole directory. Reported rather than failed -- the - UI, API and Postgres checks are the gate, and a traceback that breaks none of them - should not block a release. Promote it once we know the log is quiet in practice. + Best-effort, and on Windows usually unreadable: the app tree we deliberately orphaned is + what holds the file open, so killing the installer does not release it, and ``tg delete`` + then takes the whole directory. Reported rather than failed either way -- the UI and + Postgres checks are the gate, and a traceback that breaks neither should not block a + release. """ app_log = testgen_home() / "logs" / "app.log" text = read_text_safe(app_log) + if text is None: report.note("app log unreadable", str(app_log)) return @@ -254,9 +299,12 @@ def save_and_scan_app_log(report): def check_delete(installer, output, tool_paths, report): """The delete's own report has to match what is actually left on disk.""" survived = [] - pids = standalone_pids() - if pids: - survived.append(f"processes {pids}") + # A force-killed process can still be enumerated for a moment after it is gone, so give + # the sweep's kills a chance to settle before calling anything a leak. + if not wait_for(lambda: not standalone_procs(), timeout=20): + for pid, cmdline in sorted(standalone_procs().items()): + report.note(f"survivor {pid}", cmdline[:160]) + survived.append(f"processes {sorted(standalone_procs())}") for path in [*tool_paths, testgen_home()]: if path.exists(): survived.append(str(path)) @@ -341,7 +389,11 @@ def main(): ) running = wait_until_running(proc, installer, report) - print(install_log.read_text(encoding="utf-8", errors="replace")[-3000:], flush=True) + # Rewritten in place, so neither the CI log below nor the uploaded artifact carries the + # generated password. + install_text = redacted(install_log.read_text(encoding="utf-8", errors="replace")) + install_log.write_text(install_text, encoding="utf-8") + print(install_text[-3000:], flush=True) if not running: collect_logs(installer) @@ -354,16 +406,16 @@ def main(): step("kill the installer, orphaning the app tree") kill_installer(proc) save_and_scan_app_log(report) - orphans = standalone_pids() + orphans = standalone_procs() if orphans: - report.note("orphans left by the dirty exit", str(orphans)) + report.note(f"{len(orphans)} orphans left by the dirty exit", str(sorted(orphans))) else: report.note("nothing survived the kill", "the sweep has nothing to prove in this run") step("delete") deleted = run([*command, *DELETE_ARGS]) - print(deleted.stdout[-3000:], flush=True) - (OUT_DIR / "delete.log").write_text(deleted.stdout + deleted.stderr, encoding="utf-8") + print(redacted(deleted.stdout)[-3000:], flush=True) + (OUT_DIR / "delete.log").write_text(redacted(deleted.stdout + deleted.stderr), encoding="utf-8") report.check(deleted.returncode == 0, "tg delete exited cleanly", f"rc {deleted.returncode}") check_delete(installer, deleted.stdout, tool_paths, report) From 28520864b88fe7a5b8447e07168b958bf7a8a271 Mon Sep 17 00:00:00 2001 From: Ricardo Boni Date: Wed, 2 Sep 2026 17:41:10 -0400 Subject: [PATCH 5/8] fix(ci): stop the smoke test's own query from counting as a survivor The "survivor" the last run reported was the PowerShell query itself. The match patterns appear literally in its own command line, so it matched itself -- and since each poll spawns a new one, the settle loop just kept finding the current query under a new pid. Which means the two runs that reported a leaked process were wrong, and tg delete's sweep has been clearing the whole tree all along: the "24 orphans" were 23 plus the query. The fix is the one the installer's own sweep already uses -- spare $PID, and the driver's pid with it, rather than trusting the match to exclude them. The API port is now a gated check rather than a note. It does come up; it just binds a couple of seconds after the UI, so the single immediate probe in the first run reported it closed. Polling was enough to tell those apart. Co-Authored-By: Claude Opus 5 (1M context) --- tests/e2e/smoke_exe.py | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/tests/e2e/smoke_exe.py b/tests/e2e/smoke_exe.py index 6edaa23..ace713c 100644 --- a/tests/e2e/smoke_exe.py +++ b/tests/e2e/smoke_exe.py @@ -150,11 +150,16 @@ def standalone_procs(): """ if WINDOWS: clause = " -or ".join(f"($cmd -match '{p}') -or ($exe -match '{p}')" for p in PROC_PATTERNS) + # Spare this query and the driver that spawned it. The patterns appear literally in + # the query's own command line, so it matches itself -- the same reason the + # installer's sweep spares $PID rather than trusting the match to exclude it. script = ( "$ErrorActionPreference = 'SilentlyContinue'; " + f"$spare = @({os.getpid()}) + $PID; " "Get-CimInstance Win32_Process | Where-Object { " "$cmd = ($_.CommandLine -replace '\\\\', '/'); " - "$exe = ($_.ExecutablePath -replace '\\\\', '/'); " + clause + " } | " + "$exe = ($_.ExecutablePath -replace '\\\\', '/'); " + "$spare -notcontains $_.ProcessId -and (" + clause + ") } | " 'ForEach-Object { "$($_.ProcessId)|$($_.CommandLine)" }' ) result = run(["powershell", "-NoProfile", "-NonInteractive", "-Command", script]) @@ -163,7 +168,7 @@ def standalone_procs(): # command line is read per pid with ps, which behaves the same on both. found = run(["pgrep", "-f", "|".join(PROC_PATTERNS)]) lines = [] - for pid in (p for p in found.stdout.split() if p.strip().isdigit()): + for pid in (p for p in found.stdout.split() if p.strip().isdigit() and int(p) != os.getpid()): cmdline = run(["ps", "-p", pid, "-o", "command="]).stdout.strip() lines.append(f"{pid}|{cmdline}") result = subprocess.CompletedProcess(args=(), returncode=0, stdout="\n".join(lines), stderr="") @@ -214,14 +219,10 @@ def check_running_install(installer, report): report.note(f"health endpoint {path}", body.strip()[:40]) break - # Reported, not gated. The installer advertises "API & MCP: http://localhost:" in - # the credentials it prints, so this should be listening -- but a first run found it - # closed, and whether it binds later than the UI or not at all is TestGen's answer to - # give. Polling tells the two apart; promote to a check once we know which. - if wait_for(lambda: port_open(API_PORT), timeout=60): - report.note(f"API port {API_PORT} accepts connections") - else: - report.note(f"API port {API_PORT} never opened", "installer advertises it in the credentials") + # Polled rather than probed once: the API binds a couple of seconds after the UI does, + # so a single immediate attempt reports it closed. The installer advertises it as + # "API & MCP: http://localhost:" in the credentials, so it has to come up. + report.check(wait_for(lambda: port_open(API_PORT), timeout=60), f"API answers on port {API_PORT}") postmaster = testgen_home() / "pgdata" / "postmaster.pid" report.check(postmaster.exists(), "embedded Postgres is running", str(postmaster)) From 18fd3e8eddf3aa8276bbceb630a6d256683614e3 Mon Sep 17 00:00:00 2001 From: Ricardo Boni Date: Wed, 2 Sep 2026 17:46:39 -0400 Subject: [PATCH 6/8] ci: drop the temporary smoke dry-run workflow Its job is done: run 33686488450 exercised the whole path on a real Windows runner -- install, 24 orphans from the dirty exit, sweep, delete, nothing left behind. release_exe.yml carries the same steps from here, and once this is on main its workflow_dispatch trigger makes a branch run possible without publishing. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/_temp_smoke_dryrun.yml | 46 ------------------------ 1 file changed, 46 deletions(-) delete mode 100644 .github/workflows/_temp_smoke_dryrun.yml diff --git a/.github/workflows/_temp_smoke_dryrun.yml b/.github/workflows/_temp_smoke_dryrun.yml deleted file mode 100644 index ec324c8..0000000 --- a/.github/workflows/_temp_smoke_dryrun.yml +++ /dev/null @@ -1,46 +0,0 @@ -# TEMPORARY -- delete before merging this branch. -# -# release_exe.yml runs only on a push to main, and its workflow_dispatch trigger is not -# usable until that file is on main, so there is no other way to exercise the smoke test -# from a branch. This runs the same steps, minus everything that publishes. -name: TEMP exe smoke dry run - -on: - pull_request: - branches: [ main ] - -jobs: - dry-run: - runs-on: windows-latest - timeout-minutes: 45 - defaults: - run: - shell: bash - steps: - - name: Checkout source code - uses: actions/checkout@v4 - - - name: Set up Python - uses: actions/setup-python@v5 - with: - cache: 'pip' - python-version: "3.9" - - - name: Install project dependencies - run: | - python -m pip install --upgrade pip - pip install .[dev,test] - - - name: Build the executable - run: pyinstaller --onefile --exclude demo dk-installer.py - - - name: Smoke-test the executable - run: python tests/e2e/smoke_exe.py dist/dk-installer.exe - - - name: Upload smoke-test logs - if: always() - uses: actions/upload-artifact@v4 - with: - name: smoke-test-logs - path: smoke/ - if-no-files-found: ignore From c896b6bdc63e296db682fa4cffcc8e604f9dc300 Mon Sep 17 00:00:00 2001 From: Ricardo Boni Date: Wed, 2 Sep 2026 17:47:15 -0400 Subject: [PATCH 7/8] docs(ci): correct the smoke test's cost in the workflow comments Measured 4m19s end to end on a runner, not the ~25 minutes I guessed: the install itself takes about two minutes there. The concurrency group still earns its place -- two merges close together can overlap -- and the 45 minute timeout stays as a backstop for a slow network day. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/release_exe.yml | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/.github/workflows/release_exe.yml b/.github/workflows/release_exe.yml index 2c1006d..ed9f4c1 100644 --- a/.github/workflows/release_exe.yml +++ b/.github/workflows/release_exe.yml @@ -9,10 +9,10 @@ on: # below is gated on a push to main. workflow_dispatch: -# The smoke test makes this job ~25 minutes instead of ~3, so two merges in quick -# succession can now overlap. Queue them rather than cancelling: the release step deletes -# the tag before recreating it, and a run cancelled inside that window would leave no -# release at all. +# The smoke test adds a real install to this job -- measured at 4m19s end to end, so two +# merges in quick succession can now overlap. Queue them rather than cancelling: the release +# step deletes the tag before recreating it, and a run cancelled inside that window would +# leave no release at all. concurrency: group: release-windows-build cancel-in-progress: false @@ -20,6 +20,8 @@ concurrency: jobs: windows-release: runs-on: windows-latest + # Generous against the measured 4m19s: the install pulls uv, a Python 3.13 and TestGen's + # dependency tree, so a bad network day is much slower than a good one. timeout-minutes: 45 defaults: run: From bee619bcacdbd6322c1e24f9307a06467176c6dc Mon Sep 17 00:00:00 2001 From: Ricardo Boni Date: Thu, 3 Sep 2026 12:14:57 -0400 Subject: [PATCH 8/8] ci(release): gate on the dirty exit actually orphaning the tree Orphaning is the point of killing the installer, so an empty tree is now a failure rather than a note. It also closes a hole: if the process query comes back empty for a bad reason -- PowerShell blocked, or the patterns drifting from what the installer's sweep matches -- then "nothing survived the delete" passes for the wrong reason, on the assertion this whole test exists for. The count is reported by process kind rather than as a list of pids, which are dead by the time anyone reads the log and say nothing about what was orphaned. Pids are still printed for survivors, where a specific process has to be chased. Its parsing is covered by unit tests, which caught that a pathlib basename on a Windows path is the whole path when it runs on POSIX; separators are normalised first, as the sweep does. Also drops chronology from the comments -- how a run once failed is the commit log's job, not the code's -- and a couple of measurements that were already out of date. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/release_exe.yml | 18 ++++---- tests/e2e/smoke_exe.py | 76 +++++++++++++++++-------------- tests/test_smoke_exe.py | 64 ++++++++++++++++++++++++++ tests/test_smoke_exe_args.py | 31 ------------- 4 files changed, 115 insertions(+), 74 deletions(-) create mode 100644 tests/test_smoke_exe.py delete mode 100644 tests/test_smoke_exe_args.py diff --git a/.github/workflows/release_exe.yml b/.github/workflows/release_exe.yml index ed9f4c1..abe7e11 100644 --- a/.github/workflows/release_exe.yml +++ b/.github/workflows/release_exe.yml @@ -9,10 +9,9 @@ on: # below is gated on a push to main. workflow_dispatch: -# The smoke test adds a real install to this job -- measured at 4m19s end to end, so two -# merges in quick succession can now overlap. Queue them rather than cancelling: the release -# step deletes the tag before recreating it, and a run cancelled inside that window would -# leave no release at all. +# The smoke test adds a real install, so two merges in quick succession can overlap. Queue +# them rather than cancelling: the release step deletes the tag before recreating it, and a +# run cancelled inside that window would leave no release at all. concurrency: group: release-windows-build cancel-in-progress: false @@ -20,8 +19,8 @@ concurrency: jobs: windows-release: runs-on: windows-latest - # Generous against the measured 4m19s: the install pulls uv, a Python 3.13 and TestGen's - # dependency tree, so a bad network day is much slower than a good one. + # The install pulls uv, a Python 3.13 and TestGen's dependency tree, so a bad network day + # is much slower than a good one. timeout-minutes: 45 defaults: run: @@ -44,10 +43,9 @@ jobs: - name: Build the executable run: pyinstaller --onefile --exclude demo dk-installer.py - # Installs TestGen for real in pip mode, checks the app serves, kills the installer - # and uninstalls. Deliberately ahead of the release step: the exe is the entry point - # most Windows users take, and this is the one place it can still be gated -- a - # failure here leaves the previous release in place instead of replacing it. + # Installs TestGen for real in pip mode, checks the app serves, kills the installer and + # uninstalls. Ahead of the release step so a failure leaves the previous release in + # place instead of replacing it with a broken build. - name: Smoke-test the executable run: python tests/e2e/smoke_exe.py dist/dk-installer.exe diff --git a/tests/e2e/smoke_exe.py b/tests/e2e/smoke_exe.py index ace713c..99090de 100644 --- a/tests/e2e/smoke_exe.py +++ b/tests/e2e/smoke_exe.py @@ -5,10 +5,10 @@ install``, the embedded Postgres, ``standalone-setup``, and the orphan sweep in ``tg delete``. All of that only fails for real -- unit tests reach it through mocks. -The installer is killed rather than interrupted, on purpose. A clean Ctrl+C never reaches +The installer is killed rather than interrupted. A clean Ctrl+C never reaches ``force_kill_app_tree``, so it does not exercise the sweep at all; only an orphaned tree -makes ``tg delete`` find the processes by command line, which is where the Windows bugs -were. Delivering a real console Ctrl+C from a CI step is a separate problem, left alone. +makes ``tg delete`` find the processes by command line. A console Ctrl+C cannot be delivered +from a CI step anyway. Readiness is taken from the install marker plus the UI port, never from the installer's stdout: redirected to a file that output is block-buffered, so the line announcing the app @@ -36,25 +36,21 @@ UI_PORT = 8501 API_PORT = 8530 -# The install pulls uv, a Python 3.13 and TestGen's whole dependency tree, then runs initdb. -# 4~8 minutes is what the installer itself promises; allow well past it before calling it a hang. +# 4~8 minutes is what the installer promises; allow well past it before calling it a hang. INSTALL_TIMEOUT = 20 * 60 -# Mirrors STANDALONE_PROC_PATTERNS in dk-installer.py, with separators already normalised. -# Duplicated rather than imported: the subject here is the built artifact, not the source -# sitting next to it. +# Mirrors STANDALONE_PROC_PATTERNS in dk-installer.py, separators already normalised. +# Duplicated rather than imported: the subject is the built artifact, not the source beside it. PROC_PATTERNS = ("testgen.*run-app", "tools/dataops-testgen") OUT_DIR = Path("smoke") -# Module level so a unit test can parse them against the installer's own parser. This -# script runs only on a merge to main, so a rename here (or of a flag it passes) would -# otherwise surface 25 minutes into a release build. +# Module level so a unit test can parse them against the installer's own parser: this script +# runs only on a merge to main, so a flag renamed here surfaces during a release build. INSTALL_ARGS = ("tg", "install", "--pip", "--no-demo") DELETE_ARGS = ("tg", "delete") - -# The installer prints the generated password once, so the captured stdout carries it. The -# instance is gone with the runner, but the artifact is downloadable from a public repo. +# The installer prints the generated password once, so the captured stdout carries it, and +# artifacts on a public repo are downloadable. PASSWORD_RE = re.compile(r"(Password:\s*)(\S+)") @@ -181,6 +177,23 @@ def standalone_procs(): return procs +def summarize(procs): + """``9 postgres, 13 python, 2 testgen`` -- what is running, without a wall of pids.""" + kinds = {} + for cmdline in procs.values(): + text = cmdline.strip() + if text.startswith('"'): + executable = text[1:].split('"', 1)[0] + else: + executable = text.split(" ", 1)[0] + # Separators normalised before splitting: a pathlib basename on a Windows path is + # the whole path when this runs on POSIX, which the sweep in dk-installer.py handles + # the same way. + name = executable.replace("\\", "/").rsplit("/", 1)[-1].lower().removesuffix(".exe") + kinds[name or "unknown"] = kinds.get(name or "unknown", 0) + 1 + return ", ".join(f"{count} {name}" for name, count in sorted(kinds.items(), key=lambda kv: -kv[1])) + + def wait_for(predicate, timeout): """Poll until the predicate is true, returning whether it became true in time.""" deadline = time.time() + timeout @@ -219,9 +232,8 @@ def check_running_install(installer, report): report.note(f"health endpoint {path}", body.strip()[:40]) break - # Polled rather than probed once: the API binds a couple of seconds after the UI does, - # so a single immediate attempt reports it closed. The installer advertises it as - # "API & MCP: http://localhost:" in the credentials, so it has to come up. + # Polled: the API binds a couple of seconds after the UI. It has to come up -- the + # installer advertises it as "API & MCP: http://localhost:" in the credentials. report.check(wait_for(lambda: port_open(API_PORT), timeout=60), f"API answers on port {API_PORT}") postmaster = testgen_home() / "pgdata" / "postmaster.pid" @@ -241,9 +253,9 @@ def check_running_install(installer, report): listed = run([uv_path, "tool", "list"]).stdout if uv_path else "" report.check("dataops-testgen" in listed, "uv reports the tool installed", listed.splitlines()[0] if listed else "") - # Which major version a new cluster initialized on. Reported because it answers TG-1245: - # pgserver 0.6.0's PostgreSQL 18 build cannot start on a Windows box without - # libwinpthread-1.dll, which the wheel does not ship. + # Which major version a new cluster initialized on (TG-1245): pgserver's PostgreSQL 18 + # build cannot start without libwinpthread-1.dll, which the wheel does not ship, so a + # runner that happens to supply it masks a failure a user would hit. report.note("embedded Postgres version", (read_text_safe(testgen_home() / "pgdata" / "PG_VERSION") or "?").strip()) @@ -276,11 +288,10 @@ def kill_installer(proc): def save_and_scan_app_log(report): """Copy TestGen's log aside, then look for a traceback in it. - Best-effort, and on Windows usually unreadable: the app tree we deliberately orphaned is - what holds the file open, so killing the installer does not release it, and ``tg delete`` - then takes the whole directory. Reported rather than failed either way -- the UI and - Postgres checks are the gate, and a traceback that breaks neither should not block a - release. + Best-effort, and on Windows usually unreadable: the orphaned app tree holds the file + open, so killing the installer does not release it, and ``tg delete`` then takes the + whole directory. Reported rather than failed either way -- the UI and Postgres checks are + the gate, and a traceback that breaks neither should not block a release. """ app_log = testgen_home() / "logs" / "app.log" text = read_text_safe(app_log) @@ -317,8 +328,8 @@ def check_delete(installer, output, tool_paths, report): claimed = "TestGen uninstalled." in output if survived: - # The bug this guards against: a success message printed while the tool environment - # and shim were still on disk, because a live process held them open. + # A live process holding the tool environment open makes the uninstall a no-op, and + # the message must not outrun what is actually gone. report.check(not claimed, "delete did not claim a success it cannot back up") else: report.check(claimed, "delete reported success") @@ -384,8 +395,7 @@ def main(): stdout=log_file, stderr=subprocess.STDOUT, stdin=subprocess.DEVNULL, - # Unbuffered so the log is useful while the run is still going; readiness does - # not depend on it either way. + # Unbuffered so the log is readable mid-run; readiness does not depend on it. env={**os.environ, "PYTHONUNBUFFERED": "1"}, ) running = wait_until_running(proc, installer, report) @@ -408,10 +418,10 @@ def main(): kill_installer(proc) save_and_scan_app_log(report) orphans = standalone_procs() - if orphans: - report.note(f"{len(orphans)} orphans left by the dirty exit", str(sorted(orphans))) - else: - report.note("nothing survived the kill", "the sweep has nothing to prove in this run") + # Orphaning is the point, so an empty tree is a failure: either the app no longer outlives + # the installer, or this query cannot see it -- and then "nothing survived the delete" + # below would pass for the wrong reason. + report.check(bool(orphans), f"the dirty exit orphaned the app tree ({len(orphans)})", summarize(orphans)) step("delete") deleted = run([*command, *DELETE_ARGS]) diff --git a/tests/test_smoke_exe.py b/tests/test_smoke_exe.py new file mode 100644 index 0000000..90530e7 --- /dev/null +++ b/tests/test_smoke_exe.py @@ -0,0 +1,64 @@ +"""The exe smoke test only runs on a merge to main, so anything it can get wrong on its own +is worth pinning here: the argv it drives the installer with, and the parsing it does of +Windows command lines. +""" + +import pytest + +from .e2e.smoke_exe import DELETE_ARGS, INSTALL_ARGS, summarize +from .installer import get_installer_instance + + +@pytest.mark.unit +def test_smoke_install_args_are_accepted(): + args = get_installer_instance().parser.parse_args(list(INSTALL_ARGS)) + + assert args.prod == "tg" + assert args.install_mode == "pip" + # The demo step is skipped on purpose: it is `required = False`, so it could not gate the + # release anyway, and it is the longest part of the install. + assert args.generate_demo is False + + +@pytest.mark.unit +def test_smoke_delete_args_are_accepted(): + args = get_installer_instance().parser.parse_args(list(DELETE_ARGS)) + + assert args.prod == "tg" + # Nothing is kept: the smoke test asserts the uninstall left nothing behind. + assert args.keep_data is False + assert args.keep_images is False + + +@pytest.mark.unit +def test_summarize_counts_a_standalone_tree(): + """Shapes taken from a real install: postgres quotes its path, the python children do + not, and the shim sits in uv's bin dir rather than the tool environment.""" + procs = { + 1: '"C:\\Users\\r\\AppData\\Roaming\\uv\\tools\\dataops-testgen\\Scripts\\postgres.exe" -D C:/pgdata', + 2: '"C:\\Users\\r\\AppData\\Roaming\\uv\\tools\\dataops-testgen\\Scripts\\postgres.exe" -c config', + 3: "C:/Users/r/AppData/Roaming/uv/tools/dataops-testgen/Scripts/python.exe -m streamlit run app.py", + 4: "C:/Users/r/AppData/Roaming/uv/tools/dataops-testgen/Scripts/python.exe -m testgen run-app", + 5: "C:/Users/r/AppData/Roaming/uv/bin/testgen.exe run-app", + } + + # Ordered by count, so the dominant process kind reads first. + assert summarize(procs) == "2 postgres, 2 python, 1 testgen" + + +@pytest.mark.unit +def test_summarize_survives_a_missing_command_line(): + """Win32_Process leaves CommandLine empty for a process the query cannot read, and a + crash here would fail a release build for a cosmetic reason.""" + assert summarize({1: "", 2: " "}) == "2 unknown" + assert summarize({}) == "" + + +@pytest.mark.unit +def test_summarize_handles_posix_paths(): + procs = { + 1: "/home/t/.local/share/uv/tools/dataops-testgen/bin/python -m streamlit run app.py", + 2: "/home/t/.local/bin/testgen run-app", + } + + assert summarize(procs) == "1 python, 1 testgen" diff --git a/tests/test_smoke_exe_args.py b/tests/test_smoke_exe_args.py deleted file mode 100644 index fc4087b..0000000 --- a/tests/test_smoke_exe_args.py +++ /dev/null @@ -1,31 +0,0 @@ -"""The exe smoke test drives the installer through its CLI, and only runs on a merge to -main. Parsing its argv against the real parser here turns a 25-minute round trip into a -2-second one -- the first run of that job failed because ``--no-analytics`` is a top-level -flag that cannot follow the product name. -""" - -import pytest - -from .e2e.smoke_exe import DELETE_ARGS, INSTALL_ARGS -from .installer import get_installer_instance - - -@pytest.mark.unit -def test_smoke_install_args_are_accepted(): - args = get_installer_instance().parser.parse_args(list(INSTALL_ARGS)) - - assert args.prod == "tg" - assert args.install_mode == "pip" - # The demo step is skipped on purpose: it is `required = False`, so it could not gate - # the release anyway, and it is the longest part of the install. - assert args.generate_demo is False - - -@pytest.mark.unit -def test_smoke_delete_args_are_accepted(): - args = get_installer_instance().parser.parse_args(list(DELETE_ARGS)) - - assert args.prod == "tg" - # Nothing is kept: the smoke test asserts the uninstall left nothing behind. - assert args.keep_data is False - assert args.keep_images is False