Skip to content

More curl tests - #23333

Open
Sjord wants to merge 11 commits into
php:masterfrom
Sjord:more-curl-tests
Open

More curl tests#23333
Sjord wants to merge 11 commits into
php:masterfrom
Sjord:more-curl-tests

Conversation

@Sjord

@Sjord Sjord commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

No description provided.

@Sjord
Sjord marked this pull request as ready for review August 18, 2026 08:38
@Sjord
Sjord requested a review from adoy as a code owner August 18, 2026 08:38
@Sjord

Sjord commented Aug 18, 2026

Copy link
Copy Markdown
Contributor Author

@NickSdot Could you take a look at this?

@NickSdot NickSdot left a comment

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.

Cannot really judge in detail what's tested, but found some things worth pointing out. Also, can confirm that this closes coverage gaps. ✌️

Coverage: ext/curl

Base: c621cbe27ffe59ed98228724d674dcadcf7f6e8d upstream/master
Tree: 2aa80efcacc776c87712a9beed9487c40be6cffc working tree

+--------+-------+---------+--------------------+-------------------+--------+---------+
|        | Tests | Sources |              Lines |          Branches |   Time |  Memory |
+--------+-------+---------+--------------------+-------------------+--------+---------+
| Base   |   189 |       6 | 2520/2912 (86.54%) | 847/4640 (18.25%) | 64.23s | 45.5 MB |
| Tree   |   197 |       6 | 2538/2912 (87.16%) | 857/4640 (18.47%) | 68.17s | 45.1 MB |
| Change |    +8 |       0 |  +18 / -0 (+0.62%) | +10 / -0 (+0.22%) | +3.94s | -0.4 MB |
+--------+-------+---------+--------------------+-------------------+--------+---------+

Comment thread ext/curl/tests/curl_getinfo_CURLINFO_HEADER_OUT.phpt Outdated
Comment thread ext/curl/tests/curl_setopt_CURLOPT_POSTREDIR.phpt Outdated

// GitHub doesn't actually support SFTP, it does get far enough that the host key callback is called.
$ch = curl_init('sftp://php@github.com/file.txt');
curl_setopt($ch, CURLOPT_SSH_HOSTKEYFUNCTION, 'hostkeyfunction');

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.

Not sure what's up there, but this test says CURLOPT_SSH_HOSTKEYFUNCTION cannot be properly tested. But others do test it; looks like it needs at least a version gate in SKIPIF?

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.

I added the version check.

@Girgias Do you have any insight into why CURLOPT_SSH_HOSTKEYFUNCTION cannot be properly tested?

$ch = curl_init();
curl_setopt($ch, CURLOPT_URL, "{$host}/get.inc");
curl_setopt($ch, CURLOPT_RETURNTRANSFER, true);
curl_setopt($ch, CURLOPT_MAXFILESIZE_LARGE, 10);

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.

Version gate in SKIPIF?

php-src/ext/curl/curl.stub.php

Lines 1449 to 1454 in c621cbe

/* Available since 7.11.1 */
/**
* @var int
* @cvalue CURLOPT_MAXFILESIZE_LARGE
*/
const CURLOPT_MAXFILESIZE_LARGE = UNKNOWN;

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.

The minimum curl version PHP can be built with is 7.61, so CURLOPT_MAXFILESIZE_LARGE is always available.

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.

But then the responder needs a case for this test that sends content length.

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.

I don't really understand what you mean. I currently test against the "Hello World! Hello World!" response. Do you think that is sufficient?

Comment thread ext/curl/tests/curl_setopt_CURLOPT_WRITEHEADER.phpt Outdated
Comment thread ext/curl/tests/curl_setopt_CURLOPT_WRITEHEADER.phpt Outdated
Comment thread ext/curl/tests/curl_setopt_CURLOPT_FNMATCH_FUNCTION.phpt
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.

2 participants