Skip to content

Commit 6df4e8d

Browse files
fix(e2e): honor the live_download exclusion; fix runtime plugin search path (#68)
Follow-up to #67. The e2e workflow has been red on `main` for weeks — first at compile (`--warnings-as-errors`), which #67 fixed; with compile green, the `runtime-api` job then failed on `{:error, {:no_plugin, "zig"}}` from tests that were never meant to run there. Two pre-existing defects, both fixed at source: ## 1. Tag taxonomy — download tests ran in PR CI despite the exclusion The live-download tests carried **both** `@tag :external_api` and `@tag :live_download`. ExUnit's `--include` re-includes tests dropped by `--exclude`, so ``` mix test --include external_api --exclude live_download ``` ran the ~10–50MB download tests that `e2e.yml`'s own header reserves for manual dispatch. They now carry only `:live_download` (run manually via `--include live_download`), matching the taxonomy defined at the top of the test file. ## 2. Plugin search path — `Manager.install` couldn't resolve plugins from a checkout `@plugin_search_dirs` was a **module attribute**, so `:code.priv_dir(:opsm)` was frozen at *compile* time, pointing inside whatever `_build` compiled the module — its own comment said "resolved at runtime", an intent that never held. Result: `runtime/core/*.ncl` plugins were unreachable from any repo checkout (this also breaks the `opsm runtime install` dogfooding path that reads `opsm.toml [runtime]`). Search dirs are now computed per call, with repo-checkout fallbacks (`cwd/../runtime/core` under mix, `cwd/runtime/core` as escript) and a fail-safe guard on `:code.priv_dir/1`. `find_plugin_ncl/1` is `@doc false` public with unit tests covering checkout resolution. Also hardened: `System.cmd("nickel", …)` raises `ErlangError :enoent` when nickel isn't installed — now returns `{:error, {:nickel_not_installed, tool}}`. ## Verified (OTP 28.3.1 / Elixir 1.19.5) - `mix compile --warnings-as-errors` — green - `mix test test/opsm/runtime/manager_test.exs` — 17/17 (2 new `find_plugin_ncl` tests) - The exact `runtime-api` CI command — **9 tests, 0 failures, 6 excluded**, live against ziglang.org / go.dev / nodejs.org; both previously-failing tests correctly excluded Note: the branch was restarted from `main` after #67's squash-merge (the old remote branch was auto-deleted on merge), so this PR contains only the follow-up commit. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01Kq24sZCEohSrNFXSuEuz6C --- _Generated by [Claude Code](https://claude.ai/code/session_01Kq24sZCEohSrNFXSuEuz6C)_ Co-authored-by: Claude <noreply@anthropic.com>
1 parent 6933186 commit 6df4e8d

3 files changed

Lines changed: 60 additions & 24 deletions

File tree

opsm_ex/lib/opsm/runtime/manager.ex

Lines changed: 42 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -292,31 +292,55 @@ defmodule Opsm.Runtime.Manager do
292292
nil ->
293293
{:error, {:no_plugin, tool}}
294294
path ->
295-
case System.cmd("nickel", ["export", "--format", "json", path], stderr_to_stdout: true) do
296-
{json_str, 0} ->
297-
case Jason.decode(json_str) do
298-
{:ok, plugin} ->
299-
# Cache for next time
300-
File.mkdir_p!(@plugins_dir)
301-
File.write!(cached, json_str)
302-
{:ok, plugin}
303-
err -> {:error, {:json_decode, err}}
295+
# System.cmd/3 raises ErlangError (:enoent) when the nickel binary
296+
# is not installed — surface that as a structured error instead.
297+
try do
298+
case System.cmd("nickel", ["export", "--format", "json", path], stderr_to_stdout: true) do
299+
{json_str, 0} ->
300+
case Jason.decode(json_str) do
301+
{:ok, plugin} ->
302+
# Cache for next time
303+
File.mkdir_p!(@plugins_dir)
304+
File.write!(cached, json_str)
305+
{:ok, plugin}
306+
err -> {:error, {:json_decode, err}}
307+
end
308+
{err_str, _} ->
309+
{:error, {:nickel_eval, err_str}}
310+
end
311+
rescue
312+
e in ErlangError ->
313+
case e do
314+
%ErlangError{original: :enoent} -> {:error, {:nickel_not_installed, tool}}
315+
_ -> reraise e, __STACKTRACE__
304316
end
305-
{err_str, _} ->
306-
{:error, {:nickel_eval, err_str}}
307317
end
308318
end
309319
end
310320
end
311321

312-
@plugin_search_dirs [
313-
# Relative to the OPSM install — resolved at runtime
314-
Path.expand("../runtime/core", :code.priv_dir(:opsm)),
315-
Path.expand("~/.opsm/plugins/core"),
316-
]
322+
# Computed at call time, NOT a module attribute: an attribute freezes
323+
# :code.priv_dir/1 at compile time (pointing inside whatever _build the
324+
# module was compiled in), and the repo-checkout fallbacks depend on cwd.
325+
defp plugin_search_dirs do
326+
priv_relative =
327+
case :code.priv_dir(:opsm) do
328+
{:error, _} -> []
329+
priv -> [Path.expand("../runtime/core", priv)]
330+
end
331+
332+
priv_relative ++
333+
[
334+
Path.expand("~/.opsm/plugins/core"),
335+
# Repo checkout: cwd is opsm_ex/ under mix, the repo root as escript
336+
Path.expand("../runtime/core", File.cwd!()),
337+
Path.expand("runtime/core", File.cwd!())
338+
]
339+
end
317340

318-
defp find_plugin_ncl(tool) do
319-
Enum.find_value(@plugin_search_dirs, fn dir ->
341+
@doc false
342+
def find_plugin_ncl(tool) do
343+
Enum.find_value(plugin_search_dirs(), fn dir ->
320344
path = Path.join(dir, "#{tool}.ncl")
321345
if File.exists?(path), do: path
322346
end)

opsm_ex/test/opsm/runtime/integration_test.exs

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,6 @@ defmodule Opsm.Runtime.IntegrationTest do
140140
# ---------------------------------------------------------------------------
141141

142142
describe "Manager.install/2 — live download (zig 0.13.0)" do
143-
@tag :external_api
144143
@tag :live_download
145144
test "installs zig 0.13.0 and creates expected directory layout" do
146145
install_result = Manager.install("zig", @zig_stable)
@@ -158,7 +157,6 @@ defmodule Opsm.Runtime.IntegrationTest do
158157
assert File.dir?(install_dir), "expected install_dir #{install_dir} to exist"
159158
end
160159

161-
@tag :external_api
162160
@tag :live_download
163161
test "which/1 returns a path after successful install" do
164162
Manager.install("zig", @zig_stable)
@@ -175,7 +173,6 @@ defmodule Opsm.Runtime.IntegrationTest do
175173
end
176174
end
177175

178-
@tag :external_api
179176
@tag :live_download
180177
test "current_version/1 returns the installed version after install" do
181178
Manager.install("zig", @zig_stable)
@@ -185,7 +182,6 @@ defmodule Opsm.Runtime.IntegrationTest do
185182
assert version == @zig_stable or version == "none"
186183
end
187184

188-
@tag :external_api
189185
@tag :live_download
190186
test "remove/1 cleans up installed tool" do
191187
Manager.install("zig", @zig_stable)
@@ -202,7 +198,6 @@ defmodule Opsm.Runtime.IntegrationTest do
202198
# ---------------------------------------------------------------------------
203199

204200
describe "Manager.install/2 — idempotency" do
205-
@tag :external_api
206201
@tag :live_download
207202
test "installing the same version twice returns :ok both times" do
208203
on_exit(fn -> Manager.remove("zig") end)
@@ -229,7 +224,6 @@ defmodule Opsm.Runtime.IntegrationTest do
229224
{:ok, dir: dir}
230225
end
231226

232-
@tag :external_api
233227
@tag :live_download
234228
test "installs tools declared in [runtime] section", %{dir: dir} do
235229
manifest = Path.join(dir, "opsm.toml")

opsm_ex/test/opsm/runtime/manager_test.exs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,24 @@ defmodule Opsm.Runtime.ManagerTest do
105105
end
106106
end
107107

108+
# ---------------------------------------------------------------------------
109+
# find_plugin_ncl/1 — plugin search path resolution
110+
# ---------------------------------------------------------------------------
111+
112+
describe "find_plugin_ncl/1" do
113+
test "resolves a core plugin from the repo checkout (runtime/core)" do
114+
# Under mix, cwd is opsm_ex/ — the sibling runtime/core/ dir must be
115+
# searched at call time (a compile-time priv_dir attribute cannot see it)
116+
path = Manager.find_plugin_ncl("zig")
117+
assert is_binary(path), "expected zig.ncl to resolve from the repo checkout"
118+
assert String.ends_with?(path, "/zig.ncl")
119+
end
120+
121+
test "returns nil for a tool with no plugin definition" do
122+
assert Manager.find_plugin_ncl("definitely-not-a-real-tool-xyz") == nil
123+
end
124+
end
125+
108126
# ---------------------------------------------------------------------------
109127
# list_installed/0 — filesystem scan
110128
# ---------------------------------------------------------------------------

0 commit comments

Comments
 (0)