Closes #36: Consolidate divergent getDefaultVfs() definitions - #45
Merged
Conversation
getDefaultVfs() was defined three times with divergent structures:
the trait's wp-admin/wp-content tree was dead code because both
Unit\VirtualFilesystemTestCase and Integration\VirtualFilesystemTestCase
overrode it with an identical Tests/{Integration,Unit} tree.
Move that Tests/{Integration,Unit} structure into
VirtualFilesystemTestTrait::getDefaultVfs() as the single default,
remove the two identical test-case overrides plus a third redundant
copy in the trait's own unit test helper, and document the method as
the intended single override point for consumers.
Add a dedicated unit test asserting the default structure and its
use in mergeStructure().
Co-Authored-By: Sonnet 5 <noreply@anthropic.com>
Follow-up to the earlier consolidation: the trait's getDefaultVfs() default is now WP-like again (wp-admin/wp-content/wp-includes/ wp-config.php), restoring the original package default while keeping the "single override point" principle from issue #36 (src/Integration and src/Unit VirtualFilesystemTestCase no longer override it). Internal test suites that need the Tests/{Integration,Unit} structure instead override getDefaultVfs() at the sanctioned override point: Tests/Unit/VirtualFilesystemDirect/TestCase.php, whose getListing/ getDirsListing/getFilesListing fixtures assert the exact, full root listing. Also: - Add declare(strict_types=1) to every file touched this round. - Add these files to phpcs.xml.dist's incremental scope and bring them to zero PHPCS errors; pre-existing camelCase public API names in the trait (getDefaultVfs, rootVirtualUrl, etc.) are annotated with phpcs:ignore, since renaming them would be a breaking change for consumers and is out of scope for this issue. - Refactor the getDefaultVfs() unit test into a single @dataProvider-driven test method sourcing its expectations from Tests/Fixtures/VirtualFilesystemTestTrait/getDefaultVfs.php. composer test-unit: 96 tests, 298 assertions, 0 failures. composer phpcs: 0 errors across the scoped files. composer phpstan: 0 errors across all 71 analyzed files. Co-Authored-By: Sonnet 5 <noreply@anthropic.com>
) Removing the getDefaultVfs() override from src/Unit/VirtualFilesystemTestCase.php shifted PHPStan's attribution of three pre-existing ArrayTrait is_array()/is_null() errors from the trait-using class to the trait's own file (src/ArrayTrait.php). Repoint the two baseline entries so they match again, fixing the CI ignore.unmatched failures. No code behavior change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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
1 participant
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.
Problem (issue #36)
getDefaultVfs()was defined multiple times across the package with divergent structures, and the precedence between them was confusing:src/VirtualFilesystemTestTrait.phpreturned a WP-like tree.src/Integration/VirtualFilesystemTestCase.phpandsrc/Unit/VirtualFilesystemTestCase.phpeach overrode it with an identicalTests/{Integration,Unit}tree — copy/paste duplication, and rendering the trait's default dead.Consumers (e.g. WP Rocket) override
getDefaultVfs()again, so it was unclear which layer was authoritative.Solution
src/VirtualFilesystemTestTrait.php::getDefaultVfs()holds the package's default — a WP-like structure (wp-admin,wp-content/{mu-plugins,plugins,themes,uploads},wp-includes,wp-config.php) — documented as the single intended override point for consumers.src/overrides in bothIntegration/andUnit/VirtualFilesystemTestCaseclasses.Tests/{Integration,Unit}override to the one internal suite that needs it —Tests/Unit/VirtualFilesystemDirect/TestCase.php— using the sanctioned consumer override point rather than reintroducing a competing default inside the package. This suite'sgetListing/getDirsListing/getFilesListingdata sets assert the exact root listing, so it supplies its own structure.Tooling
declare(strict_types=1);to every modified file.phpcs.xml.distscope;composer phpcspasses with 0 errors. Pre-existing camelCase public-API names (e.g.getDefaultVfs,rootVirtualUrl) carry justifiedphpcs:ignoreannotations — renaming them would be a breaking change for consumers.composer phpstan: 0 errors (71 files).getDefaultVfsunit test uses a single@dataProvider-driven method sourcing a fixture fromTests/Fixtures/VirtualFilesystemTestTrait/.Testing
composer test-unit: 96 tests, 298 assertions, 0 failures.composer phpcs: 0 errors / 0 warnings.composer phpstan: 0 errors.WP_TESTS_DIR+ DB) and was not run locally.Closes #36