More curl tests - #23333
Conversation
Allow setting target of redirect in test
|
@NickSdot Could you take a look at this? |
NickSdot
left a comment
There was a problem hiding this comment.
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 |
+--------+-------+---------+--------------------+-------------------+--------+---------+
|
|
||
| // 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'); |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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); |
There was a problem hiding this comment.
Version gate in SKIPIF?
php-src/ext/curl/curl.stub.php
Lines 1449 to 1454 in c621cbe
There was a problem hiding this comment.
The minimum curl version PHP can be built with is 7.61, so CURLOPT_MAXFILESIZE_LARGE is always available.
There was a problem hiding this comment.
But then the responder needs a case for this test that sends content length.
There was a problem hiding this comment.
I don't really understand what you mean. I currently test against the "Hello World! Hello World!" response. Do you think that is sufficient?
No description provided.