[TASK] Unit-test XmlFileLoader - #3
Closed
CybotTM wants to merge 2 commits into
Closed
Conversation
guides-cli's unit suite covers Command and Logger only, so the config loader is reachable exclusively through fixtures that render a whole site. Those prove what a reader sees; they cannot cheaply reach the shapes an author is unlikely to write while the loader handles them explicitly. XmlFileLoaderTest pins eight: a version read as written, a trailing zero kept in release as well, the quotes stripped from version and release, the same quotes kept on every other attribute, an attribute-less <project>, a file whose only child is <project>, a <project> nested in an <extension> not being taken for the project, and a file without a <project> not gaining an empty one. The third and fourth are the pair worth stating: the stripping is backward compatibility for two attributes, not a general unquoting rule, so a title that really is quoted keeps its quotes. Nothing covered that. Each case was seen failing on a defect built for it - DOM reading removed, trim widened to every attribute, trim dropped, the direct-child search degraded to a subtree search, the project key withheld when empty, and the project key set unconditionally. Every case appears in at least one failure list. Two appeared in none until the assertion was split: `?? []` reported a missing project config and an empty one as the same state. Assisted-by: claude-code:claude-opus-5 Agent-Session: https://claude.ai/code/session_014H1xwaAmrQRWUA3vx8bJcD Agent-Host: 0493f0 Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
… issue
The unit test added in the previous commit turned the CI cell (8.4, lowest) red
with an error that is not an assertion:
PHPUnit\Runner\Baseline\FileDoesNotHaveLineException:
File "vendor/symfony/config/Loader/FileLoader.php" does not have line 0
Reproduced in a php:8.4-cli container after `composer update --prefer-lowest`,
then narrowed by removing one block at a time.
Declaring `XmlFileLoader extends FileLoader` makes PHP 8.4 raise the implicit
nullable deprecations of the inherited signatures while linking the class. Those
are attributed to the parent file with no line, so PHPUnit reports the issue at
line 0. The baseline then looks the file up, tries to read line 0 to hash it,
and throws. It only fires where the baseline already knows that file, which is
why the integration and functional suites - which load the same class - pass on
the same cell.
A baseline entry pins a line number in third-party code. That is fragile across
dependency resolutions by itself, and for this shape of issue it cannot work at
all: no line number can match. Removing the block for this one file is the
narrow fix; the deprecations it covered are now displayed, and
`failOnDeprecation="false"` keeps them from failing anything. The 39 other
vendor files in the baseline carry the same fragility, but nothing observed
justifies touching them in this branch.
Verified in the container on PHP 8.4 with the lowest set - unit 494, functional
117, integration 230, all green - and on the locked set: PHPUnit 841, PHPStan,
PHPCS, deptrac clean.
Assisted-by: claude-code:claude-opus-5
Agent-Session: https://claude.ai/code/session_014H1xwaAmrQRWUA3vx8bJcD
Agent-Host: 0493f0
Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



Stacked on phpDocumentor#1345 — the base branch is
fix/xml-version-string-coercion, notmain. Review that one first; this adds only tests plus the one baseline entry they cannot coexist with.Why
packages/guides-cli/tests/unit/coversCommandandLogger.XmlFileLoaderhas no unit test at all, so everything it does is reachable only through integration fixtures that render a whole site. Those prove what a reader sees. They do not cheaply reach the shapes an author is unlikely to write while the loader handles them explicitly: an attribute-less<project>, a file whose only child is<project>, a file with no<project>.What it pins
Eight cases. The one that had no coverage anywhere: the single-quote stripping is backward compatibility for
versionandrelease, not a general unquoting rule — atitlethat really is quoted keeps its quotes. Widening thattrimto every attribute would have gone unnoticed.Each case was seen failing on a defect built for it: DOM reading removed, trim widened to every attribute, trim dropped, the direct-child search degraded to a subtree search, the project key withheld when empty, and the project key set unconditionally. Every case appears in at least one failure list — two of them appeared in none until the assertion was split, because
?? []reported a missing project config and an empty one as the same state.The baseline entry
Adding the test turned
(8.4, lowest)red, with an error that is not an assertion:Reproduced in a
php:8.4-clicontainer aftercomposer update --prefer-lowest, then narrowed by removing one baseline block at a time.Declaring
XmlFileLoader extends FileLoadermakes PHP 8.4 raise the implicit-nullable deprecations of the inherited signatures while linking the class. Those are attributed to the parent file with no line, so PHPUnit reports the issue at line 0; the baseline then looks the file up, tries to read line 0 to hash it, and throws. It fires only where the baseline already knows that file, which is why the integration and functional suites — which load the same class — pass on the same cell.A baseline entry pins a line number in third-party code. That is fragile across dependency resolutions on its own, and for this shape of issue no line number can ever match. Removing the block for this one file is the narrow fix; the deprecations it covered are now displayed, and
failOnDeprecation="false"keeps them from failing anything. The other 39 vendor files in the baseline carry the same fragility — nothing observed justifies touching them here, and it is worth a separate look.Verified
In the container on PHP 8.4 with the lowest set: unit 494, functional 117, integration 230, all green. On the locked set: PHPUnit 842, PHPStan level max, PHPCS and deptrac clean.
Assisted by claude-code:claude-opus-5 — Session