Skip to content

perf(tests): enforce PHPUnit time limits with sensible timeout values#60739

Open
miaulalala wants to merge 6 commits into
masterfrom
perf/noid/phpunit-test-timeouts
Open

perf(tests): enforce PHPUnit time limits with sensible timeout values#60739
miaulalala wants to merge 6 commits into
masterfrom
perf/noid/phpunit-test-timeouts

Conversation

@miaulalala
Copy link
Copy Markdown
Contributor

@miaulalala miaulalala commented May 26, 2026

Summary

  • Adds enforceTimeLimit="true" and defaultTimeLimit="300" to both phpunit-autotest.xml and phpunit-autotest-external.xml
  • Reduces timeout values from the blanket 900s default to 60s / 300s / 600s for small / medium / large annotated tests
  • Adds ini-values: disable_functions="" to all phpunit workflow files that use ini-file: development
  • Reduces DB inserts in AbstractMappingTestCase::testGetListOfIdsByDn from 3,332 to 14

Why

The timeoutForSmallTests/Medium/Large attributes had no effect because enforceTimeLimit was never set to true. A hanging test could silently block a CI job for up to 15 minutes before the runner-level timeout kicked in.

On defaultTimeLimit: Nearly all existing tests have no size annotation (#[Small]/#[Medium]/#[Large]). Without defaultTimeLimit, PHPUnit 10 falls back to a 1-second default for unannotated tests, which broke the entire suite. Setting defaultTimeLimit="300" is an intentional safety net: it delivers the primary value of this change (hung tests cut at 5 min instead of 15) without requiring a mass annotation effort upfront.

Annotating tests with explicit sizes is better done incrementally — when a test is being modified, the author adds #[Small] or #[Large] as appropriate. This PR establishes the framework; annotation coverage can grow naturally from here.

On ini-values: disable_functions="": PHP's development ini preset disables pcntl_signal via disable_functions. PHPUnit's enforceTimeLimit relies on pcntl_signal to interrupt hung tests; without it, PHPUnit emits a runner-level warning ("pcntl extension required for enforcing time limits"). Because our workflows pass --fail-on-warning, that warning fails the entire job. Clearing disable_functions with ini-values: disable_functions="" re-enables pcntl_signal so enforcement actually works and the warning is suppressed.

On testGetListOfIdsByDn: The test intentionally uses 66,640 DNs (just over PostgreSQL's 65,535-value IN list limit) to verify that getListOfIdsByDn() correctly chunks and merges queries. However, it was also mapping every 20th DN, resulting in 3,332 DB inserts — far more than needed to verify the chunking behaviour. Reduced to every 5,000th DN (14 inserts) which still crosses the chunk boundary and validates the merge logic, but runs in a fraction of the time on slow backends like Oracle.

Notes

  • If CI surfaces further tests that legitimately exceed 300s, annotate them #[Large] rather than raising the default

Test plan

  • Verify no unexpected TimeoutException failures across all PHPUnit jobs
  • Confirm the "pcntl extension required" warning no longer appears in any job
  • Check all jobs pass cleanly end-to-end

🤖 Generated with Claude Code

The timeout attributes were defined but never enforced because
enforceTimeLimit was missing. Adds enforceTimeLimit="true" and reduces
values from the 900s blanket default to 60s/300s/600s for
small/medium/large tests. Unannotated tests default to medium (300s),
cutting the worst-case hang time per test from 15 min to 5 min.

Signed-off-by: Anna Larch <anna@larch.dev>
AI-Assisted-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Anna Larch <anna@nextcloud.com>
@miaulalala miaulalala requested review from skjnldsv and susnux May 26, 2026 15:20
@miaulalala miaulalala self-assigned this May 26, 2026
@miaulalala miaulalala added 3. to review Waiting for reviews performance 🚀 tests Related to tests CI labels May 26, 2026
@miaulalala miaulalala added this to the Nextcloud 35 milestone May 26, 2026
@miaulalala miaulalala marked this pull request as ready for review May 26, 2026 20:02
@miaulalala miaulalala requested a review from a team as a code owner May 26, 2026 20:02
@miaulalala miaulalala requested review from Altahrim, icewind1991, kesselb, provokateurin and salmart-dev and removed request for a team May 26, 2026 20:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review Waiting for reviews CI performance 🚀 tests Related to tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant