Skip to content

ext/curl: speed up tests #23228

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Sjord wants to merge 4 commits into
base: master
Choose a base branch
from
Open

ext/curl: speed up tests #23228

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
55f2ccf
ext/curl: speed up tests
Sjord Aug 12, 2026
3ce3929
ext/curl: smarter wait for server in test
Sjord Aug 12, 2026
8fc637d
ext/curl: disable expect/continue in tests
Sjord Aug 12, 2026
4a4d800
ext/curl: restore server timeout of 3 seconds
Sjord Aug 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions ext/curl/tests/bug54798-unix.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ function checkForClosedFilePointer($host, $curl_option, $description) {

if (CURLOPT_INFILE == $curl_option) {
curl_setopt($ch, CURLOPT_UPLOAD, 1);
curl_setopt($ch, CURLOPT_HTTPHEADER, ['Expect:']);

@mvorisek mvorisek Aug 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Disable Expect: 100-continue to prevent libcurl's 1-second delay.

May I ask where the 1 second delay come from?

@Sjord Sjord Aug 13, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Expect 100-continue is a flow control mechanism that is apparently not supported by the PHP development server. Curl waits one second for a 100-continue response and then continues anyway.

https://everything.curl.dev/http/post/expect100.html

Unfortunately, lots of servers in the world do not properly support the Expect: header or do not handle it correctly, so curl only waits 1000 milliseconds for that first response before it continues anyway.

You can avoid the wait entirely by using -H Expect: to remove the header

@Sjord Sjord Aug 13, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

PHP development server does not support Expect 100-continue flow control · Issue #23242 · php/php-src

}

curl_setopt($ch, $curl_option, $fp);
Expand Down
1 change: 1 addition & 0 deletions ext/curl/tests/bug54798.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ function checkForClosedFilePointer($host, $curl_option, $description) {

if (CURLOPT_INFILE == $curl_option) {
curl_setopt($ch, CURLOPT_UPLOAD, 1);
curl_setopt($ch, CURLOPT_HTTPHEADER, ['Expect:']);
}

curl_setopt($ch, $curl_option, $fp);
Expand Down
1 change: 1 addition & 0 deletions ext/curl/tests/curl_pause_001.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ $inputHandle = fopen(__FILE__, 'r');
$ch = curl_init();
curl_setopt($ch, CURLOPT_URL, "{$host}/get.inc?test=input");
curl_setopt($ch, CURLOPT_UPLOAD, 1);
curl_setopt($ch, CURLOPT_HTTPHEADER, ['Expect:']);
curl_setopt($ch, CURLOPT_READFUNCTION, new Input);
curl_setopt($ch, CURLOPT_INFILE, $inputHandle);
curl_setopt($ch, CURLOPT_RETURNTRANSFER, 1);
Expand Down
3 changes: 2 additions & 1 deletion ext/curl/tests/curl_read_function_error_on_int.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ include 'server.inc';
$host = curl_cli_server_start();
$ch = curl_init();
curl_setopt($ch, CURLOPT_URL, "{$host}/get.php?test=post");
curl_setopt($ch, CURLOPT_POST, ['f' => 'f']);
curl_setopt($ch, CURLOPT_POST, 1);
curl_setopt($ch, CURLOPT_HTTPHEADER, ['Expect:']);
curl_setopt($ch, CURLOPT_TIMEOUT, 2);
curl_setopt($ch, CURLOPT_READFUNCTION, "custom_readfunction" );

Expand Down
1 change: 1 addition & 0 deletions ext/curl/tests/curl_readfunc_abort.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ $host = curl_cli_server_start();
$ch = curl_init();
curl_setopt($ch, CURLOPT_URL, "{$host}/get.inc");
curl_setopt($ch, CURLOPT_POST, 1);
curl_setopt($ch, CURLOPT_HTTPHEADER, ['Expect:']);
curl_setopt($ch, CURLOPT_READFUNCTION, function () {
return CURL_READFUNC_ABORT;
});
Expand Down
1 change: 1 addition & 0 deletions ext/curl/tests/curl_readfunction_throws_abort.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ $ch = curl_init("{$host}/get.inc");

$file = new CURLFile(__DIR__ . '/curl_testdata1.txt');
curl_setopt($ch, CURLOPT_POST, 1);
curl_setopt($ch, CURLOPT_HTTPHEADER, ['Expect:']);

echo "Test: read function throws exception\n";
curl_setopt($ch, CURLOPT_READFUNCTION,
Expand Down
5 changes: 4 additions & 1 deletion ext/curl/tests/curl_setopt_ssl.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,10 @@ if ($process === false) {
}
try {
// Give the server time to start
sleep(1);
for ($i = 0; $i < 100; $i++) {

@NickSdot NickSdot Aug 12, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
for ($i = 0; $i < 100; $i++) {
for ($i = 0; $i < 50; $i++) {

Maybe 50 is enough? Would match the previous 1s; fsockopen already adds extra.

@Sjord Sjord Aug 13, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, 50 (or even 10 or 20) would work. 100 is just a number that's high but not infinite. In normal operation this loop will only iterate a couple of times, and 50 or 100 is never reached.

If you are worried about the time this test takes when the server fails to start, perhaps a better way is to get the server process status with proc_get_status and stop the test when the server is no longer running.

@NickSdot NickSdot Aug 13, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Was just a nit; wouldn't over-complicate it personally. A server not starting would die out before we arrive here. It's rather for slower envs -- git history related to the server mentions some Travis CI (and other) edge cases. As you said, normally the max should not be hit -- since the 1s didn't flake 50ish is perhaps just fine.

if (@fsockopen('127.0.0.1', $port)) break;
usleep(20000);
}

echo "case 1: client cert and key from string\n";
$ch = curl_init("https://127.0.0.1:$port/");
Expand Down
12 changes: 6 additions & 6 deletions ext/curl/tests/server.inc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ function curl_cli_server_start() {
// First, wait for the dev server to declare itself ready.
$bound = null;
stream_set_blocking($pipes[2], false);
for ($i = 0; $i < 60; $i++) {
usleep(50000); // 50ms per try

@devnexen devnexen Aug 13, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Here both retry loops keep $i < 60 while the sleep goes 50ms → 20ms, so each wait budget quietly drops from 3s to 1.2s.

@Sjord Sjord Aug 13, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes. I thought that was fine when I made it, but when I looked some more into it I saw that #2304 specifically increased the timeout to 3 seconds, so I increased the loop count to 150 in this PR to keep the 3 second timeout.

for ($i = 0; $i < 150; $i++) {
usleep(20000); // 20ms per try
$status = proc_get_status($handle);
if (empty($status['running'])) {
echo "Server is not running\n";
Expand Down Expand Up @@ -44,8 +44,7 @@ function curl_cli_server_start() {
// note: even when server prints 'Listening on localhost:8964...Press Ctrl-C to quit.'
// it might not be listening yet...need to wait until fsockopen() call returns
$error = "Unable to connect to server\n";
for ($i=0; $i < 60; $i++) {
usleep(50000); // 50ms per try
for ($i=0; $i < 150; $i++) {
$status = proc_get_status($handle);
$fp = @fsockopen("tcp://$bound");
// Failure, the server is no longer running
Expand All @@ -58,6 +57,7 @@ function curl_cli_server_start() {
$error = '';
break;
}
usleep(20000); // 20ms per try
}

if ($fp) {
Expand All @@ -74,12 +74,12 @@ function curl_cli_server_start() {
function($handle) {
proc_terminate($handle);
/* Wait for server to shutdown */
for ($i = 0; $i < 60; $i++) {
for ($i = 0; $i < 150; $i++) {
$status = proc_get_status($handle);
if (!($status && $status['running'])) {
break;
}
usleep(50000);
usleep(20000);
}
},
$handle
Expand Down
Loading