ci(release): smoke-test the built exe before publishing it - #96
Merged
Conversation
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) <noreply@anthropic.com>
Coverage Report •
|
… 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) <noreply@anthropic.com>
… 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) <noreply@anthropic.com>
…ssword 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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
Brings in the Windows PR job from #95, so this branch's checks exercise tests/test_smoke_exe_args.py on Windows too.
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) <noreply@anthropic.com>
rboni-dk
marked this pull request as ready for review
September 3, 2026 16:21
aarthy-dk
approved these changes
Sep 3, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Main is production for
dk-installer.pythe moment a merge lands — we point people at the file directly — so nothing in CI can prevent a bad.py, only detect one. The.exeis different: it is published after CI has had its say, and it is the entry point most Windows users take. So it is the one artifact still worth gating, and the smoke test sits betweenpyinstallerand the release step, where a failure leaves the previous release in place instead of replacing it.tests/e2e/smoke_exe.pyinstalls TestGen for real in pip mode and checks the app is actually serving: UI on 8501, API on 8530,postmaster.pidpresent, the marker recording pip mode,uv tool listshowing the package. It then kills the installer rather than interrupting it — a clean Ctrl+C never reachesforce_kill_app_tree, so it would not exercise the sweep at all; only an orphaned tree makestg deletefind the processes by command line.The uninstall assertion is a pairing rather than a message: if the tool environment, shim, data directory or any process survived, printing
TestGen uninstalled.is itself a failure. That is the shape of the defects found by hand in August.Verified on run 33686488450 — 24 orphans from the dirty exit, none surviving the delete, 4m19s end to end. So the release job grows by about one install.
Not covered
pgdata/PG_VERSIONreports 18, so new clusters still initialize on PostgreSQL 18 and only start on the runner because Git for Windows supplieslibwinpthread-1.dll(TG-1245). A green run here is not evidence that a clean Windows box can install — the version is reported on every run so the masking stays visible, but this needs the TestGen-side fix. Demo data is skipped:TestgenQuickStartStepisrequired = False, so it could not gate anything either way.Notes for review
Readiness comes from the install marker plus the UI port, not 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. The API port is polled rather than probed once, since it binds a couple of seconds after the UI. Both logs are redacted before being kept: the captured stdout carries the password the installer generates, and artifacts on a public repo are downloadable. The script refuses to run outside CI without
--force, since it kills every TestGen process on the machine and removes~/.testgen.release_exe.ymlbecomes explicit steps:python-job.ymltakes a singlerunstring, which cannot carry anifon the release or upload logs on failure. Publishing is gated on a push to main, soworkflow_dispatchbuilds and smokes a branch without touching thelatesttag. A concurrency group keeps two quick merges from racing to move that tag, withcancel-in-progress: false— the release step deletes the tag before recreating it.🤖 Generated with Claude Code