Applied fixers to improve test robustness - #23018
Conversation
Girgias
left a comment
There was a problem hiding this comment.
Mostly good, but I spotted some bad error messages, if you could fix those before?
| --TEST-- | ||
| readline_completion_function(): Basic test | ||
| --EXTENSIONS-- | ||
| readline | ||
| --FILE-- | ||
| <?php |
There was a problem hiding this comment.
This whole test seems to be extremely useless.
| string(4) "data" | ||
| bool(false) | ||
| bool(false) | ||
| filter_var(): "regexp" option is missing |
There was a problem hiding this comment.
This feels like we should be able to tell what argument is the problem.
| filter_var(): Option must be a valid callback | ||
| filter_var(): Option must be a valid callback | ||
| filter_var(): Option must be a valid callback |
There was a problem hiding this comment.
Ditto arg num would be helpful with the option name
| filter_var_array(): Argument #2 ($options) cannot contain empty keys | ||
| ValueError: filter_var_array(): Argument #2 ($options) cannot contain empty keys | ||
|
|
||
| Warning: filter_var_array(): Unknown filter with ID 0 in %s on line %d |
There was a problem hiding this comment.
Surprised this is a warning, probably should be elevated.
| Warning: preg_replace_callback_array(): No ending delimiter '/' found in %spreg_replace_callback_array2.php on line %d | ||
| NULL | ||
| string(1) "1" | ||
| Exception: 1 |
There was a problem hiding this comment.
What, this error message is clearly broken in some way.
There was a problem hiding this comment.
The output seems correct. But also not quite sure what's actually the goal. Was introduced here 784c60a#diff-3e9a96d15eda69304d23fedb498daa371541c29aa9bff6dd4d170c30241a295dR22 and has a security audit label from much later. Not sure.
| SodiumException: a PHP string is required | ||
| string(3) "bbc" | ||
| string(3) "abc" | ||
| string(3) "bbc" | ||
| string(3) "abc" | ||
| PHP strings are required | ||
| SodiumException: PHP strings are required |
There was a problem hiding this comment.
These should probably be converted to normal TypeErrors
| Exception: test exception | ||
|
|
||
| Exception: test exception | ||
|
|
||
| Exception: test exception |
There was a problem hiding this comment.
Why are there empty new lines all of a sudden?
Extensions with no code owners.