test: cover the shipped surface, and make the coverage figure mean something

AGENTS.md

@@ -1143,7 +1143,9 @@ one Castle set silently would be one a consumer could not see in their own
 
 - Run `mix precommit` before committing. It is the single validation gate —
   `compile --warnings-as-errors`, `deps.unlock --unused`, `format`,
-  `credo --strict`, `test`. Do not run the individual checks piecemeal.
+  `credo --strict`, `test --cover`. Do not run the individual checks piecemeal.
+  The `--cover` is what enforces the coverage threshold; see *What
+  `mix test --cover` measures* for the figure and why it is what it is.
 - `@version` in `mix.exs` is the single source of truth for the version.
 - Add user-visible changes to `RELEASE.md` on the feature branch, using
   [Keep a Changelog](https://keepachangelog.com/) sections. Do not defer release
@@ -1259,6 +1261,18 @@ shapes differ in the version directory's release files: Mix assembles one,
 shape is how `Castle.Peer` came to refuse every unpacked release as ambiguous,
 having been reviewed seven times against a directory the peer path never meets.
 
+`build/2`'s `:override` points `lib/<app>-<vsn>` at a directory other than this
+node's own copy, leaving the release file alone — which is what a target
+carrying a *different build* of one of preboot's applications looks like, and the
+only way to be on the far side of `{Castle.Peer, :resolve, 1}`.
+`stub_castle/2` builds that castle: one exported `resolve/1` answering a fixed
+atom. **It compiles with `:compile.forms/2` rather than from Elixir source**,
+and that is load bearing rather than tidy — the module is named `Castle.Peer`,
+so `Code.compile_string/1` would load the stub over the running one and every
+peer test after it would be asserting against the fixture. Abstract forms are
+the one route to a beam the code server never sees. Do not "simplify" it onto
+`provider_app/5`.
+
 `Castle.Peer.materialise/2` takes `:boot_timeout` and `:resolve_timeout` for the
 same reason `Castle.Commands` takes the module to talk to — a deadline nothing
 can shorten is a deadline no test can show is enforced. Two tests give it a
@@ -1292,6 +1306,23 @@ asserting the acknowledged cost, that the by-path `chmod` does land on the
 swapped name. With the name left alone the two behaviours are identical, so
 there is no other way to tell them apart.
 
+**`fill/3`'s three steps need three different handles to be told apart, and a
+handle that is merely closed only reaches the first.** An already-closed handle
+fails the write *and* the close, `with :ok <- written, :ok <- closed` reports the
+write, and both helpers produce the same `"Cannot write #{path}"` prefix — so
+that case pins `written/3` (its discriminator is that `IO.binwrite/2` in place of
+`:file.write/2` raises instead of returning) and says nothing whatever about
+`closed/2`. It executed `closed/2`'s error clause without being able to detect it
+breaking, and this file claimed the clause was uncovered while the report showed
+it hit; both halves were wrong in the same place. Telling them apart needs a
+handle whose **write succeeds and whose close does not**, which is a process:
+`File.io_device/0` is `pid | file_descriptor`, so a pid is within what `fill/3`
+accepts, and `:file.write/2` sends a pid an io request while `File.close/1` sends
+it a `:file_request` — two protocols, so one can answer and the other refuse.
+That test asserts the close's own reason and that the file was **not** narrowed,
+which is the "mode only once both have succeeded" half of the contract. A
+`chmod` that cannot land is the third step, reached by removing the name.
+
 `secure_dir/1` is public so that the `mkdir`-to-`chmod` window can be stood in:
 a test creates a directory at 0777, plants a `sys.config` symlink inside it, and
 asserts that securing it is refused, the directory removed and what the symlink
@@ -1345,6 +1376,14 @@ ignore. And `Stub.calls(:which_releases) == [[]]` on the successful install is n
 load bearing twice over: it says the record check happened in the call that acted,
 *and* that the classification did not ask a second time.
 
+The other things that can hold the marker's name are planted the same way, and
+each of `File.lstat/1`'s answers has to read as a sentence: a **dangling
+symlink**, which also shows that nothing was written through it since what it
+pointed at is still not there, and a **fifo** made with `mkfifo`, which is the
+only one of the `:other` types a test can create without privileges or a mode.
+That last one is why `describe_type/1` has an `:other` clause — the catch-all
+put "a other" into a refusal that ships.
+
 **The attempt-ownership tests are about states that only exist while
 `install_release/1` is in flight**, so `Castle.ReleaseHandlerStub` accepts a
 *function* as a reply and calls it with the arguments. That is the only seam
@@ -1487,6 +1526,114 @@ underneath it, and answers with the new value. What lets it is that materialisin
 leaves no `config_provider_booted` marker behind and preserves the header Mix
 wrote, so Elixir's pipeline is still armed in the file the launcher reads.
 
+### What `mix test --cover` measures
+
+`mix test --cover` measures `lib`, and `test_coverage` in `mix.exs` names the
+seven `test/support` modules out of it. A fixture is covered by being run at
+all, so counting them moved the total without their ever being the thing
+measured — and two of them moved it *down* for a reason that is not about the
+suite. `Castle.PeerProviderStub` sat at 4.17% and `Castle.IoSink` at 76% while
+being exercised by every peer test, because they execute in the peer's VM. The
+list is module names rather than a pattern: a regex over `Stub` would swallow a
+production module spelled that way, which is the one thing an exclusion must not
+do.
+
+**It is part of `mix precommit`**, which is what makes the threshold a gate
+rather than a number in a comment. Nothing else runs it, and CI's `test` matrix
+stays on a plain `mix test` deliberately, because cover's line attribution can
+differ between Elixir versions and the figure is measured on one toolchain — the
+pinned `precommit` job is where it is enforced.
+
+Lib-only that is **88.58%** — 419 of 473 relevant lines. Per module:
+`Castle.Commands` 94.85%, `Castle` 90.48%, `Castle.Peer` 81.22%,
+`Castle.Deployment` and `Castle.Error` 100%.
+
+**The threshold is the measured figure, 88.58, and not a rounder number near
+it.** One uncovered line added to `lib` gives 88.40% and fails; that was
+measured rather than assumed. It replaced an 85 that sat *below* the figure it
+was meant to floor, so it ratcheted nothing and licensed a thirteen-line
+regression. The cost of a threshold with no slack is that a refactor which
+legitimately removes covered lines fails it too, and the answer then is to
+re-measure and edit the number deliberately — the same rule every other claim
+here is held to.
+
+**What cannot be measured is the peer's VM, and the reason is where
+instrumentation is applied — not anything cover is unable to do.** An earlier
+version of this file said the peer "has no node name and `is_alive() == false`,
+so `cover` cannot be started on it", and that is false: `:cover.start/0` works
+in a VM with no distribution. What actually happens is that Mix starts cover on
+*this* node and instruments the modules loaded here, while the peer is a
+separate VM loading `Castle.Peer` from the target release's own beam files,
+which nothing has instrumented. Cover's only mechanism for reaching another VM
+is `:cover.start/1` over a *distributed* node, and this peer deliberately has
+none. So `resolve/1` and everything below the `## In the peer` comment — 33
+lines, about 7% of the shipped total — runs on every `Castle.PeerTest` and is
+counted as missed, which puts the observable ceiling near 93%.
+
+Do not raise it by calling those functions on the test node: that runs Elixir's
+pipeline in the very VM the whole mechanism exists to keep it out of, mutates
+this node's `:elixir` application environment, and asserts less than the peer
+tests already do. Splitting them into a module of their own to exclude it is
+worse — `{Castle.Peer, :resolve, 1}` is a contract with the *next* version of
+Castle, so the MFA is not free to move for a metric.
+
+**Why 90% is not the threshold, stated as arithmetic rather than as a claim of
+impossibility.** 90% needs 426 covered, seven more than there are. Twenty-one
+missed lines are observable in principle, and they divide cleanly:
+
+* **Five are the compiler's own generated clauses**, one line per defaulted
+  arity nothing calls: `Castle.install/2` and `/3`, `Castle.Commands.install/2`
+  and `/3`, and `Castle.Commands.commit/3`. These *are* hittable — calling the
+  intermediate arities was measured to cover all five and to take the total to
+  89.64% — and that is exactly the problem. Each is a delegation whose defaults
+  are a subset of an arity that is already called, so `Castle.install/1` in
+  `castle_test.exs` already establishes that the defaults are the real modules.
+  A case calling `install/3` would assert nothing that arity 1 and arity 4 do
+  not, and would move this number. That is the move this project does not make,
+  and it is the reason 90% is out of reach: five of the seven can only come from
+  here.
+* **Sixteen need a file mode, a device node, or a provider sabotaging Castle's
+  own working directory.** In `Castle.Commands`: `arm/4`'s three publish
+  failures (`work_dir/1` refusing, an `lstat` failing for anything but
+  `:enoent`, a staged marker that cannot be written or linked) together with
+  `unarmed/3` and `detail/1`, the message all three share — seven lines that
+  stand or fall together, and every route to them needs the releases directory
+  to stop behaving between one statement and the next. Plus
+  `armed_version/1`'s unreadable marker, and `describe_type/1`'s catch-all,
+  which only `:device` reaches now and a device node needs root to make. In
+  `Castle.Peer`: `release_file/1`'s and `empty/1`'s listing failures (the
+  second unreachable through `work_dir/1` at all, which has just created the
+  directory — reaching it means calling `secure_dir/1` on something it never
+  hands over, whose one observable effect is that Castle deletes what you
+  pointed it at); `keep/2`'s generic publish error, where `write_like/3` has
+  created the staging file immediately before, so the only `File.ln/2` failures
+  left are cross-device; `stop/1`'s rescue, which needs the control process
+  already gone and is a race no fixture settles; and the two writes at the end
+  of `expand/2`, whose only route is a config provider removing the working
+  directory while the peer runs.
+
+  A mode fixture is refused here for the reason `stub_stat/1` exists: root and
+  some filesystems ignore one, so it would only sometimes describe the state it
+  names.
+
+**Two things that were in this list and should not have been.** `publish/2`'s
+generic error needs none of the above — `File.ln/2` with a staging file that is
+not there answers `:enoent` and creates no destination — and it is now covered.
+So is `call/2`'s "answered … may carry a version of Castle that predates this
+mechanism": `SyntheticRelease.stub_castle/2` builds a castle application for the
+target alone, which is what `build/2`'s `:override` is for. Both were dismissed
+as needing fixtures they did not need. And `closed/2` was listed as uncovered
+while the report showed its error clause *hit* — executed incidentally by the
+closed-handle test, which cannot discriminate it, and now pinned properly by a
+test of its own. Read this list against a freshly regenerated `cover/`; stale
+HTML from a previous run is how the inconsistency survived.
+
+**A booted release** is the last of it, and that is Forecastle's `:e2e` suite,
+described just above. That suite is where the in-peer section is exercised
+against a real release rather than a synthetic one, where the marker is consumed
+by a real launcher, where `running/1` is polled across a real reboot, and where
+the exit statuses `bin/castle` returns are asserted. None of it is measured here.
+
 ## Known limitations
 
 - **Nothing in the release restarts it after an emulator restart, and that is

RELEASE.md

@@ -41,6 +41,9 @@ Forecastle 1.x and Elixir 1.18 or later.
   the shared Erlang installation.
 - Give actionable recovery instructions when `:release_handler` booted without
   an accepted `RELEASES` file.
+- Fix the wording when a restart install is refused because something unusual,
+  such as a named pipe, is already at the path Castle uses for its restart
+  marker. The message used to read "a other".
 - Report restart installs without raising `CaseClauseError`.
 - Return an empty release list without raising `Enum.EmptyError`.
 - Report `RELEASES` read and write errors instead of raising `MatchError`.

lib/castle/commands.ex

@@ -1260,8 +1260,16 @@ defmodule Castle.Commands do
       "Move whatever is there out of the way."
   end
 
+  # Four of `File.lstat/1`'s five types reach here - `:regular` is the pending
+  # marker and is answered above - and each has to read as a noun phrase in that
+  # sentence. `:other` is the awkward one: a named pipe, a socket, anything the
+  # emulator has no name for, and it went into the shipped message as "a other",
+  # which is what a `"a #{type}"` catch-all does with the one type whose atom is
+  # not a noun. The catch-all stays for `:device` and for whatever OTP adds,
+  # since "a device" reads correctly.
   defp describe_type(:directory), do: "a directory"
   defp describe_type(:symlink), do: "a symbolic link"
+  defp describe_type(:other), do: "something of another kind"
   defp describe_type(other), do: "a #{other}"
 
   defp stale(provisional, vsn, reason, refusal) do

mix.exs

@@ -16,6 +16,7 @@ defmodule Castle.MixProject do
       aliases: aliases(),
       package: package(),
       docs: docs(),
+      test_coverage: test_coverage(),
       source_url: @source_url
     ]
   end
@@ -38,6 +39,67 @@ defmodule Castle.MixProject do
   defp elixirc_paths(:test), do: ["lib", "test/support"]
   defp elixirc_paths(_env), do: ["lib"]
 
+  # `mix test --cover` measures the shipped code, which is `lib` - the modules
+  # under `test/support` are fixtures, and a fixture is covered by being run at
+  # all. Left in, they moved the total without ever being the thing measured, and
+  # two of them moved it *down* for a reason that is not about tests:
+  # `Castle.PeerProviderStub` and most of `Castle.IoSink` execute inside the peer
+  # VM, which nothing instruments - see the threshold below. Their code genuinely
+  # runs and genuinely cannot be observed from here, so the figure they
+  # contributed was an artefact of where they run rather than a gap in the suite,
+  # and raising it would have meant calling them directly on the test node, which
+  # tests nothing.
+  #
+  # Named module by module rather than matched by a pattern. A regex over `Stub`
+  # or over `Castle.*Release` would quietly swallow a production module that
+  # happened to be spelled that way, which is the one thing an exclusion list
+  # must not do. Renaming a fixture makes the total drop, which is visible.
+  #
+  # **The threshold is the measured figure and nothing rounder.** 419 of 473
+  # relevant lines, which is 88.58%; a single uncovered line added to `lib` takes
+  # it to 88.37% and fails. It is set from a measurement rather than chosen, so a
+  # refactor that legitimately removes covered lines will fail it too - and the
+  # right answer then is to re-measure and edit this number deliberately, which
+  # is the same rule every other claim in this project is held to. It replaced an
+  # 85 that sat *below* the figure it was meant to floor and so licensed a
+  # thirteen-line regression.
+  #
+  # 90% would need 426 covered, seven more than there are. What is left is 33
+  # lines in the peer's VM (below) plus 21 that are observable in principle: five
+  # are the compiler's own default-argument clauses for arities nothing calls,
+  # and the other sixteen need a file mode, a device node, or a config provider
+  # sabotaging Castle's working directory. So the seven would have to include all
+  # five of the default-argument clauses, whose only effect is on this number.
+  # That is the move this project does not make. See AGENTS.md for the line-by-
+  # line account.
+  #
+  # **What cannot be measured is the peer's VM, and the reason is where
+  # instrumentation is applied rather than anything cover cannot do.**
+  # `:cover.start/0` works perfectly well in a VM with no node name -
+  # `is_alive() == false` is no obstacle to it. What happens here is that Mix
+  # starts cover on *this* node and instruments the modules loaded here; the peer
+  # is a separate VM that loads `Castle.Peer` from the target release's own beam
+  # files on disk, which nothing has instrumented. Cover's only mechanism for
+  # another VM is `:cover.start/1` over a *distributed* node, and this peer
+  # deliberately has no distribution at all. So `resolve/1` and everything below
+  # the `## In the peer` comment - 33 lines, about 7% of the shipped total - run
+  # on every `Castle.PeerTest` and are counted as missed, which puts the
+  # observable ceiling near 93%.
+  defp test_coverage do
+    [
+      summary: [threshold: 88.58],
+      ignore_modules: [
+        Castle.DeploymentStub,
+        Castle.InitStub,
+        Castle.IoSink,
+        Castle.PeerProviderStub,
+        Castle.PeerStub,
+        Castle.ReleaseHandlerStub,
+        Castle.SyntheticRelease
+      ]
+    ]
+  end
+
   # Run "mix help deps" to learn about dependencies.
   defp deps do
     [
@@ -59,7 +121,13 @@ defmodule Castle.MixProject do
         "deps.unlock --unused",
         "format",
         "credo --strict",
-        "test"
+        # With `--cover`, so the threshold in `test_coverage/0` is a gate rather
+        # than decoration: nothing else runs it, and a floor nothing enforces is
+        # a number in a comment. This is the only place it is enforced - the CI
+        # `test` matrix stays on a plain `mix test`, deliberately, because line
+        # attribution can differ between Elixir versions and the figure is
+        # measured on one toolchain.
+        "test --cover"
       ]
     ]
   end