diff --git a/CLAUDE.md b/CLAUDE.md index 44879a56a..80e8361b5 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -127,15 +127,19 @@ gotchas (the image provides these): ### Coverage: what is covered, and what deliberately is not -Line coverage is **95.4%** (`21368/22399`). Measure it by installing pcov in the container +Line coverage is **96.6%** (`21648/22402`). Measure it by installing pcov in the container (`pecl install pcov && docker-php-ext-enable pcov` — the enable step is separate, and a previous session's `pecl install` leaves it *installed but disabled*, so a `pecl list | grep pcov` guard skips it and every file reports zero), running both suites with `--coverage-clover`, and removing the ini again — the image does not ship a coverage driver, and leaving one enabled slows every later run. When merging two clover files, read `//file` by xpath: clover nests files inside -`` elements, so `$xml->project->file` only ever finds the namespace-less ones. +`` elements, so `$xml->project->file` only ever finds the namespace-less ones. The two +runs also disagree about which lines *are* statements in a handful of files (9 of 995, 31 +statements — `Application/Account/Services/AccountSearch.php` worst at 48 vs 61), so a merge that +treats "absent from one report" as uncovered overstates those files. Confirm a per-file gap with a +focused `--coverage-clover` run before sending anyone to fix it: that file is at 100%. -The remaining ~1000 statements are not a backlog to burn down uniformly. What is left: +The remaining ~750 statements are not a backlog to burn down uniformly. What is left: - **Bootstrap (~90 statements).** `Base.php`, `Definitions/CoreDefinitions.php`, `Adapter/In/Cli/Init.php`, `Bootstrap/BootstrapBase.php`. These run *before* the container the tests build — `Base.php` is @@ -189,8 +193,8 @@ A few harness details bite when writing an integration test against a real branc drawing a group or profile id that way fails about one run in a hundred, on CI, in whichever pull request happened to be open. Use `numberBetween(1, …)`. -The rest is genuinely reachable, and is a long tail rather than a few large files: ~1000 -statements across **295** files, averaging under four statements each — individual error branches, +The rest is genuinely reachable, and is a long tail rather than a few large files: ~660 +statements across **270** files, averaging under three statements each — individual error branches, rarely-hit conditionals and unused accessors. Worth picking off when touching the surrounding code; not worth a campaign. diff --git a/src/Domain/Core/Context/SessionContext.php b/src/Domain/Core/Context/SessionContext.php index 9c7895c9a..d694c0a64 100644 --- a/src/Domain/Core/Context/SessionContext.php +++ b/src/Domain/Core/Context/SessionContext.php @@ -127,11 +127,11 @@ public function setLastActivity(int $time): void; public function getStartActivity(): int; /** - * Return the color associated with an account + * Return the colors associated with the accounts, keyed by the id they belong to * - * @return string + * @return array */ - public function getAccountColor(): string; + public function getAccountColor(): array; /** * Set the color associated with an account diff --git a/src/Infrastructure/Context/Session.php b/src/Infrastructure/Context/Session.php index 3dc95fb35..ee2a84c23 100644 --- a/src/Infrastructure/Context/Session.php +++ b/src/Infrastructure/Context/Session.php @@ -72,7 +72,9 @@ public static function close(): void */ public function getTheme(): string { - return $this->getContextKey('theme'); + // Defaulted, because a session that has not been given a theme yet would otherwise return + // null from a method declared to return a string, and raise on the way out. + return $this->getContextKey('theme', ''); } /** @@ -298,11 +300,16 @@ public function getLocale(): ?string } /** - * Returns the color associated with an account + * Returns the colors associated with the accounts, keyed by the id they belong to. + * + * An array, because that is what setAccountColor() stores: declared as a string, this raised + * on its way out the first time anybody read it back. + * + * @return array */ - public function getAccountColor(): string + public function getAccountColor(): array { - return $this->getContextKey('accountcolor'); + return $this->getContextKey('accountcolor', []); } /** diff --git a/tests/Unit/Infrastructure/Context/SessionTest.php b/tests/Unit/Infrastructure/Context/SessionTest.php index 8382a8845..3d877b61f 100644 --- a/tests/Unit/Infrastructure/Context/SessionTest.php +++ b/tests/Unit/Infrastructure/Context/SessionTest.php @@ -482,6 +482,45 @@ public function theConfigLoadTimeIsCarried() self::assertSame(1700000000, $session->getConfigTime()); } + /** + * The account colours go in as an array and come back as one. Declared as a string, this + * raised a TypeError on the way out the first time anybody read back what had been stored — + * nothing in the application did, which is the only reason it went unnoticed. + * + * @throws ContextException + * @throws SPException + */ + #[Test] + public function theAccountColoursAreCarried() + { + $session = $this->givenAStartedSession(); + + self::assertSame([], $session->getAccountColor(), 'a session with none set has none'); + + $session->setAccountColor([100 => '#ff0000', 200 => '#00ff00']); + + self::assertSame([100 => '#ff0000', 200 => '#00ff00'], $session->getAccountColor()); + } + + /** + * And the theme is a string even before one has been chosen — read from a session that has not + * been given one, it used to return null from a method declared to return a string. + * + * @throws ContextException + * @throws SPException + */ + #[Test] + public function theThemeIsCarriedAndIsAlwaysAString() + { + $session = $this->givenAStartedSession(); + + self::assertSame('', $session->getTheme()); + + $session->setTheme('material-blue'); + + self::assertSame('material-blue', $session->getTheme()); + } + /** * @throws ContextException * @throws SPException