Summary
Application::register() does an unguarded include_once of a composer autoloader that a
released install does not have, emitting two PHP warnings on every request that boots the app.
// lib/AppInfo/Application.php
include_once __DIR__.'/../../vendor/autoload.php';
composer.json requires only php: ^8.3 — everything else is require-dev. So there are no
runtime composer dependencies and a production install legitimately ships no vendor/
directory.
Evidence
On a clean Nextcloud 34 install with the app enabled, a handful of page loads produced 150 log
lines of:
include_once(/var/www/html/custom_apps/nldesign/lib/AppInfo/../../vendor/autoload.php):
Failed to open stream: No such file or directory at .../lib/AppInfo/Application.php#100
include_once(): Failed opening '.../vendor/autoload.php' for inclusion (include_path=...)
include_once never fatals, so nothing is broken. The cost is that the app's log is mostly this,
which is noise in its own right and good cover for a warning someone needs to see. It also cost me
real time: while debugging an unrelated problem these were the only nldesign lines in the log, and
they read like the root cause.
Fix
$autoloader = __DIR__.'/../../vendor/autoload.php';
if (\file_exists($autoloader) === true) {
include_once $autoloader;
}Why this is an issue and not already a PR
I wrote exactly that patch and reverted it, because it correctly failed the coverage ratchet:
FAIL: coverage dropped by 0.04% against the merge base.
merge base 3548/4424 -> head 3548/4426 statements.
This change adds 2 statements. Adding code without tests drops coverage.
The guard adds two statements that no unit test covers — there is no test for
Application::register() at all. Landing it means either weakening a working gate or writing the
first unit test for register(), which needs a mocked IRegistrationContext and exercises the
whole registration body (capability, two event listeners, the OpenRegister autoloader prelude).
That is worth doing, but it is its own piece of work rather than a drive-by on a test-coverage PR.
So: the fix is known and one line; it needs an accompanying register() unit test to land without
defeating the ratchet.
Found while raising gate-19 e2e coverage (#265).
Summary
Application::register()does an unguardedinclude_onceof a composer autoloader that areleased install does not have, emitting two PHP warnings on every request that boots the app.
composer.jsonrequires onlyphp: ^8.3— everything else isrequire-dev. So there are noruntime composer dependencies and a production install legitimately ships no
vendor/directory.
Evidence
On a clean Nextcloud 34 install with the app enabled, a handful of page loads produced 150 log
lines of:
include_oncenever fatals, so nothing is broken. The cost is that the app's log is mostly this,which is noise in its own right and good cover for a warning someone needs to see. It also cost me
real time: while debugging an unrelated problem these were the only nldesign lines in the log, and
they read like the root cause.
Fix
Why this is an issue and not already a PR
I wrote exactly that patch and reverted it, because it correctly failed the coverage ratchet:
The guard adds two statements that no unit test covers — there is no test for
Application::register()at all. Landing it means either weakening a working gate or writing thefirst unit test for
register(), which needs a mockedIRegistrationContextand exercises thewhole registration body (capability, two event listeners, the OpenRegister autoloader prelude).
That is worth doing, but it is its own piece of work rather than a drive-by on a test-coverage PR.
So: the fix is known and one line; it needs an accompanying
register()unit test to land withoutdefeating the ratchet.
Found while raising gate-19 e2e coverage (#265).