test: collect the unit tests filinq was never running, and fix the stub gap they hid - #990
Merged
Conversation
…ub gap they hid phpunit.xml collects tests/unit. A second tests/Unit existed beside it holding Controller/SetupControllerTest.php and Service/DemoDataServiceTest.php. On a case-sensitive filesystem those are different directories, so 12 tests have never run in CI. MOVING THEM ALONE WOULD HAVE TURNED DEVELOPMENT RED, which is the interesting part. Six of the twelve error out: Trying to configure method "getAppPath" which cannot be configured because it does not exist, has not been specified, is final, or is static tests/stubs/NextcloudStubs.php declares IAppManager with isInstalled, isEnabledForUser, getAppVersion and getInstalledApps, and stops there. OCP's real interface also declares getAppPath, which DemoDataServiceTest mocks. A stub that is SHORTER than the interface it stands in for does not fail where it is short: PHPUnit refuses to configure the missing method, and the test errors far from the omission, reading as a broken test rather than a stub gap. getAppPath added to the stub with OCP's signature. All 12 pass. The suite goes from 1755 collected tests to 1767, with no failures. Same defect found in larpinq (#669), where the uncollected directory held one file and no stub gap.
rubenvdlinde
requested review from
WilcoLouwerse,
bbrands02 and
rjzondervan
as code owners
September 1, 2026 21:14
Contributor
Quality Report — ConductionNL/filinq @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| test | ✅ | ||||
| test-unit | ✅ | ||||
| check-manifest | ✅ | ||||
| test-l10n | ✅ | ||||
| format | ✅ | ||||
| check-schema-l10n | ✅ | ||||
| check-l10n-js | ✅ | ||||
| composer | ✅ | ✅ 118/118 | |||
| npm | ✅ | ✅ 631/631 | |||
| app:check-code | ⏭️ | ||||
| info.xml | ✅ | ||||
| REUSE | ❌ | ||||
| PHPUnit | ✅ | ||||
| Newman | ✅ | ||||
| Playwright | ⏭️ deferred — runs on the promotion into beta/main, not on a pull request into development | ||||
| Hydra gates | ✅ |
Quality workflow — 2026-09-01 21:27 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
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.
What
phpunit.xmlcollectstests/unit. A secondtests/Unitexisted beside it, holdingController/SetupControllerTest.phpandService/DemoDataServiceTest.php. On a case-sensitive filesystem those are two different directories, so 12 tests have never run in CI.Moving them alone would have turned development red
That is the part worth reading. Six of the twelve error out the moment they are collected:
tests/stubs/NextcloudStubs.phpdeclaresIAppManagerwithisInstalled,isEnabledForUser,getAppVersionandgetInstalledApps, and stops there. OCP's real interface also declaresgetAppPath, whichDemoDataServiceTestmocks.A stub that is shorter than the interface it stands in for does not fail where it is short. PHPUnit refuses to configure the method that is missing, and the test errors far from the omission, so it reads as a broken test rather than a short stub. The test was correct all along.
getAppPathis added to the stub with OCP's signature.Result
All 12 pass. Collected tests go from 1755 to 1767, with no failures.
phpcs --standard=phpcs.xmlexits 0.Scope
The two test files are moved and their namespaces brought onto the convention the collected directory already uses (
OCA\Filinq\Tests\Unit\…; they were on a bareUnit\…). No assertion is changed.Same defect elsewhere
larpinq carries it too, fixed in ConductionNL/larpinq#669. There the uncollected directory held one file and no stub gap. A sweep of the other 19 fleet apps found no third case: every other repo either configures both spellings or configures the one its tests actually live in.