-
Notifications
You must be signed in to change notification settings - Fork 7.8k
Test ftp_ssl_connect() function : error conditions #2584
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.
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
Conversation
<?php | ||
echo "*** Testing ftp_ssl_connect() function : error conditions ***\n"; | ||
echo "\n-- Testing ftp_ssl_connect() function on failure --\n"; | ||
var_dump(ftp_ssl_connect('127.0.0.1')); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No guarantee there isn't a server listening on localhost. Maybe try a hostname of does-not-exist.invalid
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
echo "\n-- Testing ftp_ssl_connect() function timeout warning for value 0 --\n"; | ||
ftp_ssl_connect('127.0.0.1', 21, 0); | ||
|
||
?> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Long as you're at it, add a test for invalid argument type? ftp_ssl_connect(array());
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will do. i'll cover invalid for each of the 3 arguments and also cover exceeds expected number of arguments
I don't possess enough understanding of the c code to figure out how to cover this portion:
if (zend_parse_parameters(ZEND_NUM_ARGS(), "s|ll", &host, &host_len, &port,
&timeout_sec) == FAILURE) {
return;
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@chancegarcia s|ll means:
s - parameter #1 must be a string
| - following parameters are optional
l - parameter #2 must be a numeric (integer)
l - parameter #3 must be a numeric (integer)
To make this fail, you can pass something like
new stdClass
[]
To the first parameter, and the conditional should pass and the return statement is hit. If you look in the ext/standard/tests directory, there should be a lot of error condition tests you can use for reference
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah, ok. so that portion of the code is parse/validating the arguments?
Thanks for explaining it! =D
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@chancegarcia idd, the zend_parse_parameters() function translates the parameters sent to a function call into C variables. You take a look at the README.PARAMETER-PARSING-API or something similar in the root of php-src that has more in-depth about how this works and the meanings =)
- test timeout warning - ensure connection fails with invalid hostname - test invalid parameter types - test exceeds expected number of parameters
made changes and rebased the commits into a single commit. |
Comment on behalf of pollita at php.net: |
I don't know how to run coverage but I'm attempting to cover the return false on failure portion for ftp_ssl_connect and the timeout value warning from
http://gcov.php.net/PHP_HEAD/lcov_html/ext/ftp/php_ftp.c.gcov.php