Hi internals,
We currently have a large number of zpp "variation" tests, which
essentially only test the behavior of zend_parse_parameters plugged into
different functions. A random example of such a test:
https://github.com/php/php-src/blob/master/ext/standard/tests/strings/addslashes_variation1.phpt
This test passes a certain set of input values of different types to a
function with a zpp string argument and observes the behavior. Of course,
there are also hundreds of other functions that accept strings through zpp
and the behavior is always going to be the same.
Based on a quick grep I would estimate that we have around 700 of these
tests. We have a hard policy against adding new tests for zpp behavior, but
I would like to get rid of the existing ones as well. They provide
essentially no value, but are a major chore to update for even for minor
changes.
Regards,
Nikita
Based on a quick grep I would estimate that we have around 700 of these
tests. We have a hard policy against adding new tests for zpp behavior, but
I would like to get rid of the existing ones as well. They provide
essentially no value, but are a major chore to update for even for minor
changes.
I'm +1 on removing (parts of) tests which only test for ZPP failures.
--
Christoph M. Becker
Em qua, 30 de jan de 2019 às 07:46, Nikita Popov nikita.ppv@gmail.com
escreveu:
Hi internals,
We currently have a large number of zpp "variation" tests, which
essentially only test the behavior of zend_parse_parameters plugged into
different functions. A random example of such a test:https://github.com/php/php-src/blob/master/ext/standard/tests/strings/addslashes_variation1.phpt
This test passes a certain set of input values of different types to a
function with a zpp string argument and observes the behavior. Of course,
there are also hundreds of other functions that accept strings through zpp
and the behavior is always going to be the same.Based on a quick grep I would estimate that we have around 700 of these
tests. We have a hard policy against adding new tests for zpp behavior, but
I would like to get rid of the existing ones as well. They provide
essentially no value, but are a major chore to update for even for minor
changes.Regards,
Nikita
Yes, yes, yes!
When I was implementing the https://github.com/php/php-src/pull/3429, was a
pain fixing all those tests.
Gabriel Caruso
Hi!
This test passes a certain set of input values of different types to a
function with a zpp string argument and observes the behavior. Of course,
there are also hundreds of other functions that accept strings through zpp
and the behavior is always going to be the same.
This is true for functions that use standard zpp handling, but there are
a number of them that use custom zpp handling, and for those there could
be corner cases which aren't handled properly. In fact, I've seen some
zpp-using functions that so not properly handle nulls, empty strings,
etc. as arguments (zpp works fine, it's the part past zpp that fails).
So we need to be careful and remove only those tests that are indeed
handled by zpp and not the code beyond it that does need to be tested,
and not remove tests which cover custom argument handling.
--
Stas Malyshev
smalyshev@gmail.com
On Wed, Jan 30, 2019 at 7:39 PM Stanislav Malyshev smalyshev@gmail.com
wrote:
Hi!
This test passes a certain set of input values of different types to a
function with a zpp string argument and observes the behavior. Of course,
there are also hundreds of other functions that accept strings through
zpp
and the behavior is always going to be the same.This is true for functions that use standard zpp handling, but there are
a number of them that use custom zpp handling, and for those there could
be corner cases which aren't handled properly. In fact, I've seen some
zpp-using functions that so not properly handle nulls, empty strings,
etc. as arguments (zpp works fine, it's the part past zpp that fails).
So we need to be careful and remove only those tests that are indeed
handled by zpp and not the code beyond it that does need to be tested,
and not remove tests which cover custom argument handling.
Yes, we shouldn't remove tests for custom argument handling. I've started
working on this in https://github.com/php/php-src/pull/3783 and try to
check for all tests whether this is a simple zpp argument or not. I'm also
switching some cases to use zpp on the PHP 8 branch, where functions had
some custom argument handling for no good reason.
Nikita