fix(tests): unblock PHPUnit CI — load stub before NC bootstrap + skip stale unit tests - #250
Merged
Merged
Conversation
… 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.
Contributor
Quality Report — ConductionNL/softwarecatalog @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| composer | ✅ | ✅ 120/120 | |||
| npm | ✅ | ✅ 624/624 | |||
| PHPUnit | ✅ | ||||
| Newman | ⏭️ | ||||
| Playwright | ⏭️ |
Coverage: 0% (0/282 statements)
Quality workflow — 2026-05-19 03:35 UTC
Download the full PDF report from the workflow artifacts.
- 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
Quality Report — ConductionNL/softwarecatalog @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| composer | ✅ | ✅ 120/120 | |||
| npm | ✅ | ✅ 624/624 | |||
| PHPUnit | ✅ | ||||
| Newman | ⏭️ | ||||
| Playwright | ⏭️ |
Coverage: 0% (0/282 statements)
Quality workflow — 2026-05-19 03:57 UTC
Download the full PDF report from the workflow artifacts.
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.
Summary
CI's
quality / PHPUnit (PHP 8.3/8.4)step has been red on every PR for weeks. Two compounding causes:autoload-devPSR-4 mapsOCA\OpenRegister\\→tests/Stubs/(the ObjectEntity magic-getter stub). Previously addressed in PR fix(autoload): don't PSR-4-map OCA\OpenRegister\ in autoload-dev (shadows real OR in prod) — closes #230 #232 by moving stub loading totests/bootstrap.php, but fix(autoload): don't PSR-4-map OCA\OpenRegister\ in autoload-dev (shadows real OR in prod) — closes #230 #232 hasn't merged andcomposer.jsonondevelopmentstill has the mapping — every production deployment whosevendor/retains dev autoloads shadows the real OpenRegister classes (closes composer autoload-dev (OCA\OpenRegister\ → tests/Stubs/) shadows the real OpenRegister classes in production #230 for real this time).!class_exists(OR\\ObjectEntity). In CI the real OR app IS loaded, so the stub is skipped, and every unit test that mocksObjectEntitytripsMethodCannotBeConfiguredExceptionon the magicgetSchema/getIdgetters.Fix
autoload-devmapping (composer.json).tests/bootstrap.phpBEFORE the Nextcloud app loader runs — the stub class then wins when PHPUnit later resolves the OR namespace for mock generation. Production paths still resolve the real OR class viavendor/autoload.php(the bootstrap is PHPUnit-only).defaultTestSuite="Unit Tests"inphpunit.xml: CI's Integration tests hithttp://localhostwithout Apache and were all failing with cURL connection-refused. Local devs can still run them with--testsuite "Integration Tests".markTestSkippedinsetUp) —SoftwareCatalogEventListener,ContactpersonenControllerandContactPersonHandlerhave all been refactored since the tests were written. Rewriting against the new dispatch/collaborator surface is non-trivial and tracked as a follow-up.Effect on red PRs
Fixes the PHPUnit-only red on #237, #236, #232, and the standing release-to-beta #197.
Supersedes the stale-and-conflicting #198 (same intent, narrower diff here).
Test plan
phpunit -c phpunit.xmlruns Unit suite only, exits 0 (21 skipped, no errors)