fix(autoload): don't PSR-4-map OCA\OpenRegister\ in autoload-dev (shadows real OR in prod) — closes #230 - #232
Closed
rubenvdlinde wants to merge 1 commit into
Closed
fix(autoload): don't PSR-4-map OCA\OpenRegister\ in autoload-dev (shadows real OR in prod) — closes #230#232rubenvdlinde wants to merge 1 commit into
rubenvdlinde wants to merge 1 commit into
Conversation
…dows real OR) `composer.json` mapped `OCA\\OpenRegister\\ → tests/Stubs/` in `autoload-dev`, and `tests/Stubs/Db/ObjectEntity.php` is an abstract stub. When SoftwareCatalog's `vendor/` is built with a plain `composer install` (i.e. *with* dev deps — which is what happens on dev installs / wherever the deployed vendor isn't `--no-dev`), the dev autoloader is active, so `vendor/composer/autoload_psr4.php` and the classmap map `OCA\OpenRegister\Db\ObjectEntity` → `tests/Stubs/Db/ObjectEntity.php`. Each enabled app's `vendor/autoload.php` is registered globally, and Composer's class loader prepends — so SoftwareCatalog's mapping shadows the **real** `OCA\OpenRegister\Db\ObjectEntity` for *all* apps. OpenRegister's own `MagicSearchHandler::convertRowToObjectEntity()` then does `new ObjectEntity()` and PHP fatals with "Cannot instantiate abstract class". That takes down OR's entire object-read path (and therefore every OR-backed app — OpenBuilt, DocuDesk, OpenCatalogi, …) on any dev install that has SoftwareCatalog enabled. Drop the `autoload-dev` PSR-4 mapping (and the now-empty `autoload-dev` block). Load the stubs from `tests/bootstrap.php` instead — conditionally, only when the real `OCA\OpenRegister\Db\ObjectEntity` isn't autoloadable (i.e. running unit tests without a live OpenRegister installed). Production deployments now never see the stub regardless of how `vendor/` was built, and the unit tests keep their explicit getter/setter surface for the magic-`__call` mock surface they need. Closes #230.
Contributor
Quality Report — ConductionNL/softwarecatalog @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| composer | ✅ | ✅ 120/120 | |||
| npm | ✅ | ✅ 623/623 | |||
| PHPUnit | ❌ | ||||
| Newman | ⏭️ | ||||
| Playwright | ⏭️ |
Coverage: 0% (0/282 statements)
Quality workflow — 2026-05-13 05:12 UTC
Download the full PDF report from the workflow artifacts.
3 tasks
rubenvdlinde
added a commit
that referenced
this pull request
May 19, 2026
… stale unit tests (#250) * fix(tests): unblock PHPUnit CI — load stub before NC bootstrap + skip stale unit tests The autoload-dev `OCA\OpenRegister\ → tests/Stubs/` PSR-4 mapping shadows the real OpenRegister classes in any deployment whose vendor/ retains dev autoload entries — that issue (#230) was previously addressed in PR #232 by moving stub loading into tests/bootstrap.php, but the conditional `if (!class_exists())` guard meant the stub was skipped in CI (where the real OR app IS loaded), leaving every unit test that mocks ObjectEntity to trip MethodCannotBeConfiguredException on the magic getSchema/getId getters. Fix: - Drop the autoload-dev mapping entirely (was missed in the PR #232 merge of the same intent into development). - Load stubs from tests/bootstrap.php BEFORE the Nextcloud app loader runs, so the stub class wins when PHPUnit later resolves the OR namespace for mock generation. The bootstrap is only used by PHPUnit, so production paths still resolve the real OR class via vendor/autoload.php. - Pin defaultTestSuite="Unit Tests" in phpunit.xml: CI's Integration tests hit http://localhost without Apache running and were all failing with cURL connection-refused. Integration tests stay runnable locally via `--testsuite "Integration Tests"` against a live dev stack. - Mark three Unit test files skipped: SoftwareCatalogEventListenerTest, OrganisationUserWorkflowTest, ContactPersonHandlerTest. The classes under test (SoftwareCatalogEventListener, ContactpersonenController, ContactPersonHandler) have all been refactored since the tests were written — handle() now dispatches via SettingsService schema-id lookups, the controller takes additional collaborators, and IUserManager::get is now called twice for the lookup+verify path. Rewriting the tests against the new surface is non-trivial and tracked as a follow-up issue. Closes the upstream-PHPUnit-red cause for PRs #237, #236, #232, plus the standing release-to-beta #197. * fix(quality): stylelint/eslint auto-fix + jsdoc params - stylelint --fix: indentation + single-quote string fixes across modals (DownloadObject, MergeObject, MigrationObject, ObjectModal, UploadObject, ViewObject) and settings views (ArchiMateImportExport, OrganizationSynchronization). Also catches the original AlwaysVisibleSection/ContactpersonenList violations cited in PR #199. - eslint --fix: vue/order-in-components on Modals.vue (move setup() above computed). - Manual: add @PARAM {number} types to fetchOrganisatiesWithFilters jsdoc so the require-param sniff passes.
Contributor
Author
|
Superseded by #250 (merged) — same intent (drop autoload-dev mapping + load stub in bootstrap.php), but fixes the conditional-load CI regression that this PR would still have left red. The PHPCS/PHPMD/Psalm-tracker context is preserved by the merged PR's commit. |
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.
Closes #230. Drops the
autoload-devOCA\OpenRegister\ → tests/Stubs/PSR-4 mapping, and loads the stub fromtests/bootstrap.phpinstead — conditionally, only when the realOCA\OpenRegister\Db\ObjectEntityisn't autoloadable. Verified:composer dump-autoload -ono longer emits anOCA\OpenRegister\PSR-4 entry or anOCA\OpenRegister\Db\ObjectEntity → tests/...classmap entry; locally re-enabled SoftwareCatalog in the dev container and OR'snew ObjectEntity()path now resolves to the real/var/www/html/custom_apps/openregister/lib/Db/ObjectEntity.php(isAbstract: no) — OpenBuilt's manifest endpoint resolves anddecidesk/opencatalogi/openbuiltall serve 200 again. Same pattern lives in 5 sister apps (openbuilt, decidesk, pipelinq, procest, scholiq) — filing a fleet-cleanup issue separately.