Hi there,
we encountered small annoyances over time which could be easy to fix but would break tests.
Why does this matter? While it is easy to fix the PHP test suite they could potentially affect other projects' code, most probably tests.
Here are the two things on our current list:
Stack traces:
Stack traces for certain constructs do not include the exact location in the stack frame:
$a[(object)null] = 1;
$e->getTrace() => [] # <-- no stack trace
while for
file(); / file(null);
$e->getTrace() => [ ["file"]=> .. ].
The change would probably be something like removing one line
ptr = ptr->prev_execute_data;
In Zend/zend_builtin_functions.c
The BC impact Is that stack traces may contain an additional entry which is used in a lot of tests.
Work-around: Our solution right now is to manually check in the exception handler whether $e->getFile() / $e->getLine() is already contained in $e->getTrace()[0] and otherwise add it to have uniform stack traces. Works, but ugly :-)
Indentation in var_export:
The indentation for
var_export((object)[(object)[]]);
is off by one for nested structures:
(object) array(
'0' => # <-- extra space before index
(object) array(
and the fix would be to change
buffer_append_spaces(buf, level + 2)
to
buffer_append_spaces(buf, level + 1)
in ext/standard/var.c
This breaks about 60 PHP tests relying on the var_export format.
We were hoping for a new function mitigating this issue but https://wiki.php.net/rfc/readable_var_representation https://wiki.php.net/rfc/readable_var_representation seems to get rejected so the issue remains :-)
Is there any way something like there things could/should be tackled for a future PHP version?
I'm torn because of the impact vs. benefit of the improvements.
- Chris
On Fri, Feb 19, 2021 at 8:04 AM Christian Schneider cschneid@cschneid.com
wrote:
Hi there,
we encountered small annoyances over time which could be easy to fix but
would break tests.
Why does this matter? While it is easy to fix the PHP test suite they
could potentially affect other projects' code, most probably tests.Here are the two things on our current list:
Stack traces:
Stack traces for certain constructs do not include the exact location in
the stack frame:
$a[(object)null] = 1;
$e->getTrace() => [] # <-- no stack trace
while for
file(); / file(null);
$e->getTrace() => [ ["file"]=> .. ].
What version are you testing with. I know Niki killed some unnecessary
frames in 8.0's stacktrace output. (A whole repro script is more useful
than a snippet wherein $e comes into being seemingly from nowhere).
Indentation in var_export:
The indentation for
var_export((object)[(object)[]]);
is off by one for nested structures:
(object) array(
'0' => # <-- extra space before index
(object) array(
and the fix would be to change
buffer_append_spaces(buf, level + 2)
to
buffer_append_spaces(buf, level + 1)
in ext/standard/var.cThis breaks about 60 PHP tests relying on the var_export format.
I think shifting the indentation in var_export() could probably be done
without a major BC break. Trimming trailing whitespace would also resolve
one of my long-time annoyances with the function.
Feel free to propose that as a PR and/or an RFC.
-Sara
Am 19.02.2021 um 16:21 schrieb Sara Golemon pollita@php.net:
Stack traces:
What version are you testing with. I know Niki killed some unnecessary frames in 8.0's stacktrace output. (A whole repro script is more useful than a snippet wherein $e comes into being seemingly from nowhere).
I was testing with PHP 8.0.3 but it is the same with master.
You can see the difference at https://3v4l.org/0Pgfo https://3v4l.org/0Pgfo where the first line
try { $a[(object)null] = 1; } catch (Throwable $e) { print_r($e->getTrace()); }
does not create a stack trace. That's what I'd like to change.
What do you think? Should I create a PR for this?
It does break a lot of tests in PHP. Probably less so for other projects but still...
Indentation in var_export:
The indentation for
var_export((object)[(object)[]]);
is off by one for nested structures:
(object) array(
'0' => # <-- extra space before index
(object) array(
and the fix would be to change
buffer_append_spaces(buf, level + 2)
to
buffer_append_spaces(buf, level + 1)
in ext/standard/var.cThis breaks about 60 PHP tests relying on the var_export format.
I think shifting the indentation in
var_export()could probably be done without a major BC break.
An example script is at https://3v4l.org/V8r5k https://3v4l.org/V8r5k
It does break tests though. And my guess it that is would also break tests in quite some projects.
What version would you aim this for? 8.1? 9.0?
Trimming trailing whitespace would also resolve one of my long-time annoyances with the function.
Feel free to propose that as a PR and/or an RFC.
I see your point as it means there have to be trailing white-spaces in tests which can be confusing when editing/creating tests.
I can have a look at how hard this would be to fix...
Side-note: Maybe we should recommend a function to be used for tests. At my work we used to use var_export(), some of our newer tests are using json_encode() but neither is really guaranteed to be stable, right?
- Chris
Side-note: Maybe we should recommend a function to be used for tests. At my work we used to use
var_export(), some of our newer tests are usingjson_encode()but neither is really guaranteed to be stable, right?
Well, the obvious choice to me would be var_dump, which is used by the
majority of PHP's internal tests (see
https://externals.io/message/112967#112971 for some numbers). I was
considering putting together a pull request that standardises on that
for all bundled extensions, fixing the few hundred (out of thousands)
that use var_export or print_r.
However, I've never used PHPT-style tests outside of PHP internals, and
for anything like PHPUnit or PHPSpec you would normally be testing
against actual PHP values, not any string representation; so I don't
really have any opinion on that scenario.
Regards,
--
Rowan Tommins
[IMSoP]