test(e2e): kill two hypotheses about the timeouts, and warm the App Store - #153
Merged
Merged
Conversation
added 3 commits
August 19, 2026 13:50
…old start The five remaining e2e failures were all `Timeout 20000ms exceeded` on OCS calls, and one of them — `unauthenticated callers cannot list tokens` — hits ApiController::listPats(), which returns 403 on its first line and makes no outbound call. A handler that CANNOT be slow, timing out, means the request was delayed rather than the endpoint being slow. Ruled out first: the built-in server already runs with PHP_CLI_SERVER_WORKERS=8, so it is not request serialisation. That leaves Nextcloud's bruteforce protection, doing exactly its job. An e2e suite fires many deliberately-unauthenticated and wrong-credential requests from one IP, which is indistinguishable from an attack, so responses get progressively delayed. The same thing was diagnosed on portaliq, where our own runs tripped the throttle and the failures read as product defects. Disabled for the disposable CI instance only. What is being suppressed is a response to the suite's own traffic, not anything about the app; no spec asserts on throttling. Separately, the discover specs search the real App Store, and the FIRST call fetches and parses the whole catalogue — which is what pushed `discover(calendar)` past 20s, including the diagnostic request meant to explain the failure. Warming it in setup moves that cost to where it is allowed to be slow. Not fatal: an unreachable App Store still fails those specs, as it should. This only stops a cold cache being mistaken for a broken endpoint.
Disabling the bruteforce throttle did not clear the Timeout 20000ms failures (89 passed / 8 failed, against 89 / 5 before), so that hypothesis is dead. The config did apply — the seed step runs under set -e and succeeded — so the throttle was simply not the cause. Rather than a third guess costing another 25-minute round trip, this times both endpoints from the runner, twice each. The second call is the control: if call 2 is fast the cost is cold start and warming is the fix; if both are slow the handler is slow and raising the timeout would only hide it.
Best run yet: 91 passed / 7 failed / 0 skipped. The probe answered a question I was not asking. Both endpoints came back in ~60ms — but with http=401, because the curl is unauthenticated and both check authorization before doing any work. It measured the short-circuit, not the App Store fetch that makes the authenticated path slow. Kept anyway, because it settles one thing conclusively: the SERVER is not slow. `pat-management:104` is itself an unauthenticated test, and it times out at 20s in Playwright against an endpoint curl answers in 60ms — so that failure is the shared `page.request` context queueing behind an in-flight browser request, not a slow handler. Deliberately NOT extended with basic auth to reach the real path: basic-auth curl pays a bcrypt hash per request and inflates every timing, which would turn a measurement into a misleading number. Two hypotheses are now dead — request serialisation (PHP_CLI_SERVER_WORKERS is already 8) and the bruteforce throttle (disabled here, failures unchanged). The remaining cause is the runner's slow access to the external App Store, and the fix is to make those specs hermetic the way the forge fixture already does for forges. That is a change with its own design, not a 2am guess.
Contributor
Quality Report — ConductionNL/app-versions @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| composer | ✅ | ✅ 29/29 | |||
| npm | ✅ | ✅ 282/282 | |||
| app:check-code | ⏭️ | ||||
| info.xml | ✅ | ||||
| REUSE | ❌ | ||||
| PHPUnit | ✅ | ||||
| Newman | ⏭️ | ||||
| Playwright | ❌ | ||||
| Hydra gates | ✅ |
Quality workflow — 2026-08-19 13:22 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.
Best run yet: 91 passed / 7 failed / 0 skipped (from 89/5 and, before any of this work, 22 passed with 66 hidden).
This branch does three things and, as much to the point, rules two explanations out.
Dead hypothesis 1 — request serialisation.
php -Sis single-worker by default, which would make one slow request stall everything. Not it:PHP_CLI_SERVER_WORKERS: 8is already set on that step in the shared workflow.Dead hypothesis 2 — the bruteforce throttle. An e2e suite fires many unauthenticated requests from one IP, which looks exactly like an attack, and the same thing was diagnosed on portaliq. Disabled here for the disposable CI instance… and the failures did not change (5 → 8, while flaky went 4 → 1: the same tests, no longer recovering on retry). The config did apply — the seed step runs under
set -eand succeeded — so the throttle simply was not the cause. Kept, because suppressing a response to the suite's own traffic is correct for a throwaway instance and nothing asserts on throttling.The measurement that settles it. Both timing-out endpoints answer in ~60 ms, twice each:
pat-management:104is itself an unauthenticated test that times out at 20 s against an endpoint curl answers in 60 ms. That failure is the sharedpage.requestcontext queueing behind an in-flight browser request, not a slow handler.It is deliberately not extended with basic auth to reach the real path: basic-auth curl pays a bcrypt hash per request and inflates every timing, turning a measurement into a misleading number.
What remains, stated rather than papered over: the runner's slow access to the external App Store. The fix is to make those specs hermetic the way the forge fixture already does for forges — a change with its own design, not a 2 a.m. guess. No timeout has been raised to make the symptom disappear.