Skip to content

sapi/fpm: Removes XFAIL for "but test passes" test - #23435

Closed
NickSdot wants to merge 1 commit into
php:masterfrom
NickSdot:tests/fpm-xfail-warn
Closed

NickSdot wants to merge 1 commit into
php:masterfrom
NickSdot:tests/fpm-xfail-warn

Conversation

@NickSdot

Copy link
Copy Markdown
Contributor

76ca6bf removed, ea3a317 fixed race, c4639a2 restored with "Still fails intermittently" (6y ago). Later 1029537 and 4580b8b landed which could be related. Analysed 16,457 CI invocations, ran it thousands of times locally -- at least in CI and locally (macOS) it never fails but warns "XFAIL section but test passes". Worth trying the removal again. If a OS/local specific report hits we can try fixing it properly rather than sledgehammering with XFAIL.

@NickSdot
NickSdot requested a review from bukka as a code owner August 24, 2026 12:21

@Truta446 Truta446 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested this on current master (Linux x86_64, debug NTS): 300/300 isolated runs pass on an idle machine. With all 24 cores busy, 297/300 pass on the first attempt; the other 3 fail once and pass on run-tests' automatic retry (the test calls usleep(), so is_flaky() retries it), which is reported as a WARN and does not fail the job. 25 runs of the whole sapi/fpm/tests suite with -j4 under the same load had no failures. So it's still slightly timing-sensitive under heavy load, but dropping XFAIL turns a permanent "XFAIL section but test passes" warning into a rare "passed on retry" one. 👍

@bukka

bukka commented Oct 28, 2026

Copy link
Copy Markdown
Member

We had actually CI failures for this test on slow CI. I just looked into it more and trying this instead: #24998 . Just removing XFAIL is not correct so closing this.

@bukka bukka closed this Oct 28, 2026
@NickSdot
NickSdot deleted the tests/fpm-xfail-warn branch October 28, 2026 11:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants