Skip to content

2 September 2026

57 changes:

  • A caller of OutboundUrl::fetch() could not tell a 404 from a 200 — and the framework's own caller was storing the placeholder
  • The tokenactions self-repair could not run on MySQL
  • What the email second factor does when the store or the mailer fails
  • The class added this morning was the day's biggest coverage gap — and addRemoteImage() with it
  • Who counts as signed in for the administration area, and what a missing page answers with
  • A grouped page could not be asked for at all
  • Where the write spool buffers, and how long it keeps trying
  • Re-running create:model destroyed the model
  • The Redis operations that only Redis has
  • …and the same reconnect on the path most statements take
  • The Select2 branch of the CRUD generator
  • Revoking one device, and the cascade that is MySQL-only
  • What each DevPanel card shows when the thing behind it is missing
  • A security-notification class at 6% covered
  • The per-address rate limit had never run
  • The session upsert had never been issued against PostgreSQL
  • What a client is told about every MCP tool before it calls one
  • A round of 69 tests that moved coverage by one statement
  • route-list was tested through its parser and never through itself
  • The two scheduled commands whose execute() had no test at all
  • The scaffolders' refusals, and two things that cannot be tested the obvious way
  • MediaObject's error arms, and a dead-code finding that was not one
  • An application's own User class was never once returned in a test
  • Losing Redis would have taken the application down, not the cache
  • The Memcached counter had never run, and clear() empties more than you think
  • What the human check does when it breaks, and the top three targets that cannot be reached
  • The Greek in the search box had never been tested
  • The URL cache that keeps a worker's memory bounded
  • ST_MakePoint() takes longitude first, and nothing had ever checked
  • The deadlock retry, which had never retried anything
  • The privilege boundary in the admin area, and a green run that proved nothing
  • A better index: 48 methods with no covered line at all
  • TOTP replay protection, which had never run and stands down three ways
  • The webhook credential check, and the fourth test that replaced what it tested
  • The second leg of an API login had never been taken
  • The account a client-credentials token hangs on, and a one-character invariant
  • --spa-components and the two conditions that silently decide nothing happens
  • 453 statements were being excluded from measurement by accident
  • Three small classes off the never-run list, and a test that told me it was empty
  • What auth:twofactor-cleanup actually sweeps, and the assertion total I made unreadable
  • A test called "the omnibox limit is capped" that would pass on an uncapped omnibox
  • Configuring a local asset list crashed every page build
  • An addon setting called 2fa_enabled is not called that
  • The query behind hasIndex(), and the assertion that PostgreSQL does not inherit it
  • Three counters, sessions and headers — and a setter that means the opposite of its name
  • Seven more off the never-run list, in one pass
  • Three more, and a state leak I caused and had already written the guide entry for
  • Three small ones to close the gap
  • The machine account, an MCP call's identity, and a helper the framework ships untested
  • /adminer refuses with a 404, and why that is the security decision
  • The reconnect that must forget, and the first sign-in that is not a new device
  • The middleware list, and the bracket somebody will forget
  • can() and cannot() — the pair every guard clause is written with
  • The last three: two seams and the accessor every closing test reads
  • The two gaps that were in the environment, not the tests

A caller of fetch() could not tell a 404 from a 200

ignore_errors => true is deliberate: a caller that wants to read a 404's body should be able to. What did not follow is that it should be unable to know it is one. nextHop() returns null for anything outside 300–399, fetch() then returned that response's body, and statusOf() was private with no out-parameter — so a 404, a 403 and a 200 were the same return value, a string.

The obvious answer is «check the content», and it works for most bodies. It fails on the one that matters: a CDN answering 404 with a placeholder image returns bytes that are a valid PNG, so every content check passes and somebody's grey «image not found» square is stored as the thing that was requested. That is worse than storing nothing — nothing is visible as a gap, and a placeholder looks like a result.

The status is also the only place the difference between this address is permanently wrong and this server had a bad minute lives, and that difference is what decides whether a caller forgets an address or retries it tomorrow. It cannot be read from a body.

$body = OutboundUrl::fetch($url, $maxBytes, $reason, $timeout, $maxRedirects, $status);

An out-parameter, not a response object, and ignore_errors stays. $status is initialised to 0 before anything is dialled, which matters as much as the number: a caller reading it after a refusal sees «nothing was fetched» rather than whatever the variable happened to hold. A stale value is the shape of bug an out-parameter invites.

statusOf() is public now, for the same reason nextHop() is: a caller driving its own hop loop needs it, and the alternative is every one of them writing the same three-line regex — differently, and at least one of them reading the first status line. That is the trap. A chain the stream wrapper followed itself leaves several status lines in one header block, and the one describing the response in hand is the last; reading the first classifies the final 200 of a chain as a redirect.

…and the framework's own caller was doing exactly what the report describes

MediaObject::addRemoteImage() had this bug, not just the API. It fetched, ran finfo over the bytes, and entered whatever passed into the library — so a 404 answered with a placeholder PNG became a station's logo, a product photo, an avatar. It refuses anything outside 200–299 now, with the status in the message.

How it is tested

statusOf() is pure, so seven header blocks: a plain 200, a 404 that came with a body, HTTP/2 403 with no reason phrase, a followed chain where the last line wins, no status line at all, nothing at all, and a header whose value looks like a status line.

The wiring is asserted through a refusal — that $status is left at 0 when nothing was fetched — because every address this class would fetch from is by definition outside this network, and the loopback listener a test could stand up is exactly what isPublic() refuses. The report said the same thing about its own measurement, and read the source instead.

One of this file's own assertions had to be replaced on the way: it checked fetch()'s parameter count, which broke the moment a parameter was added — a test failing about something it was never asserting. It names the parameters now.

Suite: 14,528 → 14,536 tests, wall clock 2:45.

The self-repair that could not run on MySQL

Token::updateAction() carries twenty-three statements that repair a tokenactions table older than the columns it writes: the UPDATE fails, the catch adds the missing columns and indexes, and the write is retried. None had ever executed.

That combination is why this was worth a real integration test rather than a note. The code runs once, on somebody's production database, during an API call, inside a catch — and every statement in it is DDL. There is no second chance and nobody is watching.

Two defects, and the first one means the whole MySQL branch was dead:

ADD COLUMN IF NOT EXISTS is MariaDB syntax, not MySQL. MySQL 8 rejects it outright:

You have an error in your SQL syntax … near 'IF NOT EXISTS return_status INT, ADD COLUMN IF NOT …'

So on the engine most installations run, the repair threw from inside the catch that was already handling a failure. The request failed, the audit row was lost, the table was not repaired, and the next request did it all again. CREATE INDEX IF NOT EXISTS is the same story. It asks SchemaBuilder::hasColumn() and hasIndex() per name now — dialect-correct, and what the guides say to reach for.

And the PostgreSQL trigger read servertime = 0 as a real instant. servertime is declared DEFAULT 0 in the shipped migration, so a row written without one arrives as a zero rather than a null — and IF NEW.servertime IS NOT NULL stamped every such row 1 January 1970. An audit log in which «no time was given» and «this happened at the epoch» are the same row cannot be read. Measured, not inferred: a probe inserting a row with no servertime came back with action_time = 0. The condition is > 0.

Three fixture mistakes worth writing down, because each one accused the code

This file got the wrong answer three times before it got the right one, and every time the failure pointed at Token.php:

  1. Dropping the trigger function per test. DROP FUNCTION … CASCADE in tearDown() takes the trigger with it, so the first test repaired the table and every test after it ran against a repaired table with no trigger. The time assertions then measured the column's DEFAULT CURRENT_TIMESTAMP and read as «the back-fill lost the row's time». It is tearDownAfterClass() now.
  2. Reading the columns through the cache. getColumns() caches for an hour in a shared store, so asking before the repair and again after it returned the same list both times — and the test said «the column was not added» about a column that had been. Its own docblock names this case: pass $fresh when a stale answer would be wrong rather than merely slow.
  3. The PostgreSQL lane's config replaced the parent's rather than adding to it, so it lost the table prefix and pointed at the real tokenactions — where the columns already exist and the repair never runs. Those tests passed for the wrong reason, which is precisely the failure a second lane is supposed to remove.

And the prefix is a performance fix, not tidiness

Token::updateAction() names #PREFIX#tokenactions, so without a prefix of its own this class owns the real audit table — drops it at teardown, and every later class that needs it pays to rebuild it. Measured: eight tests that run in 0.97s alone added 23 seconds to the suite. With 'prefix' => 'heal_' the whole suite is 2:37 for 14,544 tests, which is faster than it measured without these tests while the interference was there.

Suite: 14,536 → 14,544 tests, wall clock 2:37.

What the email second factor does when the store or the mailer fails

The happy paths were well covered — sixteen tests over the three limits, the single use, the attempt cap, the two purposes counted separately. The file sat at 75% because none of the failure paths had run, and for a second factor those are the ones that decide whether somebody can get in.

Each one is a catch with a deliberate direction, and the direction is the assertion:

A code that cannot be stored is not reported as sent. Returning true after a failed write tells the screen a code is on its way, so the person waits for mail that never comes — and then cannot ask again, because the resend interval says they just did.

A code that could not be mailed does not consume the allowance either. The method stores the code before mailing it, on the stated grounds that a code which arrives and cannot be verified is worse than one never sent. The consequence is that a failed mail leaves a stored code nobody has, so the accounting row is written last. Without that, a mail server having a bad minute costs the person their next two minutes as well.

An unreadable accounting log allows the send. The limit exists to prevent nuisance; refusing every code because a log table is missing would turn an absent audit table into an inability to sign in. That is a choice, and the kind that gets reversed by somebody tidying a catch unless a test says which way it goes.

The scheduled cleanup swallows a failing store, because codes expire by timestamp whether it runs or not — nothing is less safe for having skipped it, and an exception would stop whatever the scheduler had queued behind it. Asserted as a comparison rather than as «it did not throw»: a real expired row is still there afterwards, and then a working cleanup removes it.

And the reserved ids are refused at all three entry points. userid 0 and 1 are not accounts, so a code issued for one has no owner and a verification that accepted it would be a factor anybody passes. Three separate guards, so three assertions — the one that loses its guard is not the one being read.

A seam for the mailer

send() built its Notifier inline, which made the mail-failure branch unreachable from a test. It is protected function notifier() now — the same shape as the seams elsewhere in this codebase, and the smallest change that makes the branch it protects observable.

And the class was rebuilding its schema per test

Four DROPs and four migrations in setUp(), costing about four seconds a test. Nothing in the class asserts anything about the schema — the assertions are about what the service does with rows — and tearDown() already deletes by userid while every test creates a new user, so a fresh table per test was buying nothing.

Once per class now, keyed on static::class so a future lane against another engine builds its own. The class went from 4.0s to 2.1s and the suite from 3:05 back to 2:36 — the sixteen tests that were already there got faster too.

Suite: 14,544 → 14,551 tests, wall clock 2:36.

The class added this morning was the day's biggest coverage gap

OutboundUrl came out of the ranking at 58% — 69 uncovered statements, the worst percentage of any sizeable file in the framework, and every one of them mine from this morning. Worth recording as the shape of the problem rather than just fixing: guarded code is more code, and a security class whose whole job is to make a network call is a class most of which a suite that makes no network calls cannot reach.

isPublic(), nextHop(), resolveLocation() and statusOf() were already pure and already tested. What was not reachable is fetch()/fetchOnce() — and it turns out two useful pieces were buried in there rather than being genuinely un-testable.

dialledUrl($url, $address) is now its own public method. It is the one part of the fetch that can be checked without a network, and it is the part that closes the DNS-rebinding window: the same URL with the approved address in place of the name, so fopen() cannot resolve it a second time and get the private answer it was refused a moment ago. Six cases, and the IPv6 one is why it deserved extracting — an address containing colons has to be bracketed or the first colon reads as a port separator and the URL points at a host that does not exist, which surfaces as «could not connect» rather than as anything suggesting a quoting bug.

The byte cap is now readCapped($handle, …), taking a stream rather than a URL — so it can be driven against php://memory, because fread/feof do not care what kind of stream they are given. Two assertions worth having: a body past the ceiling is refused rather than truncated (half a JPEG is worse than nothing — it is the shape the caller expects, so every downstream check accepts it and the failure surfaces somewhere else), and the boundary, since the comparison is > and an off-by-one there refuses a response of precisely the size a caller allowed.

What is left uncovered is the socket call itself, and it stays that way on purpose. Every address this class will fetch from is by definition outside this network — the loopback listener a test could stand up is exactly what isPublic() refuses — so reaching it means either a real network call from the suite or a seam that lets something bypass the check. Neither is worth having.

Coverage: 93.70% → 93.81%, 736 statements from 95%. Suite 14,551 → 14,559 tests; wall clock read 2:49 without these eight and 2:59 with them, which is inside the ±20s this machine drifts between identical runs — the class itself is 0.84s for 51 tests, all in memory.

…and the same for addRemoteImage(), which had the same shape of gap

Twenty uncovered statements in addRemoteImage() and seven in detectMimeTypeOfString(), all written this morning, all unreachable for the same reason: everything interesting happens after the bytes arrive, and getting bytes meant a real outbound request.

So the request is one overridable method — fetchRemote() — and the decisions are not. Same argument as the notifier() seam in the email factor, and the same instruction in the guide: overriding it means opting out of the host check, the byte cap and the redirect checking, so do not.

What that made testable is the part worth testing:

The extension follows the bytes, not the URL. This is the defect that was fixed this morning, asserted properly now: the URL says .png, the body is a JPEG, and the stored file has to say JPEG. $ext used to come from the URL's own text with a jpg fallback, so the library held files whose name asserted a type nothing had verified — and everything under www/uploads/ is served back by the web server according to that name.

An HTML error page served with a 200 is refused. The commonest real answer to a stale image URL, from a CDN that would rather show something than nothing. Stored under an image extension it becomes a broken picture on a page; refusing turns it into a visible gap instead.

A valid PNG that arrived with a 404 is refused, which is this morning's other fix and the case that proves content-checking is not enough.

An SVG is refused for a remote fetch although addImage() accepts them — it is markup, it can carry script, and served from this site's own origin that script is same-origin.

And detectMimeTypeOfString() is checked directly, four bodies through reflection. It is what makes the type decision cheap enough to take before anything is written: finfo_file() needs a path, and the alternative is putting unidentified bytes on disk to ask about them and moving them afterwards.

Suite: 14,559 → 14,577 tests, wall clock 2:48.

Who counts as signed in for the administration area

Application::adminAreaUserIsSignedIn() had zero hits across 14,000 tests. Five statements, and they are the gate on an area whose whole purpose is to be closed.

return $user !== null
    && $user !== false
    && (int) ($user->userid ?? 0) > 1
    && \Pramnos\Http\Session::staticIsLogged();

The > 1 is the method. userid 0 and 1 are the framework's guest and system rows, and they carry a usertype like any other row — so a request judged with one of them is measured against a number it has no claim to, on the way into an area gated by exactly that number. Asserted as a list rather than one case, because the failure mode is a comparison that was > 0 or >= 1 and reads perfectly well either way.

The two conditions at the end are independent on purpose, and that has its own test: an API-authenticated request carries a real identity and no session, and this is a browser tool. A gate that accepted the identity alone would let a bearer token open the administration screens — a credential nobody issued for that.

What a missing page answers with

notFound() renders the framework's own 404, and six of its statements had never run — on a page that exists because the alternative, an empty 200, tells a crawler the URL has content and tells a person nothing.

Three assertions for three readers: the status, because an empty 200 is a soft 404 that keeps the URL in the index and spends the crawl budget on it; the noindex, belt and braces for the same reader; and the link home, for the person who otherwise has the back button and a blank page.

And the message is escaped, which is the assertion with teeth. notFound() is called with a message by routing code and by controllers, and those messages are not always constants — a controller name taken from the URL is a common one. A 404 page that renders attacker-supplied markup is reflected XSS on a URL anybody can hand out, on the one page a site is most relaxed about.

And forgetVerifiedMigrations() deletes only its own markers

Two assertions, and the second is the one worth having: a *.verified glob that had been *, or a directory that had been the parent, would delete somebody's data and still pass a test that only checked the markers were gone. So a bystander file is written beside them and has to survive.

Suite: 14,577 → 14,585 tests, wall clock 2:54.

A grouped page could not be asked for at all

Model::_getPaginated() takes a $group clause, and thirteen of its statements had never executed. Writing a test for them found out why: the branch could not work.

The primary key is forced into the select — right for a row listing, because a returned model without its key cannot be reloaded or saved — and in a grouped query a column that is neither grouped nor aggregated is invalid SQL. PostgreSQL refuses it outright:

ERROR: column "a.probeid" must appear in the GROUP BY clause or be used in an aggregate function

and MySQL refuses it too under ONLY_FULL_GROUP_BY, which has been the default since 5.7. So any $group that did not happen to name the primary key produced a query the database rejected — and «a page of categories with counts», the obvious reason to group a listing, was not available.

The fix is one condition: when the group clause does not name the key, the caller's field list is used exactly as given. Grouping by the key is untouched — that is the other real use, a join that multiplies rows grouped back down to one per parent, and there the key belongs in the select.

A grouped page returns arrays, not models, and that falls out of the same fact rather than being a restriction: a group is not a row, so there is no key to index it by. The array path already indexed sequentially, so it needed nothing.

The count is a subquery, and that is the part that would fail quietly

SELECT COUNT(*) FROM (…) as grouped_query, not count(a.key) — the latter counts rows within each group and returns one row per group, so the number read back is the size of the first group. A listing of twelve rows in three groups would report that as the total, the page arithmetic would follow it, and the last page would be empty or unreachable, on a screen that renders perfectly. Three tests: the count, the paging across groups, and that GROUP BY a.rank and a.rank mean the same thing, because the parameter is documented as a «group by statement» and both spellings arrive from real callers.

And $customGetListMethod had never run either

The hook an application uses to return something other than its own rows from a listing — a summary, a joined shape, a view model. The failure it hides is specific: the method is called on each object after the key has been used to index the array, so the keys stay ids and only the values are replaced. A listing whose keys stopped being ids is one whose edit links point at array offsets.

Suite: 14,585 → 14,593 tests, wall clock 2:45.

Where the write spool buffers, and how long it keeps trying

WriteSpoolTest covers the buffering itself thoroughly — the grouping, the retries, the poison row, the transformers — through a subclass that overrides directory(). That is the right way to give it a scratch directory, and it is exactly why the real one had never run: a test that replaces a method is not a test of it, and the coverage report says «this never executed» without saying «because the test that would have run it stubbed it out». Third time this pattern has turned up today.

So the real class gets its own file. What it decides is the difference between a row written now, a row written in a batch a second later, and a row written on another server — and the whole point of the spool is that a caller cannot tell which.

  • The directory is under var/ and is created if absent. Created rather than required, because the alternative is a framework that buffers nothing on a fresh checkout and gives no reason: every write falls through to the synchronous path, which is correct and slower, and nothing says why.
  • It is resolved once, not per row. append() is on the hot path of every request that writes an audit row, and the check behind it is two filesystem calls. Asserted by taking the directory away after the first answer — a second call that returns the same string cannot have looked.
  • A configured driver is used as configured, trimmed and lower-cased, because the value comes from a settings row or an environment file and File and file are what people write.
  • An unrecognised driver is ignored rather than used. A typo is passed nowhere that would reject it, so the spool would match none of its branches and behave however the last if left it.
  • 0 attempts means «never park», not «do not retry» — the behaviour the spool had before the limit existed. A parked row is a row nobody is looking at, so an installation that would rather keep trying should be able to say so with the obvious value. Negative values are clamped.
  • resetTransformers() clears the framework's registration flag too, and that is the half that matters: forgetting the callables without the flag leaves a process that believes they are installed and has none, so rows reach the database untransformed — columns in the wrong shape rather than an error.

One of these tests cost the suite thirty seconds

testTheDirectoryIsResolvedOnceAndThenRemembered proves the cache by removing var/spool and asking again. Leaving it removed is a process-wide side effect: every later test in the run that buffers a row pays to recreate it, and the spool is on the hot path of anything writing an audit row. 2:51 → 3:20. Putting the directory back straight after the second call is the whole fix, and the note is in the test.

Suite: 14,593 → 14,603 tests, wall clock 2:40.

Re-running create:model destroyed the model

The update path in createModel() had never executed, and the reason is the same one that made it wrong: it turns on class_exists(), and a generated model is not autoloadable in the framework's own checkout — so nothing could reach it, and nothing noticed what it did.

What it did:

if (class_exists(…) && file_exists($filename)) {
    $isUpdate = true;
    // "A model that already exists is left alone: regenerating it would
    //  discard hand-written methods."
    $updateResult = "Model already exists — left untouched.\n";
    …insert getApiList() into the existing file…
}

file_put_contents($filename, $fileContent);      // ← unconditional

The comment states the intent and the code does the opposite. The file was regenerated from the schema a few statements later, so every hand-written method, property and docblock in it went — and the command reported Model updated. Nothing in the output distinguishes that from the harmless outcome, and the loss surfaces whenever something that called the method breaks.

It also made the retrofit above it pointless: that block carefully inserts getApiList() before the last closing brace of the existing file, and the overwrite discarded the result immediately.

And this is the second version of this path to be wrong. Before the message it called updateModel(), which does not exist anywhere in the framework, so re-running create:model after adding a column — what the command is for — died with a fatal error. The fix at the time replaced the call with a string and left the overwrite in place.

The guard goes on the write, where it belongs. An update now leaves the file alone except for adding getApiList() when it is absent, which is additive and cannot lose anything.

And an elseif that could never be true

if (class_exists(…) && file_exists($filename)) { … }
elseif (class_exists(…) && file_exists($filename)) { throw … }

Identical conditions, so «Model already exists and cannot be updated» was unreachable. Removed.

Four assertions, and the first is the whole point

A hand-written method survives. Then: getApiList() is added when missing and the file still parses — asserted by running php -l over it, because inserting after the last brace instead of before it puts a method outside its class; a model that already has it is not given a second copy, which would be a fatal error rather than untidiness; and an update writes no test file and adds no registry entry.

Both engines, because the update path reads the live schema before it decides anything, and a regression that made the branch depend on that answer would pass on one and destroy a model on the other.

Suite: 14,603 → 14,611 tests, wall clock 2:36.

The Redis operations that only Redis has

StructuredOperationParityTest runs against a real server and covers the hashes, the lists and the counters — everything the adapters must agree about. What a parity test structurally cannot cover is the half of RedisAdapter that exists because the backend is Redis: an atomic swap, a cursor walk over the keyspace, a pattern delete. There is no File or Array counterpart to compare them against, so nothing ever called them.

The result: every one of those methods had been entered — by RedisAdapterDegradesTest, which proves they return a safe answer with no connection — and none had ever run its body. A method whose only test is «it does nothing when switched off» is a method nobody has watched work.

swap() is GETSET, and the reason it exists rather than get-then-write is that a lock, a leader election and a «claim this job» are all «take the old value and put mine there, atomically». With a read followed by a write, two callers read the same old value and both believe they won. Asserted including the null for an unset key, separately, because Redis answers false there and passing that on would make «the key held false» and «there was no key» the same answer.

The sliding TTL is the distinction a rate limiter is built on, and the two behaviours are opposite: a fixed window expires a fixed time after the first request, so a burst is counted and forgotten; a sliding one expires that long after the last, so somebody who keeps knocking stays blocked. Choosing wrong is a limiter that either never releases or never holds. Read back from the server's own TTL rather than from anything the adapter computed — an adapter agreeing with itself is not an assertion.

keys() and the sweep behind clear() walk the keyspace with SCAN, not KEYS, because KEYS holds the whole server for the length of the sweep and the sweep is over a production cache. Both tests seed comfortably more than one SCAN step: a do/while that stopped after the first batch passes with three keys and quietly truncates with three hundred.

And clear('') is scoped to the configured prefix, which is the safety property rather than a detail. Several installations share one Redis, so a clear that swept the database would empty somebody else's sessions along with the page cache the operator asked to clear — and report success. The test puts a key outside the prefix and requires it to survive.

The class uses its own scratch database (13) and its own prefix, so its flushes cannot reach the parity test's fixtures on 11.

Suite: 14,611 → 14,618 tests, wall clock 2:45.

…and the same reconnect on the path most statements take

execute() is the prepared-statement path. runQuery(), behind query(), is the other one — every hand-written SELECT, everything the query builder compiles, every migration — and it carried the same reconnect with the same MySQL defect: the gate read mysqli_errno() after mysqli_query(), which throws rather than returning false under the error mode that has been the default since PHP 8.1. So the branch was unreachable and a lost connection came out as an uncaught mysqli_sql_exception.

PostgreSQL's half already worked here, which is worth stating precisely rather than lumping the two together: a failed pg_query marks the connection bad immediately, unlike a failed pg_execute — so isConnectionAlive() answered correctly on this path and wrongly on the other. It reads the error text now, for consistency with execute() and because the text is the more direct signal either way.

The exception is put back when the answer is «the connection is fine and your SQL is not». Without that, catching it in order to inspect it would turn every syntax error and constraint violation into a silent false, and the callers that wrap a failing statement in catch (\Exception) would stop seeing anything — a far larger change than the one being made.

The first version of these tests proved nothing, and that is the finding

They killed the connection from a second connection, exactly as the prepared-statement tests do, and passed with the reconnect broken. The reason: query() asks getConnection() first, which probes with SELECT 1 — so a connection killed before the call is replaced there and runQuery()'s reconnect never sees a dead handle.

Which means the window runQuery()'s retry actually covers is narrower than the one execute()'s covers: the probe passed, the handle was handed over, and the server went away before the statement was sent. Not contrived — that is a database restart or a failover during a request.

Staging it needed a seam: a Database subclass whose getConnection() returns the live handle and then kills that backend before handing it over. With it, the MySQL lane fails against the old code with «MySQL server has gone away» and passes against the new one. Verified in both directions, because a test that passes before and after is not evidence.

Suite: 14,618 → 14,624 tests, wall clock 2:36.

The Select2 branch of the CRUD generator

Fifteen statements in createControllerAndViewsFromWizard(), never executed, and unreachable for a reason that is itself a good decision: availability is read from the Document — isScriptRegistered('select2') — rather than by looking for www/assets/vendor/select2 on disk. A directory says a file exists; the registration says the project opted in, which is the question being asked. It also means the only way into the branch from a test is to register the script, and nothing did.

What the branch decides matters on any real schema. Without Select2 the generated edit form loads the referenced table in full, server-side, as <option>s — fine for a status lookup, and a form that takes seconds to render and megabytes to send for a foreign key to a table with thousands of rows. With Select2 the options come over AJAX from the generated fkOptions() action instead.

But the currently selected row still has to be loaded on its own, or an existing record opens with a blank field where its category used to be — and saving that form clears the reference. That is the whole content of these fifteen statements, and it is the kind of thing that looks fine in a diff and is noticed by whoever edits the second record.

A foreign key to users is the one case that cannot go through a generated model, because users is the framework's table and the application has no model for it. The generated code reaches for \Pramnos\User\User and reads username, rather than the name/title/label chain every other reference falls through — and asserting that is asserting the absence of \App\Models\Users, a class that does not exist, whose failure would arrive when somebody opened the form.

A negative assertion that could not fail

The first version of this test asserted that the Select2 output does not contain category_idOptions = — a string neither branch ever emits, so the assertion passed for free. What actually distinguishes them is $categoryList = new …; $view->categoryList = $categoryList->getList();, which is the eager load. Asserting the absence of that is asserting something.

Worth recording because it is the failure mode of negative assertions generally: assertStringNotContains on a string nobody writes is indistinguishable from a passing test, and there is nothing in a green run that points at it.

Also covered: an existing controller is refused rather than overwritten, and the file is checked to be byte-identical afterwards — the refusal is only worth anything if it happens before the write.

Suite: 14,624 → 14,627 tests, wall clock 2:36.

Revoking one device, and the cascade that is MySQL-only

SessionRevocationTest already had both engines and eleven tests, and deleteToken() was uncovered anyway — because nothing called it. The revocation tests all go through revokeOtherSessions(), which is a different method with a different question, so «sign out this one device» had never been executed.

Four things it now asserts, and three of them are about the WHERE:

  • Status 2 with a removedate, not a deleted row. A revoked token is evidence: «this credential stopped working at 14:12» is the answer to «was my account used after I signed out of that laptop», and a deleted row cannot answer it.
  • Another live token of the same account is untouched. This is what a «sign out this device» button calls, and revoking the wrong one signs somebody out of the browser they are holding.
  • A token belonging to somebody else is not revoked, whatever id is passed. The userid in the WHERE is the only thing between «revoke my device» and «revoke anybody's by guessing a number», and the ids are sequential integers that appear in URLs. Without it the method is a denial of service against any account, from an authenticated request of your own.
  • The parentToken cascade is MySQL-only, and the test says which engine does what rather than asserting one and skipping the other. An asymmetry nobody has written down is one somebody later «fixes» in whichever direction they happened to read. Cascading is the behaviour you want — parentToken is how a refreshed credential remembers the one it replaced, so revoking what a device holds should take the refresh chain, or the device renews itself back in.

And revokeOtherSessions() sparing the token the current request holds — read from $_SESSION['usertoken'], because the session is the only place that knows which credential this request arrived with. «Sign out everywhere else» that signs the caller out too is the bug a user notices immediately and cannot work around: they press it, they are ejected, and the conclusion is that the button is broken rather than that it worked.

The fixture, discovered one refusal at a time

Writing a token row directly needs every NOT NULL column that has no default, and the first two attempts found them by running the test and reading the error: deviceinfo, then scope. Taking them from the shipped migration instead is both faster and the better fixture — a column added there should break this loudly rather than being silently written as an empty string.

Suite: 14,627 → 14,635 tests, wall clock 2:39.

What each DevPanel card shows when the thing behind it is missing

This is the panel somebody opens because something is wrong, on an installation that may not have finished setting itself up — no queue table, no write spool, no scheduler definitions, no git. Every card therefore carries a catch, and none of those catches had ever run.

The failure they prevent is specific: a panel that throws is a panel that cannot be opened, so the tool for diagnosing a broken installation is the tool a broken installation takes away.

And the shape of the degraded answer matters as much as not throwing. Em-dashes, not zeros. «0 pending jobs» and «I could not read the queue» are different facts and only one of them means the queue is healthy — an operator on this panel because jobs are not running would read a zero as «the queue is empty, so the problem is upstream» and go looking in the wrong place. The failure is also reported, so the em-dash is explained rather than mysterious.

The background-work card degrades one half at a time: two independent try blocks, because the write spool and the scheduler are different subsystems and one being absent says nothing about the other. A single try around both would make a missing scheduler definition hide the spool's queue depth, which is the number somebody came to read.

detectRepoRoot() resolves in three steps — the application's ROOT if it is a checkout, the framework's source root if that is, the working directory otherwise — and the order matters: an application vendoring the framework has two .git directories, and the git card is supposed to describe the application's history. The last step is a fallback rather than an answer, which is why it must never be empty: a git command run in '' runs wherever the process happens to be.

Two things this test got wrong first

It asserted its own stub. panelError() is private, so the subclass's «override» declared a new method and recorded nothing — the assertion passed against a stub the code never called. It reads what panelErrorsHtml() renders now, which is both the real recording and the thing an operator sees.

And readProcUptime(), readProcLoadAvg() and readProcMemInfo() are left uncovered on purpose. Each carries a one-line fallback for a host with no /proc; inside a Linux container that branch is unreachable, and a seam for one return '—' is more machinery than the line it would test. Written down in the class rather than worked around.

Suite: 14,635 → 14,640 tests. Wall clock read 3:16, and the control run without these five read 3:22 — the band has moved with the machine, not with the tests, which take 0.75s.

A security-notification class at 6% covered

SecurityChangeNotifier — one method, and essentially none of it had executed. What it decides is the only signal the owner of a stolen account ever gets, so this is the highest-value 29 statements left in the framework rather than merely the lowest percentage.

The previous address is the whole class. A stolen session's first two moves are to change the email address and then the password. Every notification after the first goes to the attacker's address, so «we told the account» is worthless — the account is theirs. Mailing the address that was on the record is the one message the owner receives, and it arrives while the situation is still recoverable.

Asserted by name and by count: two sends, and the old address among them. A test that checked only the return value could not tell one send from two, which is the entire difference.

Four more decisions, each a way to lose the signal while appearing to send it:

  • the old address is skipped when it matches the current one case-insensitively and after trimming — an address that changed only in capitalisation has not changed, and two identical mails about one event teach the recipient to ignore both;
  • it is skipped when it is not an address, because it arrives from whatever the account held before and handing that to a transactional provider is a reputation charge to the sender;
  • userid 0 and 1 are refused before a user is loaded — a notice «about» the guest or system row is a mail to whatever address those rows carry, reporting a change to an account nobody owns;
  • and a failed send is swallowed, because a notification is never worth failing the change it reports: somebody told «your password could not be updated» when a mail server was down will try again, and the second attempt on an account whose password did change is what actually goes wrong.

A seam, for the third time today

new Notifier() inline made every branch unobservable — «which addresses were mailed» is a question about the notifiables that were sent to, and nothing recorded them. protected static function notifier(), reached through static:: so a subclass can answer it, which is the only reason it is not private.

That is the same change as EmailSecondFactor::notifier() and MediaObject::fetchRemote() this morning, and the pattern is worth naming: a collaborator constructed inline inside the branch you care about is a branch nobody can watch. Three classes, three days of «it has always worked», none of them observed.

Suite: 14,640 → 14,654 tests, wall clock 2:39.

The per-address rate limit had never run

Nine statements in LoginFlow::attempt(), and they are the whole of ip_rate_limit — a setting the security guide documents and nothing had ever exercised.

The order is the security property. The address is checked before the credentials are, and the method's own comment says why: a limited address must not even learn whether the password it sent was right. A limiter that verified first and refused second would still stop brute force and would also be a perfectly good oracle for credential stuffing — the attacker reads the difference between «locked» after a real password and «locked» after a wrong one, or just the timing.

Which is why the assertion is the absence of a credential check. An ordering cannot be seen any other way: both orders return locked, and only one of them keeps the secret.

And it is a second counter, not a wider one. The account lockout stops somebody guessing one password; the address limit stops somebody trying one password against ten thousand accounts, which no per-account counter can see because each account records exactly one failure. The configured window and threshold travel with the record, so the test asserts [['ip', '203.0.113.10', 300, 7]] rather than merely that something was recorded — a limiter reading a default window would silently ignore the configuration it was handed.

The off case is asserted too, and completely: with no limit configured the address is neither consulted nor recorded. A flow that still read the status would pay a lookup per sign-in for a feature nobody switched on, and one that still recorded would accumulate rows an installation never asked for and cannot see.

And an unreadable usertype is 0, not privileged

usertypeOf() is a seam whose real body had never run, and its catch returning 0 is the whole point: the value decides whether an account is held to the administrator rule that demands a second factor. A read that keeps failing would otherwise demand a factor of every account nobody can confirm is an administrator — on every login, for ever, with the reason invisible. 0 is also the safe direction: it under-privileges, so a failure cannot promote anybody.

The double had to learn the difference between two scopes

FakeLockout answered every getLockoutStatus() with one value, which is fine for the account lockout and cannot express «this address is limited but this account is not» — the exact distinction the IP path exists to make. Per-scope answers now, with the single value kept as the fallback so the twenty-odd existing tests are untouched.

Suite: 14,654 → 14,658 tests, wall clock 2:44.

The session upsert had never been issued against PostgreSQL

SessionTrackingCostTest covers the decision whether to write and how often, with no database behind it. What had never run is the statement the middleware exists to issue — and it is written twice, once per dialect.

The PostgreSQL half is the one that matters: ON CONFLICT (visitorid) DO UPDATE … RETURNING logout is hand-written dialect SQL, so a mistake in it is not a wrong number but a silent nothing. Tracking would fail on every PostgreSQL installation, the active-visitor list would be empty, and the only symptom is an absence.

RETURNING is a feature rather than a flourish, and it has its own test: the upsert deliberately does not touch logout, and reads back what it was. That flag is how «an administrator ended this session» reaches the visitor — somebody else sets it in the table, and the visitor's next request finds it. An upsert writing every column would clear the instruction before it was read, and the eject button would silently do nothing. Both lanes assert the same outcome, because the outcome is the contract and the mechanism is not.

The sweep is forced on rather than left to chance: it is one-in-session_gc_divisor by design, so a test that took the odds would pass ninety-nine times out of a hundred without running the statement it is about. And it asserts both directions — a time < comparison written the wrong way round would delete the visitors who are here and keep the ones who left.

The fixture was wrong twice, and both were mine

The cookie is namespaced. cookieget() reads $_COOKIE[substr(md5('pcms'), 0, 10)][str_rot13($name)], so a plain $_COOKIE['visitorid'] is invisible: the middleware minted its own id, wrote a perfectly good row under it, and the test looked up an id nothing had written — then reported «the upsert did not run».

And the id is hex, stored packed. It is generated as substr(md5(...), 0, 16), run through hex2bin() and stored base64, so the column holds twelve characters rather than sixteen. «Any unique string» is not a valid id at all — hex2bin() on one warns and returns false, base64_encode(false) is '', and the row goes in with an empty key.

The fix for both was to stop handing the middleware a fixture and let it mint the id, which is also what a first-time visitor actually looks like. $_SESSION['visitorid'] is where it says what it chose.

Worth writing down because the failure looked exactly like the bug the test was hunting: eight red tests, all saying «nothing was written», and the thing that was wrong was the lookup.

Suite: 14,658 → 14,666 tests, wall clock 2:46.

What a client is told about every MCP tool before it calls one

name(), description() and inputSchema() are the whole of tool discovery — a client reads them once and builds its calls from the schema. Every existing test of these tools calls execute() directly with an array it wrote itself, which is the useful thing to test and also means the discovery half was never exercised: two tools had all three methods at zero hits.

The failure that hides is total rather than partial. A malformed schema is not a wrong answer — the tool disappears from the client's list, or the client sends a shape the tool does not read, and nothing on the server logs anything because the request never arrives. The same for a description: an empty one is a tool nobody calls, and a one-word one is a tool called for the wrong thing, which is worse because the answer looks like an answer.

So the test is over every registered tool rather than one at a time, because the contract belongs to the interface and a per-tool test is a test the next tool does not have. It asks the service provider for the real server and holds all seventeen to the same rules — 69 tests, 305 assertions — so a tool added tomorrow is covered the day it is registered.

What it requires: a wire-safe name, unique across the server (the server keys tools by name, so a duplicate does not collide loudly — it replaces, and a tool the provider registered simply is not there); a description long enough to choose by; a schema whose every property has a type and a description; a required list naming only properties that exist; and a schema that survives json_encode/json_decode unchanged, since that is the round trip it makes.

And the same performance trap as this morning, twice

The class cost the suite 32 seconds while running in one, for two reasons:

  • Settings::clearSettings() in setUp(). The same trap as the settings-restore fix earlier today: it empties the store including everything loaded from app/settings/settings.php, and nothing reloads that file inside a running process — so calling it sixty-nine times hands the rest of the suite an installation with no settings, over and over. setSetting(…, false) writes the in-memory value and leaves the rest standing.
  • A data provider that booted the service provider per test method. Booting reads the filesystem, and four data-provider methods plus the duplicate-name test meant five full boots for a list that cannot change between them. Cached in a static.

2:42 control → 3:15 → 2:44 with the fixes. Worth recording because the class is pure and in-memory: the cost was entirely in what it left behind for everything after it.

Suite: 14,666 → 14,735 tests, wall clock 2:44. Coverage 93.98% → 94.05% before this round.

A round of 69 tests that moved coverage by one statement

Worth its own entry, because the cause is a thing I knew and applied backwards.

#[CoversClass] restricts what a test contributes to the coverage report. The tool-discovery test declared #[CoversClass(McpServiceProvider::class)] — true of what it asks for the server, and false of what it actually exercises, which is seventeen tool classes. So every line of tool code the test ran was discarded: 69 tests, 305 assertions, and 94.05% → 94.05%, with the three methods the class was written for still sitting at zero hits in the report.

Nothing about that is visible from a green run. The tests pass, the assertions are real, the code does execute — and the report says it did not, because the report was told to look elsewhere.

The fix is to declare nothing. The subject is a contract across many classes rather than one class, so naming all seventeen would work and would be wrong for the same reason the test is written the way it is: a tool added tomorrow would silently stop counting.

I also audited the other twelve test classes added today for the same mistake. All twelve declare the class they actually test, so their coverage counted — this was one file, and it was the one file whose subject was not a class.

The general rule, since it has now cost a full round: #[CoversClass] is a filter, not a label. If a test deliberately exercises code outside the class it names, the attribute is the wrong tool and the loss is silent.

route-list was tested through its parser and never through itself

Thirteen tests for the routes-file parser, all green, all reaching it by ReflectionMethod. Which is a reasonable way to test a parser and is also why execute() had never run: finding the candidate files, reading them, stamping each route with its file, filtering while reading, and choosing between the two answer shapes were untouched by every one of the thirteen. The report said 87%.

Six tests now go in through execute() against real files, with projectRoot()protected, for this — pointed at a directory the test owns. They wrote down two behaviours that were correct and unasserted:

  • with routes on disk and no attribute controllers, which is every console application and the console is the only kernel that reaches this tool, the answer is the keyed report and never the flat list;
  • the keyed report is in discovery order and only the combined answer is sorted by URI, because only it merges two sources with no natural order between them.

I kept both rather than making the orders uniform: reading a single file back in the order it was written is the more useful of the two, and the sort exists on the other path for a reason.

The guide gets the general form — test the private method for the algorithm and the public one for the wiring, because if every test of a class reaches past its entry point then the entry point is untested whatever the percentage says.

The two scheduled commands whose execute() had no test at all

auth:token-cleanup measured 34% covered and messages:dispatch 30%, and in both cases the covered part was looksLikeMissingTable() — four str_contains calls, reached by reflection — while execute(), which is the part with the decisions in it, had never run.

Three decisions, and the schedule depends on all three: a missing table is a success, because both features are optional and the schedule runs everywhere; any other error is a failure that says what it was; and nothing due is quiet, because messages:dispatch runs every few minutes and a line every run is a line nobody reads.

None of the three was reachable. User::cleanupAllAuthTokens() is a static call and the dispatcher was a new inside the try — neither can be made to throw from a test. So both got the seam this codebase keeps arriving at: retire() and dispatcher(), one line each, protected, for the same reason as EmailSecondFactor::notifier() and the rest — a collaborator constructed inline inside the branch you care about is a branch nobody can watch.

Fourteen unit tests over the arms, and one integration test that runs the retirement against a real table on both lanes, because the existing test of cleanupAllAuthTokens() asserts the shape of the SQL against a recording fake and nobody had checked that the statement retires the right rows. The predicate is a conjunction — created < cutoff and lastused < cutoff — so a token issued a year ago and used this morning survives, and one issued a year ago and never used at all does not, lastused being 0. Both now asserted, on MySQL/MariaDB and PostgreSQL/TimescaleDB.

And a 44-second regression that was not there

Worth its own note, because I nearly reverted a good test file over it. The suite read 3:20 against a 2:36 baseline; the control run with the new tests moved out of the tree read 3:11. Two more runs read 2:38 and 2:36, and the final tree reads 2:36 and 2:37.

The first reading was taken immediately after a --coverage run, which writes the HTML report — the machine had not finished. One wall-clock reading is not a measurement, and a control run taken inside the noise confirms whatever you already believe. The performance guide gets the rule: two agreeing readings, and never compare across a coverage run.

The scaffolders' refusals, and two things that cannot be tested the obvious way

MakeCommandBase is 4,254 lines with 94 uncovered statements, and the happy paths all have tests. What had none was every refusal — the name that is not a class name, the path that is already occupied, the write that fails — five methods repeating the same three guards, none ever executed. Which is the wrong half to leave alone: create:model used to overwrite an existing model and report success either way, and that is precisely a refusal nobody had run.

Fifteen tests now cover them. Two of the three guards were straightforward. The third produced the findings:

A write cannot be made to fail here. Putting a directory where the file belongs does not work, because file_exists() is true for a directory and every one of these methods checks "already there" first — so the directory trips the guard above the write. chmod 000 does not stop root, which is what the container runs as. Making the parent a file works only while the parent does not exist, and src/Middleware and its three siblings exist in any tree the suite has run in once. That leaves a read-only mount, a full disk or a quota. The guards stay and now carry @codeCoverageIgnoreStart with that reason written beside them, like Database::prepareInput()'s missing-extension branch — and the guide gets the rule that the comment is the point: without it, an unreachable branch is indistinguishable from one nobody got round to.

A test double's applicationInfo cannot be a property default. Application::__construct() overwrites it from APP_PATH/app.php, so a subclass declaring public $applicationInfo = [] comes back holding the fixture's namespace. That is why four : 'App' fallback lines had never run: every test that reached them supplied a namespace without meaning to, and the unconfigured-project case was unreachable by construction rather than by neglect.

One run in the middle of this reported a single failure I did not capture before re-running, and six runs since have been clean. Rather than call it fixed I made the class self-healing: it clears its own leftovers in setUp(), because it deliberately puts directories where files belong in tree locations the other scaffolding tests also write to, and a run interrupted between arrange and teardown would otherwise become somebody else's failure tomorrow.

Suite 2:35 and 2:37, two agreeing readings, inside the band.

MediaObject's error arms, and a dead-code finding that was not one

79 uncovered statements in eight clusters, all of them arms that only run when something has gone wrong. Four now run.

The finding I got wrong first. addImage() wraps copy() in catch (\Exception), and copy() does not throw — it warns and returns false. So I read it as the same shape as the mysqli === false branch found earlier and rewrote it as a return-value check. It broke a passing test, which is how I found out why the guard is right: the test that covers it installs an error handler that turns E_WARNING into ErrorException, which extends Exception. The arm is reachable, deliberately, and @ does not stop a handler in PHP 8. Reverted, and the guide now says so before somebody else "fixes" it.

What that leaves genuinely unreachable is the unlink() guard beside it: getting there needs a copy that succeeds and a delete that fails, and as root in a container the second does not happen.

Four arms covered. An EXIF reader that raises does not fail the upload — the extension is optional and a stream wrapper can throw, and an orientation nobody could read is a cosmetic loss. A thumbnail whose file has been deleted is dropped and remade rather than returned, because the record and the file are two things and only one of them is in the database. A JPEG arriving under a .gif name is still rotated, and written back as a JPEG rather than falling through to the PNG writer. A file with no image in it is refused instead of taking the process down on imagerotate(false, …).

And the thumbnails fallback, whose documented reason is not the real one. encodeThumbnails() falls back to serialize() when json_encode() refuses the payload, and offers "a filename in some encoding json_encode() refuses" as the case. That case cannot work: a byte sequence that is not valid UTF-8 does not fit in a utf8mb4 column either, so the fallback writes a value the database rejects. What it genuinely rescues is a value PHP can serialise and JSON cannot represent — INF and NAN. Asserted with INF, through save() and a real reload, so the reader is what is being tested rather than the encoder.

Two hours of that went on a row I was sure had been lost, which turned out to be two things that had nothing to do with the codec: load() reads through a ten-minute query cache keyed on the SQL text — and in a class that recreates its tables, mediaid restarts at 1, so the cached row belongs to another test — and MediaObject declares no constructor, so new MediaObject($id) ignores the id and returns an empty object. Both are in the guide now.

Suite 2:36 and 2:36, inside the band.

An application's own User class was never once returned in a test

getUser() and getCurrentUser() each carry the same four-line lookup: if the application declares a namespace and a User class exists inside it, instantiate that one. Neither branch had ever run.

Which is worth more than the eight statements it covers. Applications subclass User to add the columns and methods their own accounts have — a tenant, a billing reference, isSubscribed() — and an application whose subclass is silently ignored gets a framework object back with none of them on it. That surfaces as Call to undefined method a long way from here, in whatever page happened to call the method.

Four tests: the override is honoured by getUser(); an application that has written no User class gets the framework's, because the condition is a class_exists() and not configuration; the same override is honoured by getCurrentUser(), which is a separate test rather than a second assertion because the two methods repeat the four lines and a fix to one would leave the other handing back framework objects for the signed-in user — the object almost every page actually holds; and a guest gets false rather than an empty User, which is truthy and would make every if (getCurrentUser()) treat a guest as signed in.

Two things had to be right for any of it to run. staticIsLogged() wants $_SESSION['logged'] and a uid above 1, 1 being the anonymous account. And the class caches by id, privately and statically, so tests asking for the same id under different namespaces get the first answer twice unless the cache is cleared between them.

No such file or directory from a database that worked under a filter

The class connected fine under --filter and errored four times in the full suite with a socket path. Not a flaky database: whichever test ran before it had left Database::getInstance() holding another lane's settings, and Factory::getDatabase() hands that object back rather than building one from the settings just loaded. Dropping the reference first is what the integration tests here already do, and the guide now says why — the symptom points at "the database is not running", and passing under a filter points at "the suite interferes with itself", when the fact is that the class never asked for its own connection.

Suite 2:37 and 2:39, inside the band.

Losing Redis would have taken the application down, not the cache

The Redis adapter wraps every call in the same guard — log the exception, return an empty value of the method's own type — and sixteen of those arms had never executed. The contract they add up to is the one that matters most about a cache: losing it degrades the application rather than stopping it.

The first test to execute one of them found that none of them worked.

\pramnos\Logs\Logger::logError(…), with a lowercase p, thirty-six times across the Cache subsystem. PHP resolves class names case-insensitively, so it works whenever the class is already loaded — and when it is not, the autoloader is asked for the literal string pramnos\Logs\Logger and Composer's PSR-4 map is keyed on Pramnos\, a case-sensitive miss. Error: Class "pramnos\Logs\Logger" not found.

Inside a catch, that is the worst place for it. The line only runs when something has already gone wrong, and it converts a handled failure into an unhandled one: every one of those sixteen arms would have raised Class not found instead of returning [], so a Redis outage became a fatal on every page that reads through the cache — which is all of them. Nobody had seen it because the arms that are covered ran in processes where the Logger was loaded already, and because it needs Redis to actually fail.

All thirty-six corrected. No behaviour change where the class was already loaded, and no load-order-dependent fatal where it was not.

The tests, and why the type of the empty value is the substance. counter() answering false instead of 0 puts false into arithmetic; hashGetAll() answering false instead of [] puts it into a foreach; hashGet() has to honour the caller's own $default, because a caller writing hashGet($k, $f, 0) has said what a miss means to it and a cache answering null has silently disagreed. Each of those is a second failure, in the caller, a step away from the one that happened. The throwing client raises from __call, so it covers every method name without naming one — including the ones added after this.

Two arms that are not exception handlers came with it. A scan() that returns false breaks the sweep: the cursor is only 0 when the walk finished, so without the break a failed scan loops for ever holding the request open — not an exception, and not caught by the guard below it. And getAllItems() skips a key it cannot read rather than abandoning the listing, which is right for a diagnostic screen: one undecodable key costs that row, not the page.

Suite 2:38 and 2:36, inside the band.

The Memcached counter had never run, and clear() empties more than you think

increment() was uncovered end to end — a documented four-path concurrency algorithm nothing had ever executed. Memcached's own increment fails when the key is absent, so creating the counter goes through add, which is atomic: two requests racing to create it produce exactly one winning add, and the loser increments what the winner made.

The third path is the one the design exists for, and the one a plausible implementation gets wrong: having lost the race, returning the amount added would discard the winner's count. A rate limiter built on that undercounts by one request per race. Each path is scripted rather than raced — a test that actually races is a test that passes most of the time.

counter() answering 0 and increment() answering false for the same unreachable server is also deliberate, and now written down: counter() is a read, and zero is truthful for a counter nobody can see, while increment() is a write and 0 would claim it happened.

And the isolation boundary nobody would guess from the name. clear() with no category has two different behaviours: with a key prefix it clears the category indexes the adapter maintains itself, and without one it calls flush() — which empties the entire server, every co-tenant's data included. Memcached cannot enumerate keys, so there is no third option. The adapter logs a line saying so before doing it, and that is the only warning anyone gets. Both branches are asserted now, the dangerous one because it is true rather than because it is desirable.

The test found one of its own mistakes on the way: it passed the prefix as the constructor's third argument, which is $persistentId. $this->prefix stayed empty, clear() flushed the whole server, and the assertion that no flush had happened failed — correctly.

Suite 2:36 and 2:36, inside the band.

What the human check does when it breaks, and the top three targets that cannot be reached

Two arms in Account, neither ever executed, and they fail in opposite directions on purpose: minting a challenge that raises renders the form without one, and verifying a submission that raises refuses it. Both are right — an exception while minting would take the sign-in page down, and a check that accepted when verification broke would be bypassable by breaking verification.

What is worth writing down is the combination, which neither comment states: with the service down the form renders and every submission is refused, because a submission with no challenge is treated as a failure. The page is up and nobody can sign in. That is fail-closed and defensible, and it also means enabling human_check on the login form puts the check on the critical path for signing in. The security guide now says so, with the two log lines to look for and the advice to enable it on register and forgot first — the forms it is actually for.

Six tests, reached through a new humanCheck() seam, because the two new \Pramnos\Security\HumanCheck() expressions sat inside the very try blocks under test. One of the six is a control: with the service working, a real challenge is minted — without it the five failure tests would pass against a seam that simply never worked.

The three largest gaps are not reachable, and that is the finding

DevPanelController (96), Database.php (96) and Application.php (77) have now all been examined rather than worked, and all three are dominated by arms the suite cannot execute:

  • DevPanelControllerpanelError() catch arms, and readers for /proc files that are always present in the container.
  • Application.php — the migration-fingerprint cache. Its APCu path is uncovered because APCu is not installed in the test image, and its file path's give-up branches are gated on defined('VAR_PATH') and defined('ROOT'), which cannot be undefined inside a running process.

The second one is a genuine test-infrastructure gap rather than a coverage curiosity: APCu is the path that runs in production — it is the reason the cache exists — and nothing has ever exercised it. Installing the extension in the test image would make about fifteen statements reachable and, more to the point, would test the code that actually runs. That is an image change rather than a test change, so it is raised here rather than done.

Suite 2:36 and 2:36, inside the band.

The Greek in the search box had never been tested

_buildSearchConditions() is what every server-side listing filters with, and seven of its statements had never run. Each one is a search behaviour somebody would notice.

A Greek word is searched without its final sigma. Γιάννης becomes %Γιάννη%, because Greek inflects the ending and a visitor types the nominative while the row holds the genitive. Only the ending — Κώστας becomes Κώστα%, not Κώτα% — and both ς and σ count, since a keyboard layout that does not switch them produces either.

On PostgreSQL a Greek term is compared through unaccent(), so Γιάννης, ΓΙΑΝΝΗΣ and Γιαννης find each other. A Latin term stays plain ILIKE and does not pay for the function call, which on a large table is the difference between using an index and not.

A numeric column is matched with =. LIKE '%9%' on an integer column returns 9, 19, 29, 90 and 1999 — a filter that reads as broken rather than broad. The type comes from Model::$columnCache, which is public and static, so this is the one rule here that depends on the model having read its schema.

And a field that is not in the field list produces no condition at all, which is a boundary rather than a convenience: the names arrive from the query string, and the list is what keeps a request parameter out of the statement. That one is asserted with ' OR 1=1 --' as the value.

Twelve tests, reached by reflection, and this is the split the guide prescribes rather than a shortcut: the wiring — _getPaginated() calling this and running the query — has tests already, and the algorithm had none. What comes out is a SQL fragment, so a test that went through a real query would be asserting the database's opinion of the fragment rather than the fragment. The database is a stand-in for the same reason, which also lets both dialects be asserted in one run.

The method returns the AND-joined string, not the array of conditions it assembles — which the first version of the test got wrong, in ten places at once.

Two more targets that the suite cannot reach

Model::currentChangeSource() decides whether a change is attributed to the API or the web, and its first line is if (PHP_SAPI === 'cli' || defined('STDIN')). Under PHPUnit both are true, so the two branches that classify a request are unreachable from a CLI suite by construction. Noted rather than worked around: the CLI guard is correct, and refactoring a correct guard to make a test possible is the wrong trade.

Suite 2:37 and 2:38, inside the band.

The URL cache that keeps a worker's memory bounded

Token::urlId() resolves a request URL to a row in the deduplicated urls registry, and it is called from the spool drain — a process that does not restart. So the cache in front of it is the design rather than an optimisation, and the bound on that cache is the part that had never run.

A site that puts an id in the path or a search term in the query string generates URLs without limit. When the cache reaches URL_CACHE_LIMIT, the oldest half is dropped rather than all of it, and the assertion that matters is which half survives: the most recently used, because on a worker those are the URLs it is currently busy with. Clearing everything would make the next minute of work re-resolve exactly what it had just learned, precisely when it is busiest. Both halves of that are asserted — the newest entry is present, the oldest is gone — because a test that only counted the survivors would pass on an eviction that kept the wrong ones.

Eleven other assertions came with it, on both lanes: an unknown URL is registered once and its id returned; the same URL resolves from memory the second time; a URL another process registered is found rather than duplicated, which is what makes ids stable enough for a report to group on; and an unreachable registry resolves to 0 so the drain still writes its actions — losing the URL of an action is a gap in a report, losing the batch is a gap in the audit trail.

The eviction is driven through rememberUrlId() rather than urlId(): filling the cache means two thousand entries, and two thousand round trips to prove an array_slice would be a slow test of nothing. Everything that touches the registry goes through urlId() against the real table, on MySQL/MariaDB and PostgreSQL/TimescaleDB — where the returned id comes from a sequence rather than LAST_INSERT_ID(), so "the id I was given is the id in the table" is a claim about two mechanisms.

The guide also now records the consequence for anyone reading the table: a query string is not part of the key, so /orders?id=1 and /orders?id=2 are one row. Deliberate — the alternative gave a busy page a new row per call — and not something a report can work around from this table alone.

94.47%, 330 statements from the target. Suite 2:37 and 2:34, inside the band.

ST_MakePoint() takes longitude first, and nothing had ever checked

insertDataToTable() and updateTableData() each convert a 'geometry' field into PostGIS calls, and neither copy had ever run. The conversion accepts three shapes — a "lat, lon" string, a named ['latitude' => …, 'longitude' => …] array, and anything else as WKT — and the one thing it must get right is the coordinate order.

ST_MakePoint(23.7275, 37.9838) is Athens. ST_MakePoint(37.9838, 23.7275) is a spot in the Indian Ocean. Nothing errors either way, every insert succeeds, and the map is wrong — the kind of mistake that survives a review and is reported by a customer. A test asserting "a point was written" would pass on both, so all four assertions name the numbers in order.

Eight tests. The two shapes that carry latitude first, on insert and on update separately — separate because each method has its own copy, and a correction applied to one would leave every edit writing points the other way round, so a row inserted correctly would move when someone changed it. Negative coordinates in either position, because the western and southern hemispheres are half the world and a pattern that rejected a leading minus would fall through to ST_GeomFromText() and hand it something that is not WKT. 0, 0, because the origin is a real place and a pattern requiring a decimal point would miss it. WKT passed through, which is what lets the column hold a polygon. And MySQL emitting no PostGIS call at all, since the gate is on the driver rather than the column type.

Asserted on the generated SQL rather than through a real insert: the conversion is string construction, PostGIS is installed on neither test backend, and a round trip would prove the server accepted the call rather than that the call says what it should. The recording happens in execute(), because the $debug flag these methods offer echoes the result of insert() rather than the query — QueryBuilder::insert() compiles and runs in one call, so there is no point at which the debug echo can see the SQL.

Suite 2:36 and 2:37, inside the band.

The deadlock retry, which had never retried anything

runQuery() and execute() each carry the same loop: a failure whose message mentions a deadlock is retried three times, waiting 100ms, 200ms and 300ms, before the error is surfaced. It exists for SQLSTATE 40P01, which TimescaleDB produces when a background worker holds an advisory lock during DDL — transient, and the right answer is to wait rather than lose the write.

Neither copy had ever executed, which matters more than the six statements: a retry loop that has never run is as likely to spin for ever as to work. Both continues restart a while whose exit depends on a counter reaching zero, and nothing had ever watched it reach it.

So the assertions are the elapsed time, with a bound at both ends. At least 550ms, because a loop that retried without waiting would hammer the lock holder and be indistinguishable from no retry at all by any other measurement. And under five seconds, because a counter that never reaches zero also satisfies "took at least 600ms" — for ever. Two controls with it: an ordinary error is surfaced in under 200ms on both paths, which is what says the gate is a gate rather than four attempts at everything.

Triggered with RAISE EXCEPTION 'deadlock detected', since the gate is stripos($error, 'deadlock') on the driver's message. Deterministic, and it also shows the gate's looseness, now recorded: any error whose text mentions a deadlock is retried, including one an application raised itself. PostgreSQL only — the loop sits in the pg_query branch, and MySQL reports a deadlock as an exception rather than a false, so the same condition takes an entirely different path there.

And a performance regression I caused and then fixed

Opening a connection per test took the suite to 2:42, twice — four PostgreSQL handshakes cost about as much as the 1.2 seconds of back-off the tests exist to observe. One shared connection for the class brought it to 2:38 and 2:40. The condition for sharing is that nothing in the class writes anything, which holds here: these tests only provoke errors.

What is left is the usleep() the code itself performs, and there is no way to observe a back-off without waiting for it. So this round costs the suite about a second and a half, on purpose, and says so.

Suite 2:38 and 2:40.

The privilege boundary in the admin area, and a green run that proved nothing

UsersController::save() carries two authorisation rules and neither had ever executed:

  • the cap — nobody can assign a usertype higher than their own, and it clamps rather than refuses, so an administrator who over-reaches gets an account at their own level;
  • the edit refusal — an account of higher privilege cannot be edited at all.

The second is needed in addition to the first, which is worth spelling out: without it a junior administrator could not raise anybody, but could still rename a senior account, deactivate it, or change its email address — and therefore where its password reset goes. The cap protects the privilege column; the refusal protects the account.

Six tests, and all six passed first time, so this round confirms a boundary rather than finding a hole. Two of them are controls rather than decoration: a privilege at or below the caller's own is written unchanged, because a cap that clamped everything to zero would satisfy the first test while making the admin area unable to grant anything; and an account at the same privilege is editable, because the boundary is strictly higher — peers who could not edit each other would leave an installation unable to fix its own administrators.

With them, the two answers resetpassword() gives that are not an email: an account that is not there, and an account with no address — ordinary for one an administrator created by hand, and worth saying rather than calling a mailer with an empty recipient.

The mistake worth more than the tests

The seven methods were appended to the end of the file and landed inside UsersProbe, the helper subclass that sits after the TestCase. A test method on a class that is not a TestCase is just a method. --filter UsersControllerTest reported OK (62 tests, 183 assertions) and I read that as "my tests pass"; it said nothing about them. The full suite is what caught it, and only by its count — 14,853 before and 14,853 after, identical assertion totals.

Now in the guide, because the shape generalises: a green filtered run is not evidence that a new test ran. Check that the test count moved by the number of methods you added, look at what else lives at the end of the file, and treat a new test that has never failed as undemonstrated.

94.51%, 302 statements from the target. Suite 2:38 and 2:39.

And two files left deliberately

OutboundUrl is the worst percentage in the tree at 67%, and its 57 uncovered statements are the socket path — which cannot be reached offline by design. Every address the container can talk to is private, isPublic() refuses private addresses, and every internal call is self:: rather than static::, so a test subclass cannot widen the guard either. That last part is a security property and not an oversight, so the file stays as it is.

Which leaves a genuine risk worth naming rather than burying: the class whose entire job is to fetch a URL somebody else chose has never had its fetch executed. Reaching it needs a fixture server on an address the guard accepts, or an injectable resolver — a design decision, not a coverage one.

A better index: 48 methods with no covered line at all

Per-file gaps had stopped being useful — the three largest files are all diffuse error arms — so I indexed the tree differently: methods where every statement has zero hits. Forty-eight of them, 374 statements, against 302 needed for the target. Each one is a complete piece of behaviour nobody has ever run, which is a far better queue than "the file with the most missing lines".

The top of that list is not the biggest method. It is RequireFactorEnrolmentMiddleware::mustEnrol() — the decision that forces a privileged account to enrol a second factor.

The class had tests, and the decision had never run

Its test file overrides mustEnrol() in a subclass and asserts what handle() does with a fixed answer. Which is a reasonable way to test the pipeline, and it is why twenty-two statements of security policy had no covered line: a test that replaces a method is not a test of it. Third time that shape has appeared today, and this is the most expensive place for it.

Seven tests now run the real decision, through the FactorEnrolment seam the constructor already offers — one level below the decision, so every branch of it executes. The feature is off with no floor configured, so upgrading cannot lock an installation out. A guest is not gated, because the sign-in flow must not sit behind a screen that needs signing in. An account the service names is stopped and flagged. The service is asked about the account's current id and usertype rather than the session's copy, which is how a demoted account would otherwise keep a privilege it no longer has. An account that has enrolled is let through — the control, without which a middleware that gated everybody would pass everything else here. And a path on the way out of the wall is not even asked about, because the setup screen is itself gated and a decision that ran first would send it to itself for ever.

And the row worth writing into the security guide: it fails open. A decision that raises lets the request through. That is the opposite of the human check on the sign-in form, and right for the same reason it is right there to fail closed — the cost differs. A broken human check refuses new submissions; a broken enrolment check would redirect every page of the site to a setup screen. The consequence, now documented: this wall gets accounts enrolled and is not a containment boundary. A screen that must refuse an account without a second factor has to check that itself.

This time I verified the test count moved — 14,859 to 14,866 — which is the rule the last round bought.

Suite 2:38 and 2:38.

TOTP replay protection, which had never run and stands down three ways

Second from the 48-method index, and the same shape as the first: claimCode() is what makes a six-digit code single-use, and it had no covered line. Its call site had seven hits — always with the feature off, because cache_totp_replays is not enabled by default. So the first fact worth recording is that a TOTP code is replayable for its whole ninety-second window unless an installation turns this on.

With it on, nine tests now describe it. The first presentation wins and the second is refused. The claim carries the account, because six digits and a 30-second window make two accounts producing the same code at the same moment unlikely but possible — and a key without the account would sign one of them out of their own login. A different code from the same account is a different claim, which is the converse: a key made only of the account would refuse the second attempt of anybody who mistyped the first. The key holds a hash of the code and never the code, so a cache dump or a Redis MONITOR does not hand out live second factors. And the claim expires at ninety seconds — the window plus the drift — so a code cannot be replayed anywhere it would still verify.

Then the three ways it stands down: a cache with no atomic counter, a counter that answers false, and anything that raises. All three allow the claim. Deliberate, and the reasoning is in the method — refusing every second factor while Redis is down is a larger failure than a ninety-second replay window — but it makes the protection best-effort, which is not what "single-use" sounds like. The security guide now says so, with the requirement that follows: it needs a counting cache to be worth anything.

A ten-second regression caused by swapping an adapter on a singleton

The first version reached the store by swapping the adapter on Cache::getInstance('auth'). That is process-wide, so every later test in the run inherited the state, and the suite went from 2:38 to around 2:50 with the file present against 2:49 without it — hard to read, because the machine was also having a noisy few minutes and produced 3:23, 3:03, 2:59 and 2:57 on the same tree.

The fix is the seam this codebase keeps arriving at: authCache(), one protected line, so the test owns its store and touches nothing shared. Three agreeing readings afterwards — 2:39, 2:39, 2:40 — against a band of 2:38.

Two things learned about measuring rather than about the code. Readings that disagree with each other are noise and readings that agree are a measurement, which is the rule from earlier today and it took five contradictory numbers before I applied it. And one run reported 44,152 assertions where five others reported 44,149 — something in the suite is not deterministic in its assertion count. Noted rather than chased; it is not this file, which is nine in-memory tests with fixed assertions.

The webhook credential check, and the fourth test that replaced what it tested

Webhook::requireClient() turns the credential trait's two possible answers into an HTTP response, and it had no covered line. Its own test file has eleven tests that all run against a probe overriding requireClient() — so they assert what the actions do with an answer, and the answer was the stub's. ClientCredentialsAuthTrait, where the credentials are actually read and checked, is 100% covered; the fourteen statements between it and the response were not.

Fourth time today that a test has replaced the thing it was testing. The fix is the same each time: override one level lower. Here that is the trait's two methods, which leaves the real requireClient() running.

Four tests, and the interesting one is a pair. A request with no credentials gets a description; credentials that fail to authenticate get none. That asymmetry is deliberate and worth writing into the integration guide, because it is the entire diagnostic available to somebody wiring up a client: a description means nothing arrived, no description means what arrived was wrong. Saying which half was wrong would let a client id be confirmed by trying it — the same reason a sign-in form does not say which field failed.

The other two are the ones that would catch a real hole. Both halves of the credential must reach the check, in order: a requireClient() that passed the id twice, or dropped the secret, would authenticate anybody who knew a client id — and all eleven pre-existing tests would still pass, because they only ever asserted the outcome the stub was told to give. And an authenticated client must come back as an int rather than a Response, since that is what every action branches on.

94.58%, 261 statements from the target. Suite 2:42 and 2:39, and the test count checked: 14,875 → 14,879.

The second leg of an API login had never been taken

ApiAccount::login2fa() finishes a login that stopped for a second factor, and none of it had ever executed. Every existing test drives login(), and the flow those tests build has two-factor turned off — so the second leg was unreachable from either direction.

Five tests, one per answer, because the four answers are the endpoint as far as a client is concerned. Two of them turned out to be worth more than the coverage.

400 missing_code costs no attempt. The code is checked for emptiness before the flow is asked, and the flow counts attempts towards a lockout — so without that check, a client with a bug in its form would spend its own users' attempts and lock them out of an account nobody was attacking. The test asserts the flow was not called at all, not just the status code.

The token belongs to the account the second factor verified. login2fa() answers with tokenResponse($this->userFor($result->userId)), and $result->userId comes from the flow rather than from the request. A version that answered for whoever was pending, or for an id the client sent, would issue a working bearer token for an account nobody proved they held — and every other test here would still pass. So the success test asserts the user id in the body is the one the flow named.

The rest: a POST is required, because a code in a query string ends up in access logs, browser history and every proxy in between; 429 is distinct from 401 and carries retry_after, because a client that cannot tell "wrong code" from "stop asking" turns a lockout into a loop; and a success is byte-for-byte the shape of a login that needed no second factor, so a client needs one code path rather than two. All five are now in the API guide as a table of answers.

Suite 2:39 and 2:43; count checked, 14,879 → 14,884.

The account a client-credentials token hangs on, and a one-character invariant

A client_credentials grant has no end user, and usertokens.userid is a foreign key — so each application gets a machine account of its own, created on first use and reused afterwards. systemUserId() decides which, and none of it had ever run. Two entries from the 48-method index turned out to be the same feature, so one file covers both.

The invariant is one character:

if ($this->systemuser !== null && (int) $this->systemuser > 1) {

> 1, not > 0. Zero and one are the framework's guest and system rows, so an application whose column holds either has a gap rather than an account — and a token stored under one of them sits beneath an identity shared with every other application in the same state, which makes "what has this account been doing" unanswerable. Nine tests, and the invariant is asserted on both sides of the seam: the column when it is read, and the row creation when it answers, because either route would reach the shared id.

Three more refusals, each returning 0 rather than raising: an application with no id gets no account, since assignSystemUser() refuses an appid of 0 too and a created row would be orphaned on every call; a creation that raises answers 0, because a database that cannot make a machine account is not a reason to hand a client a stack trace; and an empty client identifier is refused before anything is looked up. The reason they return rather than raise is written on the caller: the insert then fails on its foreign key, which is the honest outcome — a token for a client that cannot be resolved is not stored under a user invented for it.

One data set was dropped rather than kept: '0' as a string cannot be assigned, because the property is typed ?int. So the (int) cast in the guard is belt-and-braces against something PHP now refuses earlier — worth leaving in the code, and not something a test can reach. Recorded in the test rather than deleted silently.

The integration guide gets the operator-facing half: which ids the account is never, and the two log lines to look for when a client_credentials grant answers with a server error instead of a token.

Suite 2:37 and 2:38; count checked, 14,884 → 14,893.

--spa-components and the two conditions that silently decide nothing happens

The largest entry left in the uncovered-method index: twenty-five statements deciding what project:resync --spa-components hands a project, none of them ever executed. A poor place for it, because this is the one group in the command that writes files a project has been editing — the value of shipping a DataTable is that projects extend it.

Two conditions gate it and both fail silently, which is what the tests are really about:

  • app_style must not be mvc, and mvc is the default. Asserted with a Svelte stack configured, because that is the combination where a laxer check would write files — the stack is right and the style says there is nowhere to put them.
  • spa_stack must be svelte. A project on a no-build stack has an SPA and nothing that compiles .svelte, so the files would sit there as text and the resync would have reported success.

The count is asserted against Init::SPA_SHARED_COMPONENTS and SPA_SHARED_COMPONENT_TESTS rather than a literal, so a component added to Init without being added here fails the test instead of quietly never reaching a project — which is the failure this group exists to prevent. Also: the destination follows an explicit spa_source_dir, so a project whose front end lives in admin-ui/ is helped without renaming its directory; the rendered files carry the project's own name and API prefix, because an unrendered {{ appName }} reaching a project is a syntax error in its build and a confusing way to learn a resync went wrong; and nothing here is marked executable, which is the kind of wrongness that surfaces in somebody's git status months later.

Two of my own mistakes worth keeping. The first version left app_style unset, so every Svelte case returned nothing — the default is mvc, which is exactly the condition the test was meant to be past. And I asserted the API prefix had lost its trailing slash by checking the rendered file does not contain /api/2.0/ — but a trimmed prefix followed by a path is /api/2.0/strings. The thing a kept slash actually produces is //, so that is what the assertion looks for now.

94.62%, 233 statements from the target. Suite 2:37 twice; count checked, 14,893 → 14,899.

453 statements were being excluded from measurement by accident

I went to write tests for AuthTwoFactorStatus::privilegedAccounts() and found it already wrapped in @codeCoverageIgnoreStart — a live-DB boundary the tests override. Yet the never-run index had listed it, which means clover still held its lines. So the annotation was not working, and that was worth more than the round's tests.

php-code-coverage 11 matches these by exact string comparison, and the codebase writes them the natural way:

// @codeCoverageIgnoreStart — reached only when the socket is already closed

Nothing matches, so no Start is registered. On its own that is a harmless no-op. The damage comes from the End handler, which does range($start, $token[2]) — and never resets $start. A Start that failed to match leaves it holding the line of the last one that did, so the next End ignores everything from there to itself. Whole regions vanish from the report, and nothing warns you, because the numbers only improve.

Eighty-five broken line annotations, 121 fixed across 32 files, and 453 statements restored to the measurement. Init.php was missing 157 of its own lines, ProjectResync 122, MakeCommandBase 59 — including the ones I added myself this morning, written in the same broken style as everything around them.

The direction is the part worth stating plainly: the metric had been flattering me all day. And correcting it still moved the total up, 94.62% to 94.71%, because the accidentally-excluded code is mostly covered — those exclusions were not hiding untested code, they were hiding the size of the codebase. 178 statements from the target now, against 233 before, with 453 more statements in the denominator.

Docblock annotations are unaffected: there the check is str_contains(), so * @codeCoverageIgnore Requires a live server has always worked. The exact-match rule applies only to // comments, which is exactly the trap — the two forms look interchangeable and are not.

The guide gets the rule, the grep that audits it, and the way to verify one took effect: an ignored line is absent from clover.xml, not present with a count of zero. That check is what I should have run the first time I marked something ignored, and it is how this was found in the end.

Suite 2:39 and 2:43, identical test and assertion counts — the change is comments only.

Three small classes off the never-run list, and a test that told me it was empty

With the denominator honest, the never-run index is the queue. Three entries this round, all pure and all with something real underneath.

ThemeTokens::token() — reading one colour out of the palette from PHP, for the two places a custom property cannot reach: a chart drawn on a canvas, and an HTML email, which has no custom properties at all. Eleven statements, and what they add up to is a lookup that never throws and never returns anything the caller did not choose: an undeclared theme, an undeclared token and an empty palette all give back the fallback. Asserted as a set in one test, because the guarantee is that there is no input for which this surprises you — a chart with the wrong shade is cosmetic, an exception in a mail template is a mail nobody receives.

ReservedJob::toArray() — where the names are the behaviour. id, type, payload, attempts, run_at, because that is the shape the classic JobQueue used and worker loops written against it must keep working. So the test asserts array_keys() exactly rather than checking five keys are present: an added key is also a change to the shape. Plus that payload stays an array rather than being encoded on the way out, and that run_at carries the reserved time — filling it in at serialisation would make every delayed job due immediately and turn a retry backoff into a hot loop.

Theme::themeDirectories() — the fix for a silent defect: searching ROOT/themes alone returned [] on a project laid out the way init lays one out, which showed up as an empty theme picker with nothing in any log. Both directories are searched, a non-existent one is dropped, and no path appears twice — the last because APP_PATH and ROOT are the same directory on some layouts, and a duplicate would list every theme twice, which reads as a broken theme rather than a broken search.

That last one taught me something about my own tests. The first version passed while asserting nothing: the fixture project ships neither theme directory, so the method returned [] and two tests looped over nothing. PHPUnit said so — "This test did not perform any assertions" — and that notice is the only reason I looked. A vacuous test reports itself; a test that asserts something weak does not. The tests now create both directories, assert the result is not empty before iterating it, and remove one to prove the filter drops it.

94.71% on the corrected denominator, 178 statements from the target at the start of this round. Suite 2:42 twice; count checked, 14,899 → 14,913.

What auth:twofactor-cleanup actually sweeps, and the assertion total I made unreadable

Fifth instance of the same shape: AuthTwoFactorCleanup has tests, and every one of them overrides sweeps(). They prove the loop reports a failure per sweep and handles a missing table — and they say nothing about the sweeps being the right ones, because the sweeps were the test's.

These are deletes, which is why it matters more than the ten statements. A sweep pointed at the wrong table deletes rows nobody asked about; a sweep on the right table with the wrong predicate deletes live second factors. Both report success, because a DELETE that matches nothing and one that matches everything look identical from outside.

So each sweep runs against real rows on both lanes, with one row that must go and one that must stay. The interesting half is the setup sweep's predicate — used = 1 or expired — where the first clause is the one worth pinning: a setup session that has done its job is rubbish immediately, and leaving it would let the same temporary secret be presented again inside its fifteen minutes. Its counterpart is somebody halfway through scanning a QR code, whose unused, unexpired session must survive.

The map's keys are asserted too, because they are operator-facing: the loop reports which sweep failed, so renaming a key changes a message somebody reads in a schedule log to work out what broke.

authserver. is a real schema on PostgreSQL and a table-name prefix on MySQL, so "the sweep found its table" is a different claim on each lane — and a sweep that silently matched no table would look exactly like one with nothing to delete.

The non-deterministic assertion count was mine

I flagged twice today that the suite reported different assertion totals between identical runs. This round found the cause I could actually attribute: ThemeDirectoriesTest, written an hour earlier, asserted once per theme directory it discovered — so its contribution depended on what the checkout had. Fixed by asserting the property once instead of iterating, and three consecutive runs now agree at 44,285.

Worth the detour, because the assertion total is the cheapest signal available for "did this change add assertions or quietly lose some" — the same signal that caught seven tests landing in the wrong class earlier today. A variable total is a signal you can no longer read, and I had made it variable myself.

Suite 2:45 twice. The band has drifted from 2:37 to 2:45 across the day as tests accumulated, which is 8 seconds for about 250 new tests.

A test called "the omnibox limit is capped" that would pass on an uncapped omnibox

Sixth instance of the pattern, and the sharpest, because this one had a good reason and a name that claimed the opposite.

ApiAdmin::search() caps the per-source limit at twenty — an endpoint that took the number from the request would be a denial-of-service endpoint with a friendly name. testTheOmniboxLimitIsCapped() overrode search() in its probe and re-implemented min(20, max(1, ...)) inside the test, with a docblock explaining, reasonably, that running six real searches would be asserting the registry's behaviour rather than the cap.

The reasoning was sound and the consequence was not: the test asserted its own arithmetic. Change the source to min(500, ...) and it passes, because the source is never called. The endpoint's eight statements were on the never-run list while a test named after them was green.

Fixed by recording from below rather than in place: one protected line, searchRegistry(), which the probe overrides instead. The real search() now runs, the real cap computes, and the number asserted is the one the endpoint produced. Two assertions became possible that were not before — the term comes from q, and the default limit is five. The first matters more than it sounds: a search() reading the wrong parameter would answer an empty result set for every query, which looks exactly like an omnibox over an empty database.

The guide gets the check to run on any probe: for each method it overrides, ask whether the source could change and the test still pass. If the override contains a copy of an expression from the source, the answer is yes.

94.77%, 145 statements from the target. Suite 2:43 and 2:48; count 14,923 to 14,927, assertions stable.

And this entry is in the wrong commit

It belongs in a5c333c2, with the change it describes. The script that appends it failed on a quoting error while the one that edited the guide had already succeeded, so the commit went out with the guide and without its changelog — the one pairing this project asks for. Since it is pushed, the entry lands here rather than by rewriting a commit somebody may already have.

The lesson is small and mine: write the changelog before staging, not as the last step of a chain where an earlier link has already committed.

Configuring a local asset list crashed every page build

Three pure never-run methods this round, and the third one was not fine.

Document::servesDefaultsFromCdn() decides whether jQuery and the two plugins come from a CDN or from this server. It read documentAssetSource and handled the plain-array form. Its sibling localHandles() reads the same setting and says so in its own comment — "settings round-trip a list as an array, an stdClass or a JSON string depending on how it was stored, and a comma-separated string is what somebody types" — and handles all four.

So the first test that ever called the first method got Object of class stdClass could not be converted to string. On an installation that had configured a list, that is a fatal on every page build, because the constructor calls it. And the two string forms were worse in a quieter way: not recognised as lists, so reported as "on the CDN", meaning a page emitted CDN tags for scripts the installation had deliberately vendored — the exact outcome somebody sets this to avoid.

The fix is one line. "Are they on the CDN" is "is nothing local":

protected static function servesDefaultsFromCdn(): bool
{
    return self::localHandles() === [];
}

Behaviour-identical everywhere the old code was right, and correct in the three forms where it was not — because the parsing now happens once, in the method that already did it properly. Reading the same setting twice meant reading it two different ways.

The other two were fine, and worth pinning anyway. Logger::formatBytes() has two guards that are arithmetic rather than defensiveness: log(0) is -INF, so an empty log file would index $units[-INF], and min($pow, count($units) - 1) stops a petabyte from indexing past TB. Both asserted, along with the ladder's boundaries — a file the shell calls 1.0K should not be shown as 1024 B.

Init::generateRandomPassword() is the first administrator's password on a new installation. It cannot be tested for randomness, so what is asserted is the shape: the length, that every character comes from the declared alphabet over a 200-character sample, that two calls differ — the cheapest check against the worst bug, since a generator returning a constant satisfies everything else — and that the look-alikes are absent. No i, l, o, I, L, O, 0 or 1, over 800 characters, because this is a password read off a terminal and typed by hand, and every one of those pairs is a failed sign-in that looks like a wrong password rather than a misread character.

Suite 2:43 and 2:40; count 14,927 to 14,957.

An addon setting called 2fa_enabled is not called that

Two more never-run methods, and one of them hides a rename an addon author has to know about.

Addon::addSetting() is a nine-argument pass-through to the settings form, except for one line: a name whose first character is a digit gets an underscore in front of it. 2fa_enabled becomes _2fa_enabled, because the name is used as both a form field and a property and neither may begin with a digit. The rename is silent, and it is the name the value comes back under — so it is now in the guide, along with the fact that a digit anywhere else is left alone: prefixing oauth2_key would rename a setting for no reason, and an addon upgrading into that would lose its stored value.

Six tests, including the two that keep the pass-through honest: that the type, options, default and required flag arrive on the field the form built — nine positional arguments are easy to transpose, and a transposition gives every select box its description as its option list — and that two declarations land on the same form, since a settingsForm() that built a new one per call would leave every addon with exactly one setting, the last one declared.

McpCall::server() is the second, and its branch matters more than its ten statements: an application's own server is used when there is one. An application registers its tools on the container's mcp.server, so building a fresh one would answer mcp:call with the framework's defaults and none of the project's — a tool list correct about the framework and silently wrong about the application, which is the shape of report somebody trusts. Asserted by identity, because anything other than the container's own instance means the application's tools are missing.

The fallback is asserted for what it registers rather than for existing: a bare McpServer would report that this installation has no tools, which is a different claim from "there is no application here". And the middle case — an application whose container has no server — is why the check is has() rather than !== null: the MCP feature is opt-in, so asking a container for a service it never registered must not be an error.

94.81%, 117 statements from the target. Suite 2:43 and 2:42; count 14,957 to 14,967.

The query behind hasIndex(), and the assertion that PostgreSQL does not inherit it

SchemaGrammar::compileHasIndex() is the fallback for a driver the framework does not know, and it had no covered line. A wrong answer here is not an error: it is a migration that skips an index the schema needs, or one that tries to create a duplicate and fails on a name already taken. Both look like a migration problem rather than a grammar problem.

The base uses the MySQL-shaped information_schema.statistics, deliberately — the standard has no index view, so the fallback resembles the driver an unknown one is most likely to resemble. Its schema clause is conditional, and both halves are silent when wrong: without the filter, an index of the same name in another schema answers yes and the migration skips a table with no index; with an empty filter added, table_schema = '' matches nothing and every check answers no.

The assertion that matters most is that both real grammars override it. PostgreSQL has no information_schema.statistics; a grammar that inherited the base would make every hasIndex() raise — every migration that guards an index — with an error naming a view nobody in the project ever wrote.

Two PostgreSQL behaviours came with it, both now in the schema guide. A qualified name is split, so authserver.usertokens becomes schema authserver and table usertokens — without that, tablename = 'authserver.usertokens' matches nothing and every index on the framework's own tables reads as absent. And with no schema given it excludes pg_catalog and information_schema rather than searching everything, because those carry thousands of indexes and a name colliding with one would answer yes for a table the project owns.

The stand-in for "an unknown driver" implements the five abstract members the base leaves open, which is the honest shape of the case rather than a convenience — none of them is reached by compileHasIndex(), and a real driver would have to answer all five. One of them cost two attempts: compileSetVal() carries a default in the interface, and omitting it is a signature incompatibility rather than a stricter override.

Suite 2:44 twice; count 14,967 to 14,973.

Three counters, sessions and headers — and a setter that means the opposite of its name

Three never-run methods, and the third one turned up an API trap.

Cache::increment() is the front door to an atomic counter, which is what rate limits and replay guards are built on. Its docblock states the thing nothing in the type system says — false is not zero — and that is what the tests pin: a caller treating the answer as a number reads a broken cache as "no requests yet", which is a limiter that lets everything through at exactly the moment it cannot count. Two guards produce that false and neither is an error: caching turned off, and an adapter that cannot count. The second is asserted to not call the adapter at all, because a read-modify-write here loses increments under precisely the concurrency a limiter exists for.

Session::startIfPresent() is why the page cache can store anything. A first-time anonymous visitor gets no session, therefore no Set-Cookie, therefore a response that is safe to serve to the next anonymous visitor. Start a session for everybody and every response carries a session id unique to one person, so the cache correctly refuses to store any of them — and a page cache that refuses everything is a page cache that does nothing. Both halves asserted, since laziness that dropped existing sessions would sign everybody out; and the decision reads the configured session name rather than a hardcoded PHPSESSID, because an installation that renamed its cookie would otherwise look sessionless to every request while start() kept working.

StreamedResponse::getHeaders() flattens each name to the line that goes on the wire, and writing the test found this:

public function withHeader(string $name, string $value): static    // appends
public function withRawHeader(string $name, string $value): static // replaces

In PSR-7 it is withHeader() that replaces and withAddedHeader() that appends. Here the familiar name does the opposite, so correcting a Content-Type with it produces text/html, application/json — a header no browser will use and no log will explain. Appending is right for Vary and Cache-Control; it is wrong for anything single-valued. Both are now pinned in one test and written into the realtime guide, because the difference is invisible until a response goes out wrong.

My first version of that test called withAddedHeader(), which does not exist, and new StreamedResponse(...), whose constructor is private — two wrong guesses about an API I had not read. The second is the useful one to notice: the constructor is private because an SSE response and a plain streamed one differ in the headers they start with, and the named constructors are what say which you asked for.

Suite readings this round were 2:47 and 2:52, then 3:15 and 3:29 — disagreeing, so noise rather than a measurement, on a machine that has been unsteady all evening. Fifteen tests do not cost forty seconds. Recorded as unmeasured rather than as either a pass or a regression; the next quiet run settles it.

Seven more off the never-run list, in one pass

Working straight through rather than a target per round. Seven methods, all reachable, all with something worth stating.

StreamedResponse's remaining mutators. Every with* returns a clone, asserted by checking that the original is untouched after three different mutations — middleware passes a response along after inspecting it, and a mutator returning $this would make every earlier stage's reference point at the final state. withoutHeader() removes every value of a name and is silent about a name that was never set, which is what lets middleware strip a header it may not have been given without checking first.

AbstractAdapter::swap() writes a value and reports the previous one, non-atomically and deliberately so. null means the key was unset — and I wrote the third test asserting the base implementation cannot tell that apart from a stored empty string. It can: === false || === null is exactly what keeps them apart, so '' and '0' come back as themselves. Corrected, and the note kept, because a falsy test there would tell the next caller a claimed key was free.

MessagesController::currentUserId() — three statements in front of every read and write of somebody's messages, and the shape is the point: the session decides and the user object only supplies the id. The application caches currentUser for the length of a request, so the object outlives a sign-out — reading the id straight off it would serve a signed-out visitor a cached account's messages. Asserted with exactly that setup: a cached user, no session, answer 0.

Init::report() is the summary a scaffolding run ends with, and both lists are sorted — not cosmetic, since a run writes in dependency order and an unsorted list is one nobody can diff against the previous run. Also that a run which did nothing reports two empty lists rather than missing keys, because the caller indexes both.

Adminer::terminate() is an exit with a PHPUnit guard, and the test finishing is the assertion. A terminate() that lost its guard would not fail — it would end the whole suite at whichever test reached it first, with no failure and no report. Called twice, to rule out a guard that works once.

MakeService::execute() — six statements of wiring and one refusal. Without it, createService('') derives a class name from nothing and writes src/Services/.php: a dotfile PHP will never autoload, reported as created.

Document::loadtheme() resolves a theme and, the part that matters, keeps it. A version that returned without storing would leave themeObject null, and everything asking the document which theme it is in would fall back to the default while the page rendered with something else. Also that it passes $load = false — a document recording which theme it uses must not run the theme's bootstrap as a side effect.

15,010 tests. The suite read 3:25, on a machine that has been giving 2:37 to 3:29 for the same tree all evening — recorded as unmeasured rather than as a regression, and to be settled when it is quiet.

Three more, and a state leak I caused and had already written the guide entry for

ScheduledTask::runShellCommand() is the one line in the scheduler that reaches the shell, and a documented seam — every other test in that area asserts what would be run by overriding it, which is why the real one had never executed. Somebody has to run it, or the seam is the only thing that has ever worked. What it must get right is the exit status: passthru() puts it in a by-reference argument, the easiest thing in PHP to forget, and forgetting it reports every scheduled task as successful. Asserted with true, false, exit 3, exit 42 and a command that does not exist — 127, as a status the scheduler can log rather than an exception from inside a batch.

Gdpr::userFromToken() decides whose personal data a GDPR request may export or erase, which makes it the whole of that controller's authorisation. The guard is >= 2, not > 0: zero is "no account was loaded" and one is the framework's system row, so >= 1 would hand an unauthenticated caller the system account and an export of it is an export of whatever the framework attributes to nobody.

Oauth::endWebSession() gets a webLogout() seam, because its four statements are entirely the try/catch and the Auth was constructed inside the try. The catch is the behaviour: the tokens are already revoked by the time this runs, so a session that could not be ended is a log line and not a failed request — raising there would report an unsuccessful logout for one that had succeeded in the part that matters, and the client would retry a revocation it had completed.

The leak

The Gdpr test needs a connection, so its setUp() dropped the Database singleton and rebuilt it from the MySQL fixture — and did not put it back. Eleven OauthControllerIntegrationTest tests then failed in the full suite while passing under a filter, because they inherited this class's connection.

Which is the failure I wrote a guide section about earlier today, from the other side: "a class that connects fine on its own and fails in the full suite is the singleton". I read that as a thing to watch for in other people's tests and then wrote a test that caused it. Restored in tearDown(), and the guide already says why.

94.92% before this batch — 50 statements from the target. 15,019 tests, suite 2:46 with the machine quiet again.

Three small ones to close the gap

Worker::__construct() — every test in that area builds a worker through a double that skips the constructor, so its three statements had never run. The worker id is what makes a claim attributable: a batch abandoned by a worker that died can only be reclaimed if the claim says who took it, and null has to stay null rather than becoming '' — an unattributed claim and one attributed to a worker whose name is nothing are different things.

The first version of that test asserted assertNull($worker->asked[1] ?? 'not-null'), which cannot fail the way I intended: null ?? 'x' is 'x', so ?? cannot tell a null value from an absent key. Asserted on the array now.

Unsubscribe::respond() is the reply to a List-Unsubscribe-Post — a mail client pressing the button on somebody's behalf — so the audience is software and the requirements are software's: a status it can branch on, a content type it will not render, a body it can log. The headers_sent() guard is not defensiveness: reached after output has begun, header() prints a PHP warning into the response, which is worse than the missing header because it corrupts the body the client is about to read.

McpServe::trafficLogPath() defaults into the framework's log directory rather than the working directory, so the file shows up in the log viewer and log-errors --files mcp.log answers "which call failed". A default of ./mcp.log would land wherever the daemon was started from, which is / under most service managers. A blank --traffic-log= — what a script produces when the variable it interpolates is unset — falls back rather than becoming an unopenable empty path.

94.93%, 42 statements from the target before this batch. 15,032 tests.

The machine account, an MCP call's identity, and a helper the framework ships untested

Auth\Application::createSystemUserRow() is a documented seam — the one thing in that class needing a database — so its ten statements had never run, including under the test written earlier today, which overrides it. What the row must be is the substance, and each property answers a way this could go wrong:

  • usertype 1, below every administrative threshold, so a token issued to an application cannot be mistaken for one issued to an operator. A machine account at 90 would pass every admin check in the framework.
  • sys_ plus sixteen hex characters from random_bytes, because users.username is unique and two applications registering in the same second must not collide — a collision is a registration that fails, not one that shares an account.
  • active and validated, or a token presentation fails the account checks and every client-credentials call is refused with nothing in the log naming the account as the reason.
  • @system.local, because the column is often required and unique so it cannot be blank, and it must not be routable — anything that mails an account would mail it.

Both lanes, since the id comes from a sequence on one and LAST_INSERT_ID() on the other.

McpController's constructor and authenticatedUser(). The constructor registers exactly one action: POST /mcp is a single JSON-RPC endpoint, and a controller exposing its helpers as actions would let a URL address the ones that read tokens and scopes. authenticatedUser() asks the framework who the request turned out to be rather than parsing the bearer header a second time — the middleware already validated it, and a second reading is a second opinion — and it returns null rather than an empty User, because an empty User is truthy and every if ($user) downstream would run the authenticated path for a call that presented nothing.

BaseTestCase::sameOriginApiHeaders() is a helper the framework ships to applications, and nothing in the framework's own suite had ever called it — so a helper offered to every project was untested. It encodes the rule ApiAuthMiddleware enforces: a page presents no API key, it presents its session plus a CSRF token. Three of its four assertions are about being usable rather than correct-looking: the key is HTTP_X_CSRF_TOKEN and not the wire name, because a test dispatcher takes a $_SERVER array; the token is the session's own rather than a fresh one, or the request fails the check the helper exists to pass; and it carries nothing else.

15,050 tests, suite 2:43.

/adminer refuses with a 404, and why that is the security decision

Nine statements guarding a tool that can read every table, never executed.

The refusal is a 404, not a 403, and that is the whole of it. A 403 says "this exists and you may not have it" — a page nothing else on the site produces, so it tells whoever is looking that there is a database console here and that they were turned away. The site's own 404 says nothing. The test asserts that on the message: it contains 404, it does not contain 403 or Forbidden, and it does not name Adminer.

It also delegates rather than reimplementing, so /adminer refuses exactly the way every other missing page does — a hand-rolled 404 would be distinguishable from the real one, which is the thing being avoided. And it does not terminate afterwards: notFound() ends the request itself, so a second ending would be a second ending.

A branch I left alone and said why. With no application on the controller it falls back to Application::currentInstance(), and below that is a hand-written status-and-terminate for the case with no application anywhere. That one is not reachable from a test run — currentInstance() returns whatever earlier test built an application, and there always is one by the time this file runs. My first version of the test assumed setting the controller's own application to null would reach it; it does not, and the two tests that assumed so failed in the full suite while passing under a filter, for the third time today. Rewritten to assert the fallback order, which is the reachable and more useful claim.

15,053 tests, suite 2:40.

The reconnect that must forget, and the first sign-in that is not a new device

RedisSubscriberSocket::close() is the method a broadcast daemon calls most — every reconnect goes through it — and five statements had never run. Three properties, all about a process that does not restart: the stream is closed, or hours of reconnects leak a descriptor each until nothing can be opened; the handle becomes null, so the next isConnected() says no rather than answering from a closed resource; and the buffer is emptied, which is the subtle one. Half a frame left over from the dropped connection would be prefixed onto the first frame of the new one, and the reader would see a single message that parses as neither. Also safe twice, because a reconnect loop cannot know whether the socket it is dropping was ever open.

LoginFlow::qualifiesForDemand() picks between the two readings of "this sign-in needs a second look". new_device asks whether this browser has been seen on this account; suspicious asks SignInRisk and accepts only signals hard to explain innocently — a country the account has never used, two places at once, a country change too soon to have travelled.

I expected the two to disagree about an account with no history, and wrote the test asserting that. They agree, and the reason is the better answer: isNew() returns false when there are no known fingerprints at all, because the first sign-in on an account is not a new-device event. There is nothing to compare against, and alerting somebody about their own first login is noise that teaches them to ignore the next one. So the two readings agree on a fresh account and differ on an established one, which is the shape that makes the setting worth having rather than a switch between "always" and "never".

Third time today a test of mine encoded a wish rather than the behaviour, and the third time the behaviour turned out to be the more considered of the two.

15,060 tests, suite 2:39.

The middleware list, and the bracket somebody will forget

Three statements, never executed, and they decide what wraps every dispatch — a MiddlewarePipeline is built from whatever this returns.

The type guard is the substance. middleware comes out of app.php, a file edited by hand, and a single entry written without brackets —

'middleware' => \App\Middleware\Cors::class,   // a string, not a list

— would reach a foreach as a string. Returning [] for anything that is not an array turns that into "no middleware", which the developer sees the moment they test what they just added; the alternative is foreach over a string, and PHP's answer to that is a warning printed into the response of every request.

Five shapes asserted through a data provider — a bare class name, null, a number, true, an object — plus that a declared list comes back in order, because middleware wraps in declaration order and a reordered list would run an authentication check after the thing it guards.

15,068 tests, suite 2:39.

can() and cannot() — the pair every guard clause is written with

Four statements between them, never executed, in the two methods an application asks before it acts. Both are thin by design: the decision belongs to the Gate, and a controller that reimplemented any of it would hold a second opinion about the same ability.

cannot() is !$this->can(…) rather than its own Gate call, and that is the detail worth pinning. The two can never disagree, and a rule registered after the controller was constructed is seen by both. An independent implementation is how a codebase ends up allowing something in one guard clause and refusing it in another.

Three claims:

  • An undefined ability is refused, not allowed. A typo, or a provider that did not boot, must not be answered yes.
  • The arguments reach the rule, in order. A rule is fn($user, $subject) => …, so an ability asked without its subject is a different question — one a rule comparing an owner would answer by comparing against nothing.
  • cannot() passes them through too, asserted separately because it delegates: a cannot() that called can($ability) and dropped the rest would negate the answer to a different question, and every negated guard clause in an application would be wrong in the same invisible way.

15,073 tests, suite 2:39.

The last three: two seams and the accessor every closing test reads

ApiCrudController::legacyAclTable() and db() exist so a test can point the legacy-permissions probe at a table that certainly does not exist — the only state the bug they were extracted for was visible in, and the state every new installation is in. Which is exactly why they had no covered line: every test in that area overrides them. The table name honours DB_PERMISSIONSTABLE, because an installation that renamed the table set that constant and a probe reading the default would report "no legacy permissions" for a table full of them — wrong in the direction that loses a migration. db() hands back the process's connection by identity, because the probe's behaviour is a dialect fact and a seam building its own connection could be talking to another backend than the test set up.

ApplicationClosedException::getBody() is what a test reads instead of the response a real request would have sent: close() throws rather than exiting under PHPUnit, and this is the only way to see what the visitor would have got. An exception carrying a body nobody can read makes every test of a closing path assert the status alone. And with no body it is '' rather than null, because callers search it and assertStringContainsString() on null is a TypeError — a test failing on its own assertion rather than on the code.

15,077 tests, suite 2:41.

The two gaps that were in the environment, not the tests

Both of these were on the list of "cannot be tested here", and both turned out to be the environment rather than the code.

APCu was never installed

The migration fingerprint cache prefers APCu and falls back to a marker file. APCu is the path that runs in production — it is the reason the cache exists at all — and it was unreachable twice over: the extension was not in the image, and apc.enable_cli defaults to 0 even once it is. So the branch that executes on every live request had no covered line, and the one that had was the fallback.

Both fixed in the Dockerfile. Six tests now assert the branch that actually runs, and two of them are about a shared PHP-FPM pool rather than about caching: the key is namespaced by the application root, because APCu is per pool and without it two applications would answer each other's question — and "your migrations are verified", from another application's schema, is the worst answer to get right by accident. And forgetting uses an APCUIterator over that prefix rather than apcu_clear_cache(), so clearing one migration check does not drop every other application's sessions and rate-limit counters as a side effect. That one is asserted by storing an unrelated entry and checking it survives.

OutboundUrl had nowhere legitimate to fetch from

The class refuses every private, loopback, link-local and reserved address — the whole point of it — so nothing a container can reach is fetchable and its socket path had never opened. In a class whose entire job is fetching a URL somebody else chose.

The tempting fix is to widen the guard: self:: to static:: so a test subclass can override isPublicAddress(), or a flag. A guard a test can relax is a guard an application can relax by accident, so instead the environment gained an address. The test container now sits on a second network in 203.0.113.0/24 — TEST-NET-3, RFC 5737, reserved for documentation and routed nowhere — and PHP's NO_PRIV_RANGE | NO_RES_RANGE does not exclude the documentation blocks, so it reads as public while the container can still only reach itself. Apache was already listening. The guard is untouched; nothing is overridden and no seam is widened.

Nine tests, and three of them could not have existed before:

  • A redirect to 169.254.169.254 is refused, over a real socket, after a first hop that was allowed. That is the whole reason each hop is a fresh question, and it had been asserted only against nextHop() as a pure function — never against a fetcher that had actually followed one.
  • The byte cap refuses while the body arrives, not after. A cap checked with strlen() at the end has already spent the memory it was trying not to; asserted with a 40,000-byte fixture and a 1,024-byte cap, plus the control that the same fixture comes back whole under a large cap.
  • The status distinguishes a 404 from a 200 with ignore_errors still on, which matters for the case content checks cannot see: a CDN answering 404 with a placeholder image returns bytes that are a valid PNG, and every content check passes while the placeholder is stored as the thing that was asked for.

Both suites of tests skip rather than fail where the environment is absent, so a checkout run outside this compose file is unaffected. The guide gets the rule this pair taught: before deciding a branch is untestable, ask whether it is the environment that cannot reach it. An extension that is not installed, a SAPI setting that defaults off, an address family a container does not have — all three look exactly like unreachable code, and none of them is.

15,092 tests. Suite 2:42 and 2:41; the first run after rebuilding the image read 3:11 and was the image warming up.