diff --git a/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-03-cmd-mxcli-tests-write-real-mxcli-logs.json b/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-03-cmd-mxcli-tests-write-real-mxcli-logs.json new file mode 100644 index 0000000000..91e66be0c7 --- /dev/null +++ b/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-03-cmd-mxcli-tests-write-real-mxcli-logs.json @@ -0,0 +1 @@ +{"area":"cmd/mxcli","date":"2026-10-03","symptom":"`go test ./cmd/mxcli/` on any OS appended session_start/session_end lines to the developer's real ~/.mxcli/logs/mxcli-.log, and could delete their older log files","cause":"in-process command tests (e.g. runCheckFiles in TestCheck_TestFileResolvesTheRunnersModule) reach newLoggedExecutorTo -> diaglog.Init, which writes to logDirectory() (MXCLI_LOG_DIR, else ~/.mxcli/logs) and runs cleanOldLogs on it. The MXCLI_LOG_DIR hook existed but only the two subprocess tests set it, and the HOME-only overrides in other tests do not move the log on Windows (os.UserHomeDir reads USERPROFILE)","file":"`cmd/mxcli/testmain_test.go` (TestMain); `mdl/diaglog/diaglog.go` (logDirectory, cleanOldLogs)","insight":"A package-level TestMain that points MXCLI_LOG_DIR at a temp dir (unless already set) is the one place that covers every present and future in-process test; per-test Setenv only protects the tests whose author remembered. cleanOldLogs makes the leak destructive, not just noisy. Prove it with a sentinel home: run go test with HOME, USERPROFILE, APPDATA and LOCALAPPDATA all pointed at a fresh temp dir and find it for .mxcli/logs before and after (before: one mxcli-.log with one session in check mode, because diaglog is one session per process; after: nothing). Setting HOME alone proves nothing on Windows","refs":["mendixlabs/mxcli#1273","mendixlabs/mxcli#1256"]} diff --git a/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-03-windows-handshake-signal0-reads-live-pid-as-dead.json b/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-03-windows-handshake-signal0-reads-live-pid-as-dead.json new file mode 100644 index 0000000000..68369d14aa --- /dev/null +++ b/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-03-windows-handshake-signal0-reads-live-pid-as-dead.json @@ -0,0 +1 @@ +{"area": "cmd/mxcli", "date": "2026-10-03", "symptom": "mendixlabs/mxcli#1284: on Windows `mxcli test --attach` always says the app that published `.mxcli/test-endpoint.json` \"is no longer running\" although the pid is alive; the dev loop's `run-local.json` \"already serving\" detection never fires either", "cause": "pid liveness was `os.FindProcess(pid)` + `Signal(0)`; Go supports no signal but Kill on Windows and returns EWINDOWS for the rest, so every pid read dead", "file": "`internal/procalive/` (Alive, build-tagged unix/windows); callers `cmd/mxcli/testrunner/handshake.go`, `cmd/mxcli/devloop_handshake.go`, `cmd/mxcli/docker/procgroup_windows.go`", "insight": "The correct Windows check (OpenProcess(SYNCHRONIZE) + WaitForSingleObject(h, 0) == WAIT_TIMEOUT) already existed in docker/procgroup_windows.go since the waitReady incident, but two pid-based copies of the Signal(0) idiom were written separately and never moved over; Linux-only CI could not see it. Four tests were already red on a native Windows run (TestHandshakeRoundTrip, TestAttachUsesTheAdminPasswordNotTheEndpointToken, TestDevLoopHandshake_RoundTrip, TestWarnIfDevLoopServing_FiresForALiveDevLoop). Grep for `Signal(syscall.Signal(0))` / `Signal(0)` before trusting any new liveness check; use procalive.Alive for a bare pid.", "refs": ["mendixlabs/mxcli#1284", "#897"]} diff --git a/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-09-windows-run-stop-exited-run-still-alive.json b/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-09-windows-run-stop-exited-run-still-alive.json new file mode 100644 index 0000000000..6d0c903216 --- /dev/null +++ b/.claude/skills/fix-issue/findings/cmd-mxcli/2026-10-09-windows-run-stop-exited-run-still-alive.json @@ -0,0 +1 @@ +{"area": "cmd/mxcli", "date": "2026-10-09", "symptom": "Windows: `mxcli run stop -p App.mpr` prints \"failed: 1 process(es) of pid N's run are still alive: [N]\" (exit 1) for a run that is shutting down or gone; a few seconds later the pid no longer exists and no ports are held", "cause": "docker.PidAlive (session_other.go, used by run stop / run status) treated \"os.FindProcess can open the pid\" as alive on Windows. Windows keeps an exited process object, and so an openable pid, for as long as anyone holds a handle to it (the parent that started it, typically), so a terminated run read as alive. It arrived after the #1284 handshake fix was written, so procalive did not cover it", "fix": "PidAlive delegates to internal/procalive.Alive: OpenProcess(SYNCHRONIZE) + WaitForSingleObject(h, 0) == WAIT_TIMEOUT on Windows, the same Signal(0) as before elsewhere off Linux; Linux keeps its /proc PidAlive", "insight": "\"Can I open it\" is not \"is it running\" on Windows: an exited process is openable until the last handle closes. The regression test holds its own SYNCHRONIZE handle, kills the child, waits for WAIT_OBJECT_0, then asks; the old PidAlive says alive. When porting a liveness fix onto a newer main, grep for every pid-liveness helper (processAlive, PidAlive, Signal(0)), not just the ones the original PR named. E2E on Windows 11 / Mendix 11.13.0: `run stop` went from \"failed: … still alive\" on every stop to \"stopped: pid N in 400ms\"", "file": "cmd/mxcli/docker/session_other.go (PidAlive)", "test": "cmd/mxcli/docker/session_windows_test.go (TestPidAlive_ExitedProcessWithAnOpenHandleIsNotAlive)", "refs": ["mendixlabs/mxcli#1284"]} diff --git a/.claude/skills/fix-issue/findings/mdl-executor/2026-10-09-submenu-keeps-same-caption-page-item-action.json b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-09-submenu-keeps-same-caption-page-item-action.json new file mode 100644 index 0000000000..904f4b6d35 --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-09-submenu-keeps-same-caption-page-item-action.json @@ -0,0 +1 @@ +{"area":"mdl/executor","date":"2026-10-09","symptom":"`create or replace navigation` turning page item 'Work' into a sub-menu 'Work' reports success (plus `kept the stored action of menu item 'Work' (Forms$FormAction): MDL cannot express it`), then `mx check` fails with CE0548 \"Items with subitems cannot have an action themselves.\" (v0.25.0 regression; a different sub-menu caption avoids it)","cause":"`keepStoredMenuAction` had a sub-menu branch that kept ANY stored non-NoAction, on the reasoning that a sub-menu \"takes no OnClick, so a stored action on one is never stated\". Pairing is by caption only, so the stored page item's action was carried onto the new sub-menu. Navigation profiles and menu documents share the decision","file":"`mdl/executor/cmd_navigation.go` (`keepStoredMenuAction`)","insight":"**A keep-what-MDL-cannot-say rule must only keep what the target element can legally hold.** \"The script states no action\" is not \"the script wants the stored one\" when the element's kind changed under the same caption — a sub-menu can hold no action at all, so the carry is never right there. The tell in the output was the false reason: `Forms$FormAction` is printable MDL, yet the message said MDL cannot express it. Guards `TestNavigationRewrite_SubMenuDoesNotKeepAStoredAction` / `TestCreateMenu_SubMenuDoesNotKeepAStoredAction` (control: a leaf still keeps an unprintable action, #980). Measured on 11.14.0 with `mdl-examples/bug-tests/1341-navigation-submenu-keeps-page-action.mdl`: unfixed binary -> docker check CE0548; fixed -> 0 errors","refs":["mendixlabs/mxcli#1341","ako/mxcli#980"],"ce":["CE0548"]} diff --git a/.claude/skills/fix-issue/findings/other/2026-10-02-windows-tests-home-setenv-leaks-real-mxcli.json b/.claude/skills/fix-issue/findings/other/2026-10-02-windows-tests-home-setenv-leaks-real-mxcli.json new file mode 100644 index 0000000000..4152f051dc --- /dev/null +++ b/.claude/skills/fix-issue/findings/other/2026-10-02-windows-tests-home-setenv-leaks-real-mxcli.json @@ -0,0 +1 @@ +{"area":"internal/testutil","date":"2026-10-02","symptom":"On Windows, `go test` of cmd/mxcli, cmd/mxcli/docker, cmd/mxcli/marketplace, internal/auth and mdl/executor wrote into the developer's REAL `%USERPROFILE%\\.mxcli` (a fixture PAT in `auth.json`, fake `mxbuild/99.99.98`, `mxbuild/99.99.99`, `runtime/99.99.99`, `marketplace-catalog-default.json`), and `TestAuthList_EmptyStore`, `TestResolve_NoCredential`, `TestClientFor_NoCredential` failed because an earlier run had left a credential behind","cause":"Thirteen test files isolated the home directory with `t.Setenv(\"HOME\", dir)`. `os.UserHomeDir()` reads `USERPROFILE` on Windows and `HOME` everywhere else, so on Windows the override was ignored and the tests used the real profile; the leaked state persisted between runs, which made the failures order- and history-dependent","file":"`internal/testutil/home.go` (`SetHome` sets HOME and USERPROFILE); `internal/testutil/guard_test.go` (`TestNoBareHomeSetenv`)","insight":"It went unnoticed because CI runs the tests on Linux, where HOME is what UserHomeDir reads, and the Windows CI job is `-run`-scoped (#897) so it never executes these tests. Two files (`mdl/diaglog`, `cmd/mxcli/docker/download_test.go`) had already been fixed locally with a per-OS helper, but a copy-local fix teaches the next test author nothing: the repair is one shared helper plus a guard that fails on a bare `Setenv(\"HOME\"`. To prove a home-isolation fix, run the suite with USERPROFILE and HOME pointing at an empty sentinel directory and list `.mxcli` in it afterwards (before the fix: auth.json, mxbuild, runtime, catalog cache; after: only `logs/`, written by `TestCheck_TestFileResolvesTheRunnersModule`, which never sets a home at all). `diaglog.logDirectory` also honours `MXCLI_LOG_DIR`, the other lever for tests that execute cobra commands","refs":["mendixlabs/mxcli#1256","#897"]} diff --git a/CHANGELOG.md b/CHANGELOG.md index 4303a3dcac..68988b1d9a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,8 +28,12 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Fixed +- **A navigation sub-menu no longer keeps the action of the page item it replaces** (mendixlabs/mxcli#1341) — turning menu item `'Work'` into sub-menu `'Work'` with `create or replace navigation` (or `create or modify menu`) paired the two by caption and carried the page item's `Forms$FormAction` onto the sub-menu, reporting it as an action "MDL cannot express"; `mx check` then failed with CE0548 "Items with subitems cannot have an action themselves." A sub-menu is now always written with no action (measured on 11.14.0). +- **`mxcli test --attach` and the dev loop's "already serving" detection see a live app on Windows** (mendixlabs/mxcli#1284) — both read a pid from a handshake file (`.mxcli/test-endpoint.json`, `.mxcli/run-local.json`) and tested it with `Signal(0)`, which Go supports only as Kill on Windows, so every pid read as dead: `test --attach` always refused with "the app that published … is no longer running" while the app was up, and a command that looks for a dev loop already serving the project (the recompile warning, for one) never found it. Liveness is now `internal/procalive.Alive`, which uses OpenProcess and WaitForSingleObject on Windows (the check `run --local` already used for mxbuild) and `Signal(0)` elsewhere. No change on Linux or macOS. +- **`run stop` on Windows no longer reports a run that has shut down as still alive** — it printed "failed: 1 process(es) of pid N's run are still alive" and exited 1 for a run that had stopped, because its liveness check counted any pid Windows could still open as alive, and an exited process stays openable while its parent holds a handle to it. It now uses the same check as `test --attach` (above), and reports "stopped: pid N". - **`retrieve $u from $obj/System.owner` passed check and exec, then failed the build with CE0136** (mendixlabs/mxcli#1358) — `owner` and `changedBy` are entity flags, not modelled associations: mxbuild resolves the name in a retrieve by association but derives no entity from it, `[CE0136] "Retrieve object must specify the 'Entity' property."`, with or without `owner: AutoOwner` (measured on 11.12.2, `System.changedBy` alike). `check` and `exec` now refuse it as **MDL-RETRIEVE02** and name the form that builds: `retrieve $u from System.User where [id = $obj/System.owner] first;`. - **`run --local` and `test --local` work on Windows without Developer Mode or admin rights** (mendixlabs/mxcli#1286) — the first run for a new Mendix version failed with `linking runtime into mxbuild cache: symlink …: A required privilege is not held by the client.` mxcli links the downloaded runtime into the mxbuild cache, and an ordinary Windows user may create a symbolic link only with Developer Mode or an elevated token. When Windows refuses the symlink, mxcli now makes a directory junction instead, which needs no privilege (the same link `mklink /J` makes; it needs `cmd.exe`, which every Windows has). The other link site, the PAD files `docker build` links into the mxbuild cache, gets the same fallback. A cache that was already linked by hand with `New-Item -ItemType Junction` is left as it is. +- **`run --local --watch` applies a domain model change on Windows** (mendixlabs/mxcli#1342) — page and microflow edits hot-reloaded, but adding an entity or an attribute failed every rebuild with "The process cannot access the file '…\deployment\web' because it is being used by another process", and the app then served 404 until `run --local` was restarted. A domain model change makes mxbuild recreate `deployment/web`, and the incremental web client bundler runs inside it, which Windows will not let anyone delete. On that failure the watch loop now stops the bundler, builds again, and starts a fresh bundler on the new `web/`; page edits keep the incremental path. Applies to Mendix versions whose deployment has `web/rollup.config.mjs` (reported on 11.12.4, reproduced on 11.13.0); 11.12.6 and 11.14+ bundle in mxbuild and were never affected. - **`run stop` and `run status` find a detached run's processes on every machine** — the guard against a reused session id rebuilt each process's start time from the kernel's boot time, which is whole seconds, and allowed only one second of slack. On a machine that booted late in a second, a run's own processes read as older than the run and were skipped, so `run stop` could report "was not running" for a live run. The slack is now two seconds. This was also the intermittent `TestRunStop_*` / `TestSessionMembers_*` failure on CI. - **The catalog names a call's and a delete's target** (mendixlabs/mxcli#1305) — `activities_for()` and `CATALOG.ACTIVITIES` returned every microflow, nanoflow, Java action and JavaScript action call with `action_ref=""`, and every delete with `entity_ref=""`, although `refs_from()` had both targets; a loop-scoped lint rule could not follow a call out of the loop. `action_ref` / `ActionRef` is now the called document, `entity_ref` / `EntityRef` a delete's entity (when the flow types the variable: a parameter, a create or retrieve output, a loop iterator), and the new `queue_ref` / `QueueRef` the task queue a microflow or Java action call runs in, so a rule can skip a call that runs asynchronously. A nanoflow's JavaScript action call now has a `refs_from()` `call` row (`target_type` `"JAVASCRIPT_ACTION"`), so `show callers` sees it. The catalog schema is bumped to 23; a cached catalog rebuilds. - **A DataGrid 2 column's `Visible:` expression is written** — `Visible: $showPrices`, `Visible: if … then … else …`, `visible: not(…)` and `Visible: [cond]` on a column passed `check` and `exec` and were stored as `true`, so the column was always visible; only the old quoted `Visible: ''` was kept. `alter page … set (Visible: ) on grid column(…)` was refused as "column property VisibleIf not found", and an inserted column dropped `Visible: false` too. `describe` now prints the bare expression. A column's visibility is evaluated once for the grid, with no row object, so `$currentObject` there is CE0117 at build; `check` refuses it as **MDL-WIDGET43** (measured on 11.14.0). Use a page variable or parameter. Projects regenerate their widget definitions (generator version 18). diff --git a/cmd/mxcli/cmd_auth_test.go b/cmd/mxcli/cmd_auth_test.go index 995d90b457..7d1c7a5b05 100644 --- a/cmd/mxcli/cmd_auth_test.go +++ b/cmd/mxcli/cmd_auth_test.go @@ -15,6 +15,7 @@ import ( "testing" "github.com/mendixlabs/mxcli/internal/auth" + "github.com/mendixlabs/mxcli/internal/testutil" "github.com/spf13/cobra" "github.com/spf13/pflag" ) @@ -24,7 +25,7 @@ import ( func withTestHome(t *testing.T) string { t.Helper() home := t.TempDir() - t.Setenv("HOME", home) + testutil.SetHome(t, home) t.Setenv(auth.EnvPAT, "") t.Setenv(auth.EnvProfile, "") return home diff --git a/cmd/mxcli/cmd_marketplace_install_file_test.go b/cmd/mxcli/cmd_marketplace_install_file_test.go index ff2a4382a7..c3a77e2c59 100644 --- a/cmd/mxcli/cmd_marketplace_install_file_test.go +++ b/cmd/mxcli/cmd_marketplace_install_file_test.go @@ -12,6 +12,7 @@ import ( "testing" "github.com/mendixlabs/mxcli/internal/marketplace" + "github.com/mendixlabs/mxcli/internal/testutil" "github.com/spf13/cobra" ) @@ -48,7 +49,7 @@ func buildLocalMPK(t *testing.T, name string, entries map[string]string) string // placeholder .mpr — the widget path only checks that the project exists. func runInstallFile(t *testing.T, args ...string) (string, error) { t.Helper() - t.Setenv("HOME", t.TempDir()) + testutil.SetHome(t, t.TempDir()) origFactory := marketplaceClientFactory marketplaceClientFactory = func(_ context.Context, _ *cobra.Command) (*marketplace.Client, error) { @@ -183,7 +184,7 @@ func TestInstallFile_NoContentIDAndNoFile(t *testing.T) { // and still reaches the client factory (which this helper makes fatal). func TestInstallFile_ContentIDStillUsesTheClient(t *testing.T) { mpr := placeholderProject(t) - t.Setenv("HOME", t.TempDir()) + testutil.SetHome(t, t.TempDir()) called := false origFactory := marketplaceClientFactory marketplaceClientFactory = func(_ context.Context, _ *cobra.Command) (*marketplace.Client, error) { diff --git a/cmd/mxcli/cmd_marketplace_test.go b/cmd/mxcli/cmd_marketplace_test.go index 641d380d08..9388fa10c1 100644 --- a/cmd/mxcli/cmd_marketplace_test.go +++ b/cmd/mxcli/cmd_marketplace_test.go @@ -12,6 +12,7 @@ import ( "testing" "github.com/mendixlabs/mxcli/internal/marketplace" + "github.com/mendixlabs/mxcli/internal/testutil" "github.com/spf13/cobra" "github.com/spf13/pflag" ) @@ -37,7 +38,7 @@ func runMarketplace(t *testing.T, handler http.HandlerFunc, args ...string) (str t.Helper() // Isolate the catalog cache (~/.mxcli/...) into a temp HOME so tests never // read or write the real user cache. - t.Setenv("HOME", t.TempDir()) + testutil.SetHome(t, t.TempDir()) ts := httptest.NewServer(handler) t.Cleanup(ts.Close) diff --git a/cmd/mxcli/devloop_handshake.go b/cmd/mxcli/devloop_handshake.go index 3a7e40718f..d3b0e4ac59 100644 --- a/cmd/mxcli/devloop_handshake.go +++ b/cmd/mxcli/devloop_handshake.go @@ -7,8 +7,9 @@ import ( "fmt" "os" "path/filepath" - "syscall" "time" + + "github.com/mendixlabs/mxcli/internal/procalive" ) // devLoopHandshakeName is what `mxcli run --local` publishes so another mxcli @@ -95,22 +96,9 @@ func readDevLoopHandshake(projectPath string) (devLoopHandshake, error) { if err := json.Unmarshal(body, &h); err != nil { return h, fmt.Errorf("%s is not valid JSON: %w", path, err) } - if !processAlive(h.PID) { + if !procalive.Alive(h.PID) { return h, fmt.Errorf("%s refers to process %d, which is no longer running\n"+ " (the dev loop was stopped without cleaning up; start a new one)", path, h.PID) } return h, nil } - -// processAlive reports whether a pid exists. Signal 0 is delivered to no one but -// still performs the existence and permission checks. -func processAlive(pid int) bool { - if pid <= 0 { - return false - } - p, err := os.FindProcess(pid) - if err != nil { - return false - } - return p.Signal(syscall.Signal(0)) == nil -} diff --git a/cmd/mxcli/docker/check_test.go b/cmd/mxcli/docker/check_test.go index b1ee6d7163..5ef6fc560e 100644 --- a/cmd/mxcli/docker/check_test.go +++ b/cmd/mxcli/docker/check_test.go @@ -8,6 +8,8 @@ import ( "path/filepath" "runtime" "testing" + + "github.com/mendixlabs/mxcli/internal/testutil" ) func TestCheck_SkipUpdateWidgets(t *testing.T) { @@ -275,7 +277,7 @@ func TestCheck_UpdateWidgetsReceivesAbsolutePath(t *testing.T) { func TestResolveMxForVersion_PrefersExactCachedVersion(t *testing.T) { dir := t.TempDir() - setTestHomeDir(t, dir) + testutil.SetHome(t, dir) setTestApplicationsDir(t, t.TempDir()) // prevent real macOS Studio Pro from matching // Point PATH at an empty temp dir (rather than clearing it) so exec.LookPath // still works for any other testing infrastructure but can't find mx. diff --git a/cmd/mxcli/docker/detect_test.go b/cmd/mxcli/docker/detect_test.go index 79e89034fb..272bb9835a 100644 --- a/cmd/mxcli/docker/detect_test.go +++ b/cmd/mxcli/docker/detect_test.go @@ -7,6 +7,8 @@ import ( "path/filepath" "runtime" "testing" + + "github.com/mendixlabs/mxcli/internal/testutil" ) func TestResolveMxBuild_ExplicitPath(t *testing.T) { @@ -285,7 +287,7 @@ func TestResolveMxBuild_PrefersStudioProOverCache(t *testing.T) { func TestResolveMxBuild_PrefersExactCachedVersion(t *testing.T) { dir := t.TempDir() - setTestHomeDir(t, dir) + testutil.SetHome(t, dir) setTestApplicationsDir(t, t.TempDir()) // prevent real macOS Studio Pro from matching // Point PATH at an empty temp dir (rather than clearing it) so exec.LookPath // still works for any other testing infrastructure but can't find mxbuild. diff --git a/cmd/mxcli/docker/download_test.go b/cmd/mxcli/docker/download_test.go index 5f539f08f7..33b65f7dd5 100644 --- a/cmd/mxcli/docker/download_test.go +++ b/cmd/mxcli/docker/download_test.go @@ -7,16 +7,9 @@ import ( "path/filepath" "runtime" "testing" -) -func setTestHomeDir(t *testing.T, dir string) { - t.Helper() - if runtime.GOOS == "windows" { - t.Setenv("USERPROFILE", dir) - } else { - t.Setenv("HOME", dir) - } -} + "github.com/mendixlabs/mxcli/internal/testutil" +) // setTestApplicationsDir redirects the macOS Applications directory scan to dir // so tests that set up cached binaries are not confused by real Studio Pro installs. @@ -66,7 +59,7 @@ func TestCachedMxBuildPath_NotCached(t *testing.T) { func TestCachedMxBuildPath_Cached(t *testing.T) { // Create a fake cached mxbuild dir := t.TempDir() - setTestHomeDir(t, dir) + testutil.SetHome(t, dir) version := "99.99.99" modelerDir := filepath.Join(dir, ".mxcli", "mxbuild", version, "modeler") @@ -83,7 +76,7 @@ func TestCachedMxBuildPath_Cached(t *testing.T) { func TestAnyCachedMxBuildPath_Empty(t *testing.T) { dir := t.TempDir() - setTestHomeDir(t, dir) + testutil.SetHome(t, dir) path := AnyCachedMxBuildPath() if path != "" { @@ -93,7 +86,7 @@ func TestAnyCachedMxBuildPath_Empty(t *testing.T) { func TestAnyCachedMxBuildPath_Found(t *testing.T) { dir := t.TempDir() - setTestHomeDir(t, dir) + testutil.SetHome(t, dir) modelerDir := filepath.Join(dir, ".mxcli", "mxbuild", "11.6.3", "modeler") os.MkdirAll(modelerDir, 0755) @@ -108,7 +101,7 @@ func TestAnyCachedMxBuildPath_Found(t *testing.T) { func TestAnyCachedMxBuildPath_PicksNewestNumericVersion(t *testing.T) { dir := t.TempDir() - setTestHomeDir(t, dir) + testutil.SetHome(t, dir) versions := []string{"9.24.40.80973", "11.6.3", "11.9.0"} var newest string @@ -161,7 +154,7 @@ func TestCachedRuntimePath_NotCached(t *testing.T) { func TestCachedRuntimePath_Cached(t *testing.T) { dir := t.TempDir() - setTestHomeDir(t, dir) + testutil.SetHome(t, dir) version := "99.99.99" launcherDir := filepath.Join(dir, ".mxcli", "runtime", version, "runtime", "launcher") @@ -177,7 +170,7 @@ func TestCachedRuntimePath_Cached(t *testing.T) { func TestResolveMxBuild_FindsCachedVersion(t *testing.T) { dir := t.TempDir() - setTestHomeDir(t, dir) + testutil.SetHome(t, dir) // Set up a fake cached mxbuild modelerDir := filepath.Join(dir, ".mxcli", "mxbuild", "11.6.3", "modeler") diff --git a/cmd/mxcli/docker/ensuredb_test.go b/cmd/mxcli/docker/ensuredb_test.go index cbcf0ee39b..6dde385441 100644 --- a/cmd/mxcli/docker/ensuredb_test.go +++ b/cmd/mxcli/docker/ensuredb_test.go @@ -13,6 +13,8 @@ import ( "strings" "testing" "time" + + "github.com/mendixlabs/mxcli/internal/testutil" ) func TestSplitHostPort(t *testing.T) { @@ -159,7 +161,7 @@ func newStubPATH(t *testing.T) (dir, logPath string) { } dir = t.TempDir() t.Setenv("PATH", dir) - t.Setenv("HOME", t.TempDir()) + testutil.SetHome(t, t.TempDir()) // A stubbed PATH is only hermetic if the server-binary lookup is stubbed // too: this host really does have /usr/lib/postgresql/*/bin, so a test that // asserts "the tools are missing" would otherwise run a real initdb. diff --git a/cmd/mxcli/docker/mxserve_resolution_test.go b/cmd/mxcli/docker/mxserve_resolution_test.go index 6bf8a39fc8..9ff619e6a5 100644 --- a/cmd/mxcli/docker/mxserve_resolution_test.go +++ b/cmd/mxcli/docker/mxserve_resolution_test.go @@ -8,6 +8,8 @@ import ( "path/filepath" "strings" "testing" + + "github.com/mendixlabs/mxcli/internal/testutil" ) // The local loop resolves mxbuild once (ResolveMxBuildForLocal) and then started @@ -37,7 +39,7 @@ func plantCacheEntry(t *testing.T, home, version string) string { // A known version that is not cached must fail HERE, naming both versions. func TestStartServeRefusesVersionSubstitution(t *testing.T) { home := t.TempDir() - t.Setenv("HOME", home) + testutil.SetHome(t, home) plantCacheEntry(t, home, "11.14.0") _, err := StartServe(ServeOptions{Version: "11.12.2"}) @@ -58,7 +60,7 @@ func TestStartServeRefusesVersionSubstitution(t *testing.T) { // half-populated directory that defeats CachedMxBuildPath is mxcli's own work. func TestStartServeRefusesModelerlessCacheDir(t *testing.T) { home := t.TempDir() - t.Setenv("HOME", home) + testutil.SetHome(t, home) plantCacheEntry(t, home, "11.14.0") // What ensureMxBuildRuntimeSibling leaves behind for the project's version. @@ -90,7 +92,7 @@ func TestStartServeRefusesModelerlessCacheDir(t *testing.T) { // mismatched against. func TestStartServeUsesAnyCachedOnlyWithoutAVersion(t *testing.T) { home := t.TempDir() - t.Setenv("HOME", home) + testutil.SetHome(t, home) planted := plantCacheEntry(t, home, "11.14.0") if got := resolveServeMxBuild(ServeOptions{}); got != planted { diff --git a/cmd/mxcli/docker/new_project_version_test.go b/cmd/mxcli/docker/new_project_version_test.go index 3cf6cd2ec9..b37fd53bc0 100644 --- a/cmd/mxcli/docker/new_project_version_test.go +++ b/cmd/mxcli/docker/new_project_version_test.go @@ -19,6 +19,8 @@ import ( "os" "path/filepath" "testing" + + "github.com/mendixlabs/mxcli/internal/testutil" ) // cacheFakeMx plants a fake cached mx binary for each version under a fake HOME. @@ -41,7 +43,7 @@ func cacheFakeMx(t *testing.T, home string, versions ...string) map[string]strin func isolateResolution(t *testing.T, home string) { t.Helper() - setTestHomeDir(t, home) + testutil.SetHome(t, home) setTestApplicationsDir(t, t.TempDir()) // no real macOS Studio Pro t.Setenv("PATH", t.TempDir()) // no mx on PATH } diff --git a/cmd/mxcli/docker/procgroup_windows.go b/cmd/mxcli/docker/procgroup_windows.go index 6e1a3fd916..fd4508365d 100644 --- a/cmd/mxcli/docker/procgroup_windows.go +++ b/cmd/mxcli/docker/procgroup_windows.go @@ -9,6 +9,8 @@ import ( "os/exec" "strconv" "syscall" + + "github.com/mendixlabs/mxcli/internal/procalive" ) // procgroup_windows.go provides the Windows implementations of the process @@ -76,19 +78,11 @@ func killProcessTree(pid int) error { // had exited the instant it was launched, and the local loop never got past // "Starting mxbuild --serve...". WaitForSingleObject on a SYNCHRONIZE handle is // the real check: it returns WAIT_TIMEOUT while the process runs and -// WAIT_OBJECT_0 once it has terminated (even before it is reaped). +// WAIT_OBJECT_0 once it has terminated (even before it is reaped). The check +// itself now lives in procalive.Alive, shared with the handshake readers. func processAlive(p *os.Process) bool { if p == nil { return false } - h, err := syscall.OpenProcess(syscall.SYNCHRONIZE, false, uint32(p.Pid)) - if err != nil { - return false - } - defer syscall.CloseHandle(h) - event, err := syscall.WaitForSingleObject(h, 0) - if err != nil { - return false - } - return event == syscall.WAIT_TIMEOUT + return procalive.Alive(p.Pid) } diff --git a/cmd/mxcli/docker/runlocal_test.go b/cmd/mxcli/docker/runlocal_test.go index e9b27699d3..d4efe98bf6 100644 --- a/cmd/mxcli/docker/runlocal_test.go +++ b/cmd/mxcli/docker/runlocal_test.go @@ -12,6 +12,7 @@ import ( "testing" "time" + "github.com/mendixlabs/mxcli/internal/testutil" "github.com/mendixlabs/mxcli/model" ) @@ -245,7 +246,7 @@ func TestLocalRunOptions_DefaultsRespectOverrides(t *testing.T) { func TestEnsureMxBuildRuntimeSibling(t *testing.T) { // Point the cache roots at a temp HOME so we don't touch the real cache. home := t.TempDir() - t.Setenv("HOME", home) + testutil.SetHome(t, home) version := "99.99.99" // Build the runtime cache with a runtime/ dir. @@ -275,7 +276,7 @@ func TestEnsureMxBuildRuntimeSibling(t *testing.T) { func TestEnsureMxBuildRuntimeSibling_MissingSource(t *testing.T) { home := t.TempDir() - t.Setenv("HOME", home) + testutil.SetHome(t, home) version := "99.99.98" mxbuildCache, _ := MxBuildCacheDir(version) _ = os.MkdirAll(filepath.Join(mxbuildCache, "modeler"), 0o755) diff --git a/cmd/mxcli/docker/session_other.go b/cmd/mxcli/docker/session_other.go index 3319fda913..731f640a8c 100644 --- a/cmd/mxcli/docker/session_other.go +++ b/cmd/mxcli/docker/session_other.go @@ -5,10 +5,9 @@ package docker import ( - "os" - "runtime" - "syscall" "time" + + "github.com/mendixlabs/mxcli/internal/procalive" ) // SessionMembers is Linux-only (it reads /proc). Elsewhere `run stop` relies on @@ -19,17 +18,9 @@ func SessionMembers(sid int, notBefore time.Time) []int { return nil } func ProcessCmdline(pid int) string { return "" } // PidAlive reports whether pid is a live process. -func PidAlive(pid int) bool { - if pid <= 0 { - return false - } - p, err := os.FindProcess(pid) - if err != nil { - return false - } - if runtime.GOOS == "windows" { - // FindProcess opens a handle there, which fails for a pid that is gone. - return true - } - return p.Signal(syscall.Signal(0)) == nil -} +// +// On Windows this used to be "os.FindProcess can open it", but an exited +// process stays openable while anyone holds a handle to it (its parent, +// typically), so `run stop` kept reporting a run that had shut down as still +// alive. procalive.Alive asks whether the process has terminated instead. +func PidAlive(pid int) bool { return procalive.Alive(pid) } diff --git a/cmd/mxcli/docker/session_windows_test.go b/cmd/mxcli/docker/session_windows_test.go new file mode 100644 index 0000000000..1744f3db48 --- /dev/null +++ b/cmd/mxcli/docker/session_windows_test.go @@ -0,0 +1,49 @@ +// SPDX-License-Identifier: Apache-2.0 + +//go:build windows + +package docker + +import ( + "syscall" + "testing" +) + +// `run stop` waits for PidAlive to turn false, then reports whatever is still +// alive. PidAlive used to say a pid was alive whenever os.FindProcess could +// open it, and Windows keeps an exited process openable for as long as anyone +// holds a handle to it — the parent that started it, typically. So a run that +// had shut down read as alive, `run stop` waited out its grace period, killed +// a process that was already gone, and still printed "failed: 1 process(es) … +// are still alive" (the same class as mendixlabs/mxcli#1284). +func TestPidAlive_ExitedProcessWithAnOpenHandleIsNotAlive(t *testing.T) { + cmd := helperCmd(t, "sleep") + if err := cmd.Start(); err != nil { + t.Fatal(err) + } + pid := cmd.Process.Pid + t.Cleanup(func() { _ = cmd.Process.Kill(); _ = cmd.Wait() }) + + if !PidAlive(pid) { + t.Fatalf("PidAlive(%d) = false for a running process", pid) + } + + // Hold a handle of our own, as the run's parent does, so the process object + // outlives the process. + h, err := syscall.OpenProcess(syscall.SYNCHRONIZE, false, uint32(pid)) + if err != nil { + t.Fatalf("OpenProcess: %v", err) + } + defer syscall.CloseHandle(h) + + if err := cmd.Process.Kill(); err != nil { + t.Fatal(err) + } + if ev, err := syscall.WaitForSingleObject(h, 10_000); err != nil || ev != syscall.WAIT_OBJECT_0 { + t.Fatalf("process did not exit after Kill: event=%d err=%v", ev, err) + } + + if PidAlive(pid) { + t.Fatalf("PidAlive(%d) = true for a process that has exited (a handle to it is still open)", pid) + } +} diff --git a/cmd/mxcli/marketplace/refcache_test.go b/cmd/mxcli/marketplace/refcache_test.go index 50e3985dcd..46a40b9aeb 100644 --- a/cmd/mxcli/marketplace/refcache_test.go +++ b/cmd/mxcli/marketplace/refcache_test.go @@ -8,6 +8,8 @@ import ( "strings" "testing" "time" + + "github.com/mendixlabs/mxcli/internal/testutil" ) // TestCacheReadyNeedsTheMarker guards the invariant the whole cache rests on: a @@ -210,7 +212,7 @@ func readFile(t *testing.T, path string) string { // hold anything and every run would silently rebuild. func TestPruneKeepsNewestAndBoundOfOne(t *testing.T) { root := t.TempDir() - t.Setenv("HOME", root) + testutil.SetHome(t, root) refRoot := filepath.Join(root, ".mxcli", "marketplace-refs", "ref") // Three entries, oldest first, with distinct marker times. @@ -250,7 +252,7 @@ func TestPruneKeepsNewestAndBoundOfOne(t *testing.T) { // TestPruneDisabledByZero checks the escape hatch for anyone with disk to spare. func TestPruneDisabledByZero(t *testing.T) { root := t.TempDir() - t.Setenv("HOME", root) + testutil.SetHome(t, root) refRoot := filepath.Join(root, ".mxcli", "marketplace-refs", "ref") for _, n := range []string{"a", "b", "c"} { mustWrite(t, filepath.Join(refRoot, n, completeMarker), "") diff --git a/cmd/mxcli/oneliner_test.go b/cmd/mxcli/oneliner_test.go index 967c66f468..3102dd15a3 100644 --- a/cmd/mxcli/oneliner_test.go +++ b/cmd/mxcli/oneliner_test.go @@ -6,13 +6,15 @@ import ( "bytes" "strings" "testing" + + "github.com/mendixlabs/mxcli/internal/testutil" ) // runOneLinerForTest runs `mxcli -c ` without a project and returns // the exit code and everything written to the error stream. func runOneLinerForTest(t *testing.T, commands string, continueOnError bool) (int, string, string) { t.Helper() - t.Setenv("HOME", t.TempDir()) // the session log is not the developer's + testutil.SetHome(t, t.TempDir()) // the session log is not the developer's var errOut bytes.Buffer var code int out, _ := captureStdout(t, func() error { diff --git a/cmd/mxcli/testmain_test.go b/cmd/mxcli/testmain_test.go new file mode 100644 index 0000000000..b319662869 --- /dev/null +++ b/cmd/mxcli/testmain_test.go @@ -0,0 +1,50 @@ +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "fmt" + "os" + "testing" +) + +// TestMain keeps every test in this package out of the developer's real +// ~/.mxcli/logs. +// +// A test that runs a command in-process (runCheckFiles, runCommandLine, ...) +// reaches diaglog.Init through newLoggedExecutorTo. Init appends session_start +// and session_end lines to ~/.mxcli/logs/mxcli-.log and then runs +// cleanOldLogs on that directory, so on every OS a plain `go test` wrote into +// the developer's own diagnostics log and could delete their older log files. +// +// MXCLI_LOG_DIR is diaglog's own hook for this ("keeps a test out of the user's +// real log directory"), but only the two tests that spawn a subprocess set it. +// Setting it once here covers every in-process test, present and future, +// instead of each one having to remember a per-test Setenv. A developer who +// has pointed MXCLI_LOG_DIR somewhere on purpose keeps that choice. +// +// Child processes inherit the variable, which is what they want; the two +// per-test overrides (json_output_purity_test.go, session_start_test.go) give +// their child a directory of its own to assert on and stay as they are. +func TestMain(m *testing.M) { + if os.Getenv("MXCLI_LOG_DIR") != "" { + os.Exit(m.Run()) + } + + logDir, err := os.MkdirTemp("", "mxcli-test-logs-*") + if err != nil { + fmt.Fprintf(os.Stderr, "TestMain: cannot create a temporary log directory: %v\n", err) + os.Exit(1) + } + if err := os.Setenv("MXCLI_LOG_DIR", logDir); err != nil { + os.RemoveAll(logDir) + fmt.Fprintf(os.Stderr, "TestMain: cannot set MXCLI_LOG_DIR: %v\n", err) + os.Exit(1) + } + + code := m.Run() + // diaglog holds the day's log file open until the process ends, so on + // Windows the removal can fail; a leftover temp directory is harmless. + _ = os.RemoveAll(logDir) + os.Exit(code) +} diff --git a/cmd/mxcli/testrunner/handshake.go b/cmd/mxcli/testrunner/handshake.go index e46f686830..54bb73fc00 100644 --- a/cmd/mxcli/testrunner/handshake.go +++ b/cmd/mxcli/testrunner/handshake.go @@ -7,13 +7,10 @@ import ( "fmt" "os" "path/filepath" - "syscall" "time" -) -// syscallSignalZero is signal 0: delivered to no one, but still performs the -// process-exists and permission checks. The idiom for "is this pid alive?". -const syscallSignalZero = syscall.Signal(0) + "github.com/mendixlabs/mxcli/internal/procalive" +) // HandshakeFile is where a `run --local --test-endpoint` session publishes what // `mxcli test --attach` needs to reach it. It lives beside the project rather @@ -109,7 +106,7 @@ func ReadHandshake(projectPath string) (*Handshake, error) { if h.Token == "" || h.AppPort == 0 { return nil, fmt.Errorf("%s is incomplete; stop and restart the hosting 'mxcli run --local --test-endpoint'", path) } - if !processAlive(h.PID) { + if !procalive.Alive(h.PID) { return nil, fmt.Errorf("the app that published %s (pid %d, started %s) is no longer running\n"+ " Start one with: mxcli run --local --test-endpoint -p %s", path, h.PID, h.Started.Format(time.RFC3339), projectPath) @@ -117,18 +114,5 @@ func ReadHandshake(projectPath string) (*Handshake, error) { return &h, nil } -// processAlive reports whether a pid names a live process. Signal 0 performs the -// existence and permission checks without delivering anything. -func processAlive(pid int) bool { - if pid <= 0 { - return false - } - proc, err := os.FindProcess(pid) - if err != nil { - return false - } - return proc.Signal(syscallSignalZero) == nil -} - // nowFunc is time.Now, indirected so a test can pin the timestamp. var nowFunc = time.Now diff --git a/internal/auth/client_test.go b/internal/auth/client_test.go index 8b9cfcace6..77251c0a62 100644 --- a/internal/auth/client_test.go +++ b/internal/auth/client_test.go @@ -8,6 +8,8 @@ import ( "net/http/httptest" "net/url" "testing" + + "github.com/mendixlabs/mxcli/internal/testutil" ) // rewriteTransport routes all requests to a given test server's URL while @@ -122,7 +124,7 @@ func TestAuthTransport_DoesNotMutateCallerRequest(t *testing.T) { } func TestClientFor_ResolvesFromEnv(t *testing.T) { - t.Setenv("HOME", t.TempDir()) + testutil.SetHome(t, t.TempDir()) t.Setenv(EnvPAT, "env-pat") t.Setenv(EnvProfile, "") @@ -139,7 +141,7 @@ func TestClientFor_ResolvesFromEnv(t *testing.T) { } func TestClientFor_NoCredential(t *testing.T) { - t.Setenv("HOME", t.TempDir()) + testutil.SetHome(t, t.TempDir()) t.Setenv(EnvPAT, "") t.Setenv(EnvProfile, "") diff --git a/internal/auth/resolver_test.go b/internal/auth/resolver_test.go index 05bb9826a8..0e941835e6 100644 --- a/internal/auth/resolver_test.go +++ b/internal/auth/resolver_test.go @@ -6,6 +6,8 @@ import ( "context" "errors" "testing" + + "github.com/mendixlabs/mxcli/internal/testutil" ) func TestCredentialFromEnv_PAT(t *testing.T) { @@ -55,7 +57,7 @@ func TestCredentialFromEnv_WhitespaceTrimmed(t *testing.T) { func TestResolve_EnvWinsOverStore(t *testing.T) { // Point DefaultFileStore at a tempdir to avoid touching the real // ~/.mxcli/auth.json. - t.Setenv("HOME", t.TempDir()) + testutil.SetHome(t, t.TempDir()) t.Setenv(EnvPAT, "env-pat") t.Setenv(EnvProfile, "") @@ -76,7 +78,7 @@ func TestResolve_EnvWinsOverStore(t *testing.T) { } func TestResolve_FallsBackToStore(t *testing.T) { - t.Setenv("HOME", t.TempDir()) + testutil.SetHome(t, t.TempDir()) t.Setenv(EnvPAT, "") t.Setenv(EnvProfile, "") @@ -93,7 +95,7 @@ func TestResolve_FallsBackToStore(t *testing.T) { } func TestResolve_NoCredential(t *testing.T) { - t.Setenv("HOME", t.TempDir()) + testutil.SetHome(t, t.TempDir()) t.Setenv(EnvPAT, "") t.Setenv(EnvProfile, "") @@ -105,7 +107,7 @@ func TestResolve_NoCredential(t *testing.T) { } func TestResolve_EmptyProfileDefaultsToDefault(t *testing.T) { - t.Setenv("HOME", t.TempDir()) + testutil.SetHome(t, t.TempDir()) t.Setenv(EnvPAT, "env-pat") t.Setenv(EnvProfile, "") diff --git a/internal/procalive/alive_test.go b/internal/procalive/alive_test.go new file mode 100644 index 0000000000..589627c6ec --- /dev/null +++ b/internal/procalive/alive_test.go @@ -0,0 +1,67 @@ +// SPDX-License-Identifier: Apache-2.0 + +package procalive + +import ( + "os" + "os/exec" + "testing" + "time" +) + +// helperEnv makes the re-executed test binary run TestHelperSleep instead of +// the real tests. +const helperEnv = "PROCALIVE_TEST_SLEEP_HELPER" + +// TestHelperSleep is not a test: it is the body of the child process the tests +// below start. It sleeps until it is killed, bounded so a leaked child cannot +// outlive the test run for long. +func TestHelperSleep(t *testing.T) { + if os.Getenv(helperEnv) != "1" { + t.Skip("helper process only") + } + time.Sleep(2 * time.Minute) +} + +func TestAliveSelf(t *testing.T) { + if !Alive(os.Getpid()) { + t.Fatal("Alive(os.Getpid()) = false for the running test process") + } +} + +func TestAliveRejectsNonPositivePid(t *testing.T) { + for _, pid := range []int{0, -1} { + if Alive(pid) { + t.Errorf("Alive(%d) = true", pid) + } + } +} + +func TestAliveFollowsAChildProcess(t *testing.T) { + cmd := exec.Command(os.Args[0], "-test.run=^TestHelperSleep$") + cmd.Env = append(os.Environ(), helperEnv+"=1") + if err := cmd.Start(); err != nil { + t.Fatalf("starting the helper process: %v", err) + } + reaped := false + t.Cleanup(func() { + if !reaped { + _ = cmd.Process.Kill() + _ = cmd.Wait() + } + }) + + if !Alive(cmd.Process.Pid) { + t.Fatalf("Alive(%d) = false for a running child", cmd.Process.Pid) + } + + if err := cmd.Process.Kill(); err != nil { + t.Fatalf("killing the helper process: %v", err) + } + _ = cmd.Wait() // reaps it: on POSIX an unreaped zombie still answers signal 0 + reaped = true + + if Alive(cmd.Process.Pid) { + t.Fatalf("Alive(%d) = true for a child that was killed and reaped", cmd.Process.Pid) + } +} diff --git a/internal/procalive/alive_unix.go b/internal/procalive/alive_unix.go new file mode 100644 index 0000000000..110b8f22b5 --- /dev/null +++ b/internal/procalive/alive_unix.go @@ -0,0 +1,26 @@ +// SPDX-License-Identifier: Apache-2.0 + +//go:build !windows + +package procalive + +import ( + "os" + "syscall" +) + +// Alive reports whether pid names a running process. On POSIX, signal 0 is +// delivered to no one but still performs the existence and permission checks, +// so it succeeds for a live process. It also succeeds for an unreaped zombie, +// which is why a caller that owns the child must reap it (cmd.Wait) before +// asking. +func Alive(pid int) bool { + if pid <= 0 { + return false + } + p, err := os.FindProcess(pid) + if err != nil { + return false + } + return p.Signal(syscall.Signal(0)) == nil +} diff --git a/internal/procalive/alive_windows.go b/internal/procalive/alive_windows.go new file mode 100644 index 0000000000..d7ec844d0b --- /dev/null +++ b/internal/procalive/alive_windows.go @@ -0,0 +1,32 @@ +// SPDX-License-Identifier: Apache-2.0 + +//go:build windows + +package procalive + +import "syscall" + +// Alive reports whether pid names a running process. +// +// os.Process.Signal(syscall.Signal(0)) is NOT a liveness test on Windows: Go +// returns EWINDOWS ("not supported by windows") for every signal except Kill, so +// it reports a live process as dead (mendixlabs/mxcli#1284). WaitForSingleObject +// on a SYNCHRONIZE handle is the real check: it returns WAIT_TIMEOUT while the +// process runs and WAIT_OBJECT_0 once it has terminated (even before it is +// reaped). A pid that cannot be opened (no such process, or access denied) reads +// as not alive. +func Alive(pid int) bool { + if pid <= 0 { + return false + } + h, err := syscall.OpenProcess(syscall.SYNCHRONIZE, false, uint32(pid)) + if err != nil { + return false + } + defer syscall.CloseHandle(h) + event, err := syscall.WaitForSingleObject(h, 0) + if err != nil { + return false + } + return event == syscall.WAIT_TIMEOUT +} diff --git a/internal/procalive/doc.go b/internal/procalive/doc.go new file mode 100644 index 0000000000..fd48cd81b8 --- /dev/null +++ b/internal/procalive/doc.go @@ -0,0 +1,12 @@ +// SPDX-License-Identifier: Apache-2.0 + +// Package procalive answers one question: is the process with this pid still +// running? +// +// It exists because the portable-looking answer, os.FindProcess(pid) followed by +// Signal(0), is wrong on Windows: Go supports no signal but Kill there, so every +// pid reads as dead (mendixlabs/mxcli#1284). `mxcli test --attach` and the dev +// loop's "already serving" check both read a pid out of a handshake file, so on +// Windows they refused a live host. The per-OS files hold the one check that +// works on each platform; callers that only have a pid use Alive. +package procalive diff --git a/internal/testutil/guard_test.go b/internal/testutil/guard_test.go new file mode 100644 index 0000000000..164249155e --- /dev/null +++ b/internal/testutil/guard_test.go @@ -0,0 +1,81 @@ +// SPDX-License-Identifier: Apache-2.0 + +package testutil + +import ( + "bufio" + "io/fs" + "os" + "path/filepath" + "runtime" + "strings" + "testing" +) + +// moduleRoot walks up from this file to the directory holding go.mod. +func moduleRoot(t *testing.T) string { + t.Helper() + _, file, _, ok := runtime.Caller(0) + if !ok { + t.Fatal("runtime.Caller failed") + } + dir := filepath.Dir(file) + for { + if _, err := os.Stat(filepath.Join(dir, "go.mod")); err == nil { + return dir + } + parent := filepath.Dir(dir) + if parent == dir { + t.Fatalf("no go.mod above %s", file) + } + dir = parent + } +} + +// TestNoBareHomeSetenv fails for any test file that isolates the home +// directory with Setenv("HOME", ...) alone. On Windows os.UserHomeDir reads +// USERPROFILE, so such a test silently reads and writes the developer's real +// ~/.mxcli (#1256). SetHome sets both variables. +func TestNoBareHomeSetenv(t *testing.T) { + root := moduleRoot(t) + skipNames := map[string]bool{"vendor": true, "node_modules": true, ".git": true} + skipRel := map[string]bool{ + filepath.Join("mdl", "grammar", "parser"): true, // generated, gitignored + filepath.Join("internal", "testutil"): true, // home of SetHome itself + } + needle := `Setenv("HOME"` + + err := filepath.WalkDir(root, func(path string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + if d.IsDir() { + rel, _ := filepath.Rel(root, path) + if skipNames[d.Name()] || skipRel[rel] { + return filepath.SkipDir + } + return nil + } + if !strings.HasSuffix(path, "_test.go") { + return nil + } + f, err := os.Open(path) + if err != nil { + return err + } + defer f.Close() + rel, _ := filepath.Rel(root, path) + sc := bufio.NewScanner(f) + for n := 1; sc.Scan(); n++ { + if strings.Contains(sc.Text(), needle) { + t.Errorf("%s:%d: bare Setenv(\"HOME\", ...): use testutil.SetHome(t, dir) instead; "+ + "os.UserHomeDir reads USERPROFILE on Windows, so a HOME-only override leaves the developer's real ~/.mxcli in use (#1256)", + filepath.ToSlash(rel), n) + } + } + return sc.Err() + }) + if err != nil { + t.Fatal(err) + } +} diff --git a/internal/testutil/home.go b/internal/testutil/home.go new file mode 100644 index 0000000000..6184b524dd --- /dev/null +++ b/internal/testutil/home.go @@ -0,0 +1,24 @@ +// SPDX-License-Identifier: Apache-2.0 + +// Package testutil holds helpers shared by tests across the module. +// +// The one that matters most is SetHome. os.UserHomeDir reads USERPROFILE on +// Windows and HOME everywhere else, so a test that isolates the home directory +// with t.Setenv("HOME", dir) only does so on Unix. On Windows it silently reads +// and writes the developer's real %USERPROFILE%\.mxcli: a fixture PAT in +// auth.json, fake mxbuild/runtime caches, the marketplace catalog cache +// (mendixlabs/mxcli#1256). TestNoBareHomeSetenv fails the build for any test +// file that does it. +package testutil + +import "testing" + +// SetHome points os.UserHomeDir at dir for the duration of the test, on every +// OS. It sets both HOME and USERPROFILE rather than switching on runtime.GOOS: +// setting the variable an OS ignores is harmless, and there is no branch to +// get wrong. +func SetHome(t testing.TB, dir string) { + t.Helper() + t.Setenv("HOME", dir) + t.Setenv("USERPROFILE", dir) +} diff --git a/internal/testutil/home_test.go b/internal/testutil/home_test.go new file mode 100644 index 0000000000..f3942a8700 --- /dev/null +++ b/internal/testutil/home_test.go @@ -0,0 +1,20 @@ +// SPDX-License-Identifier: Apache-2.0 + +package testutil + +import ( + "os" + "testing" +) + +func TestSetHomeRedirectsUserHomeDir(t *testing.T) { + dir := t.TempDir() + SetHome(t, dir) + got, err := os.UserHomeDir() + if err != nil { + t.Fatalf("UserHomeDir: %v", err) + } + if got != dir { + t.Errorf("os.UserHomeDir() = %q, want %q", got, dir) + } +} diff --git a/mdl-examples/bug-tests/1284-test-attach-windows.test.mdl b/mdl-examples/bug-tests/1284-test-attach-windows.test.mdl new file mode 100644 index 0000000000..35e7ddc8af --- /dev/null +++ b/mdl-examples/bug-tests/1284-test-attach-windows.test.mdl @@ -0,0 +1,17 @@ +/** + * mendixlabs/mxcli#1284: on Windows, `mxcli test --attach` refused a live app + * with "the app that published .mxcli/test-endpoint.json (pid N) is no longer + * running": the handshake checked the pid with Signal(0), which Windows does + * not support, so every pid read as dead. + * + * Setup: `mxcli run --local --db-type hsqldb --test-endpoint -p App.mpr`, then + * from another terminal `mxcli test -p App.mpr --attach`. Needs no + * project content: the test only has to reach the running app and run there. + * `mxcli run stop -p App.mpr` afterwards reports "stopped: pid N" (it said + * "failed: 1 process(es) … are still alive" on Windows before the PidAlive fix). + * + * @test the attached app runs a test + * @expect $Two = 2 + */ +declare $Two Integer = 1 + 1; +/ diff --git a/mdl-examples/bug-tests/1341-navigation-submenu-keeps-page-action.mdl b/mdl-examples/bug-tests/1341-navigation-submenu-keeps-page-action.mdl new file mode 100644 index 0000000000..bb213a2c41 --- /dev/null +++ b/mdl-examples/bug-tests/1341-navigation-submenu-keeps-page-action.mdl @@ -0,0 +1,42 @@ +mdl 1; +-- ============================================================================ +-- mendixlabs/mxcli#1341: a sub-menu does not keep a same-caption item's action +-- ============================================================================ +-- +-- Turning the page item 'Work' into a sub-menu 'Work' paired the two by caption +-- and carried the page item's Forms$FormAction onto the sub-menu (v0.25.0): +-- +-- kept the stored action of menu item 'Work' (Forms$FormAction): MDL cannot +-- express it, and the item states none +-- +-- exec and describe reported success; mx check then failed with CE0548 "Items +-- with subitems cannot have an action themselves." The sub-menu must be written +-- with Forms$NoAction, and `mxcli docker check` must report 0 errors after the +-- second statement. + +create module NavSubmenu1341; + +create page NavSubmenu1341.Accounts ( Title: 'Accounts', Layout: Atlas_Core.Atlas_Default ) { + dynamictext t (Content: 'Accounts') +}; + +create page NavSubmenu1341.Sessions ( Title: 'Sessions', Layout: Atlas_Core.Atlas_Default ) { + dynamictext t (Content: 'Sessions') +}; + +-- Step 1: 'Work' is a page item. +create or modify navigation Responsive + home page NavSubmenu1341.Accounts + { + menu item 'Work' ( OnClick: show page NavSubmenu1341.Accounts ) + }; + +-- Step 2: 'Work' becomes a sub-menu under the same caption. +create or modify navigation Responsive + home page NavSubmenu1341.Accounts + { + menu 'Work' { + menu item 'Accounts' ( OnClick: show page NavSubmenu1341.Accounts ) + menu item 'Sessions' ( OnClick: show page NavSubmenu1341.Sessions ) + } + }; diff --git a/mdl/diaglog/diaglog_test.go b/mdl/diaglog/diaglog_test.go index 1d654a953d..c09e4b59b2 100644 --- a/mdl/diaglog/diaglog_test.go +++ b/mdl/diaglog/diaglog_test.go @@ -5,10 +5,11 @@ package diaglog import ( "os" "path/filepath" - "runtime" "strings" "testing" "time" + + "github.com/mendixlabs/mxcli/internal/testutil" ) func TestNilLoggerIsSafe(t *testing.T) { @@ -23,20 +24,10 @@ func TestNilLoggerIsSafe(t *testing.T) { l.Close() } -func setHomeDir(t *testing.T, dir string) { - t.Helper() - // Windows uses USERPROFILE, Unix uses HOME. os.UserHomeDir() checks both. - if runtime.GOOS == "windows" { - t.Setenv("USERPROFILE", dir) - } else { - t.Setenv("HOME", dir) - } -} - func TestInitAndClose(t *testing.T) { // Use a temp dir for logs tmpDir := t.TempDir() - setHomeDir(t, tmpDir) + testutil.SetHome(t, tmpDir) // Init is a per-process singleton (ako/mxcli#617); start from a clean one so // this test does not inherit the previous test's open session. resetForTest() @@ -65,7 +56,7 @@ func TestInitAndClose(t *testing.T) { func TestCommandLogging(t *testing.T) { tmpDir := t.TempDir() - setHomeDir(t, tmpDir) + testutil.SetHome(t, tmpDir) // Init is a per-process singleton (ako/mxcli#617); start from a clean one so // this test does not inherit the previous test's open session. resetForTest() @@ -104,7 +95,7 @@ func TestCommandLogging(t *testing.T) { func TestDisabledViaEnv(t *testing.T) { tmpDir := t.TempDir() - setHomeDir(t, tmpDir) + testutil.SetHome(t, tmpDir) // Init is a per-process singleton (ako/mxcli#617); start from a clean one so // this test does not inherit the previous test's open session. resetForTest() diff --git a/mdl/executor/cmd_navigation.go b/mdl/executor/cmd_navigation.go index e857e2ff13..a01f844474 100644 --- a/mdl/executor/cmd_navigation.go +++ b/mdl/executor/cmd_navigation.go @@ -261,11 +261,11 @@ func keepStoredMenuAction(ctx *ExecContext, def ast.NavMenuItemDef, match *types if menuItemStatesAction(def) || len(match.StoredAction) == 0 { return false, "" } - // A sub-menu takes no OnClick, so a stored action on one is never stated. + // A sub-menu cannot have an action at all — mx check CE0548 "Items with + // subitems cannot have an action themselves" — so nothing stored is kept on + // it, least of all a page item's action paired by caption + // (mendixlabs/mxcli#1341). if len(def.Items) > 0 { - if t := menuActionTypeName(match); !isNoMenuAction(t) && t != "NoAction" { - return true, t - } return false, "" } if _, note := menuItemActionMDL(ctx, match); note != "" { diff --git a/mdl/executor/issue1341_submenu_keeps_page_action_test.go b/mdl/executor/issue1341_submenu_keeps_page_action_test.go new file mode 100644 index 0000000000..1e93172e3d --- /dev/null +++ b/mdl/executor/issue1341_submenu_keeps_page_action_test.go @@ -0,0 +1,121 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "bytes" + "strings" + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/backend/mock" + "github.com/mendixlabs/mxcli/mdl/types" + "github.com/mendixlabs/mxcli/mdl/visitor" + "github.com/mendixlabs/mxcli/model" +) + +// mendixlabs/mxcli#1341: "CREATE OR REPLACE NAVIGATION keeps a page item's +// action on a same-caption sub-menu (mx check CE0548; v0.25.0 regression)" — +// CE0548 "Items with subitems cannot have an action themselves". The rewrite +// paired the new sub-menu 'Work' with the stored page item 'Work' by caption +// and carried its Forms$FormAction, because a sub-menu "states no action". +// A sub-menu cannot have one, so nothing stored may be kept on it. + +// storedWorkPageItem is the menu as v0.25.0 read it back: a top-level page +// item 'Work' (a Forms$FormAction, which describe prints) beside 'Reports', +// whose action describe cannot print. +func storedWorkPageItem(pageAction, unknown []byte) []*types.NavMenuItem { + return []*types.NavMenuItem{ + {Caption: "Work", ActionType: "PageAction", Page: "Administration.Account_Overview", + StoredAction: pageAction, ActionDoc: map[string]any{"$Type": "Forms$FormAction"}}, + {Caption: "Reports", ActionType: "Forms$UnknownFutureClientAction", + StoredAction: unknown, ActionDoc: map[string]any{"$Type": "Forms$UnknownFutureClientAction"}}, + } +} + +func TestNavigationRewrite_SubMenuDoesNotKeepAStoredAction(t *testing.T) { + pageAction := []byte("stored-form-action") + unknown := []byte("stored-unknown-action") + var got types.NavigationProfileSpec + mb := &mock.MockBackend{ + IsConnectedFunc: func() bool { return true }, + GetNavigationFunc: func() (*types.NavigationDocument, error) { + return &types.NavigationDocument{Profiles: []*types.NavigationProfile{{ + Name: "Responsive", Kind: "Responsive", MenuItems: storedWorkPageItem(pageAction, unknown), + }}}, nil + }, + UpdateNavigationProfileFunc: func(_ model.ID, _ string, spec types.NavigationProfileSpec) error { + got = spec + return nil + }, + } + ctx, buf := newMockCtx(t, withBackend(mb)) + prog, errs := visitor.Build(`create or replace navigation Responsive + menu ( + menu 'Work' ( + menu item 'Accounts' page Administration.Account_Overview; + menu item 'Sessions' page Administration.ActiveSessions; + ); + menu item 'Reports'; + ) +;`) + if len(errs) > 0 { + t.Fatal(errs[0]) + } + assertNoError(t, execAlterNavigation(ctx, prog.Statements[0].(*ast.AlterNavigationStmt))) + + if len(got.MenuItems) != 2 || got.MenuItems[0].Caption != "Work" { + t.Fatalf("spec items = %+v, want 'Work' and 'Reports'", got.MenuItems) + } + work := got.MenuItems[0] + if work.KeepAction != nil { + t.Errorf("sub-menu 'Work' kept the stored page action %q: mx check CE0548 "+ + "\"Items with subitems cannot have an action themselves\"", work.KeepAction) + } + if len(work.Items) != 2 { + t.Errorf("sub-menu 'Work' has %d items, want 2", len(work.Items)) + } + if strings.Contains(buf.String(), "menu item 'Work'") { + t.Errorf("no action is kept on 'Work', so none may be reported:\n%s", buf.String()) + } + // CONTROL: a leaf that states no action still keeps one MDL cannot print + // (#980). A converter that kept nothing at all would pass the checks above. + if !bytes.Equal(got.MenuItems[1].KeepAction, unknown) { + t.Errorf("leaf 'Reports' must keep its stored action, got KeepAction=%q", got.MenuItems[1].KeepAction) + } +} + +// The menu-document writer shares the decision (menuItemsFromAST). +func TestCreateMenu_SubMenuDoesNotKeepAStoredAction(t *testing.T) { + pageAction := []byte("stored-form-action") + unknown := []byte("stored-unknown-action") + existing := &types.MenuDocument{ID: "menu-1", ContainerID: "folder-9", Name: "Main_Menu", + Items: storedWorkPageItem(pageAction, unknown)} + var updated *types.MenuDocument + mb := menuBackend(existing) + mb.GetModuleByNameFunc = func(name string) (*model.Module, error) { + return &model.Module{BaseElement: model.BaseElement{ID: "mod-1"}, Name: name}, nil + } + mb.UpdateMenuDocumentFunc = func(md *types.MenuDocument) error { updated = md; return nil } + + ctx, _ := newMockCtx(t, withBackend(mb)) + assertNoError(t, execCreateMenu(ctx, &ast.CreateMenuStmt{ + Name: ast.QualifiedName{Module: "MyModule", Name: "Main_Menu"}, + CreateOrModify: true, + Items: []ast.NavMenuItemDef{ + {Caption: "Work", Items: []ast.NavMenuItemDef{{Caption: "Accounts"}}}, + {Caption: "Reports"}, + }, + })) + + if updated == nil || len(updated.Items) != 2 { + t.Fatalf("UpdateMenuDocument got %+v", updated) + } + if work := updated.Items[0]; work.StoredAction != nil || work.ActionType != "NoAction" { + t.Errorf("sub-menu 'Work' kept ActionType=%q StoredAction=%q: CE0548", work.ActionType, work.StoredAction) + } + // CONTROL: the leaf keeps what MDL cannot print. + if !bytes.Equal(updated.Items[1].StoredAction, unknown) { + t.Errorf("leaf 'Reports' must keep its stored action, got %q", updated.Items[1].StoredAction) + } +} diff --git a/mdl/executor/roundtrip_helpers_mx_test.go b/mdl/executor/roundtrip_helpers_mx_test.go index 4ba6c34e70..0d5b1a7b07 100644 --- a/mdl/executor/roundtrip_helpers_mx_test.go +++ b/mdl/executor/roundtrip_helpers_mx_test.go @@ -10,6 +10,7 @@ import ( "testing" "github.com/mendixlabs/mxcli/cmd/mxcli/docker" + "github.com/mendixlabs/mxcli/internal/testutil" ) func TestNewestVersionedPath_PicksNewestNumericVersion(t *testing.T) { @@ -50,7 +51,7 @@ func TestFindMxBinary_PrefersPathOverCachedDownloads(t *testing.T) { } } - t.Setenv("HOME", home) + testutil.SetHome(t, home) t.Setenv("PATH", pathDir) t.Setenv("MX_BINARY", "")