Hi,
I have been looking into how to test some cases where integration tests are
very difficult or even impossible to create for. Those are often found in
networking related and system specific code code (network.c, streams, FPM
and more). I was recently fixing one such bug and decided to give a try
which resulted in this PR: https://github.com/php/php-src/pull/16987 .
There was a suggestion of RFC but that might be a bit too much as it's just
an internal change / addition. But certainly some overview on internals
should be done so writing this instead.
I decided to use cmocka in that PR because I had some experience with that.
It's quite small and still very powerful and allow vast mocking options.
It's a bit manual but it gives a bigger control over the mock. It relies on
--wrap linking option that replaces original functions with wraps. This is
however available only on Linux (or maybe some other Unix variants) but
doesn't work on MacOS or Windows. The developers that want to use it on
those platforms would need to use some Linux Virtualisation option (e.g.
Docker). It also requires static library which is supported by embed SAPI
that can be compiled statically. That limits number of extensions to use
but the main use cases don't really have deps so it should be fine.
I did also some research into the other mocking libraries in C. There is a
Unity with CMock, FFF and some C++ libs like GUnit, Criterion and
Trompeloeil that I looked into. I quickly discarded GUnit and Trompeloeil
as they relay on C++ virtual methods and require wrapping C code to C++
which is very inconvenient. FFF seems too simple and maybe quite inflexible
for our needs as well. Criterion also optionally uses wrap so I didn't see
much advantages compare to cmocka. So it left Unity with CMock that allows
generating custom mocks using a Ruby script. That seemed initially quite
nice but after spending around two hours with trying to make it works for
PHP codebase, I just gave up. It gets quite messy for complex scenarios and
I just didn't figure out how to nicely mock libc functions without any
modification to php-src.
In terms of CI. It has got its own build which is very simple and it tests
just specific parts so we could just limit it to run only for changed files
which might be quite convenient.
So the proposed PR is probably the only reasonable unit testing that I can
come up with. I think it should be completely optional initially for people
to use - more like an experiment. If it becomes used, then good of course.
And if it becomes pain, we can just get rid of it. Has anyone got any
objections to get this merged? If not I plan to merge it early in January.
Cheers
Jakub
Hi,
I have been looking into how to test some cases where integration tests are very difficult or even impossible to create for. Those are often found in networking related and system specific code code (network.c, streams, FPM and more). I was recently fixing one such bug and decided to give a try which resulted in this PR: https://github.com/php/php-src/pull/16987 .
There was a suggestion of RFC but that might be a bit too much as it's just an internal change / addition. But certainly some overview on internals should be done so writing this instead.
I decided to use cmocka in that PR because I had some experience with that. It's quite small and still very powerful and allow vast mocking options. It's a bit manual but it gives a bigger control over the mock. It relies on --wrap linking option that replaces original functions with wraps. This is however available only on Linux (or maybe some other Unix variants) but doesn't work on MacOS or Windows. The developers that want to use it on those platforms would need to use some Linux Virtualisation option (e.g. Docker). It also requires static library which is supported by embed SAPI that can be compiled statically. That limits number of extensions to use but the main use cases don't really have deps so it should be fine.
I did also some research into the other mocking libraries in C. There is a Unity with CMock, FFF and some C++ libs like GUnit, Criterion and Trompeloeil that I looked into. I quickly discarded GUnit and Trompeloeil as they relay on C++ virtual methods and require wrapping C code to C++ which is very inconvenient. FFF seems too simple and maybe quite inflexible for our needs as well. Criterion also optionally uses wrap so I didn't see much advantages compare to cmocka. So it left Unity with CMock that allows generating custom mocks using a Ruby script. That seemed initially quite nice but after spending around two hours with trying to make it works for PHP codebase, I just gave up. It gets quite messy for complex scenarios and I just didn't figure out how to nicely mock libc functions without any modification to php-src.
In terms of CI. It has got its own build which is very simple and it tests just specific parts so we could just limit it to run only for changed files which might be quite convenient.
So the proposed PR is probably the only reasonable unit testing that I can come up with. I think it should be completely optional initially for people to use - more like an experiment. If it becomes used, then good of course. And if it becomes pain, we can just get rid of it. Has anyone got any objections to get this merged? If not I plan to merge it early in January.
Cheers
Jakub
I'm assuming that uses ELF symbol interposition or something like that,
which is why it seems Linux/BSD specific. That seems fragile to me.
I think currently for wanting to test C functions, we're adding custom
functions into ext/zend_test and writing PHPT. Would this work? We
already have that, after all. If not, it'd be helpful to list the
challenges that approach faces.
Hi,
I have been looking into how to test some cases where integration tests
are very difficult or even impossible to create for. Those are often found
in networking related and system specific code code (network.c, streams,
FPM and more). I was recently fixing one such bug and decided to give a try
which resulted in this PR:
https://github.com/php/php-src/pexpectionsull/16987
https://github.com/php/php-src/pull/16987 .There was a suggestion of RFC but that might be a bit too much as it's
just an internal change / addition. But certainly some overview on
internals should be done so writing this instead.I decided to use cmocka in that PR because I had some experience with
that. It's quite small and still very powerful and allow vast mocking
options. It's a bit manual but it gives a bigger control over the mock. It
relies on --wrap linking option that replaces original functions with
wraps. This is however available only on Linux (or maybe some other Unix
variants) but doesn't work on MacOS or Windows. The developers that want to
use it on those platforms would need to use some Linux Virtualisation
option (e.g. Docker). It also requires static library which is supported by
embed SAPI that can be compiled statically. That limits number of
extensions to use but the main use cases don't really have deps so it
should be fine.I did also some research into the other mocking libraries in C. There is
a Unity with CMock, FFF and some C++ libs like GUnit, Criterion and
Trompeloeil that I looked into. I quickly discarded GUnit and Trompeloeil
as they relay on C++ virtual methods and require wrapping C code to C++
which is very inconvenient. FFF seems too simple and maybe quite inflexible
for our needs as well. Criterion also optionally uses wrap so I didn't see
much advantages compare to cmocka. So it left Unity with CMock that allows
generating custom mocks using a Ruby script. That seemed initially quite
nice but after spending around two hours with trying to make it works for
PHP codebase, I just gave up. It gets quite messy for complex scenarios and
I just didn't figure out how to nicely mock libc functions without any
modification to php-src.In terms of CI. It has got its own build which is very simple and it
tests just specific parts so we could just limit it to run only for changed
files which might be quite convenient.So the proposed PR is probably the only reasonable unit testing that I
can come up with. I think it should be completely optional initially for
people to use - more like an experiment. If it becomes used, then good of
course. And if it becomes pain, we can just get rid of it. Has anyone got
any objections to get this merged? If not I plan to merge it early in
January.Cheers
Jakub
I'm assuming that uses ELF symbol interposition or something like that,
which is why it seems Linux/BSD specific. That seems fragile to me.
It is replaced when linking the program by ld (see
https://man7.org/linux/man-pages/man1/ld.1.html - the --wrap option for
more info). The flag should be stable on linux and it is not really fragile
as few mocking libraries are built around it.
I think currently for wanting to test C functions, we're adding custom
functions into ext/zend_test and writing PHPT. Would this work? We
already have that, after all. If not, it'd be helpful to list the
challenges that approach faces.
ext/zend_test is just for normal ext testing that does not require any
mocking. What I need is to test certain flow where mocked function returns
specific response / set output params in certain way. That requires mocking
that function and setting exceptions. This is not possible in zend_test.
The only exception there is
https://github.com/php/php-src/blob/284c4e3e318a75b8133b52c153bc912b48d1bbab/ext/zend_test/test.c#L1499-L1514
which mocks copy_file_range but it uses dlsym with RTLD_NEXT to replace the
function in runtime. I was actually checking if any mocking lib uses dlsym
but it is not as it's probably a bit more fragile than wrap. And without
the mocking lib, it's not really useful because we would have to
re-implement what they do to support expectations from different tests
(some global state manager) and other features. So I didn't really go
there...
Regards
Jakub
I have been looking into how to test some cases where integration tests are
very difficult or even impossible to create for. Those are often found in
networking related and system specific code code (network.c, streams, FPM
and more). I was recently fixing one such bug and decided to give a try
which resulted in this PR: https://github.com/php/php-src/pull/16987 .
It makes sense to me to have some unit tests for code which is otherwise
hard or even impossible to test via integration tests.
There was a suggestion of RFC but that might be a bit too much as it's just
an internal change / addition. But certainly some overview on internals
should be done so writing this instead.
I'm fine with not going through the RFC process, although the policy[1]
police might come after us. :)
I decided to use cmocka in that PR because I had some experience with that.
It's quite small and still very powerful and allow vast mocking options.
It's a bit manual but it gives a bigger control over the mock. It relies on
--wrap linking option that replaces original functions with wraps. This is
however available only on Linux (or maybe some other Unix variants) but
doesn't work on MacOS or Windows. The developers that want to use it on
those platforms would need to use some Linux Virtualisation option (e.g.
Docker).
Especially on Windows, where we have different code paths, and sometimes
even completely different code, it would be great to also have these
unit tests. Given that link.exe supports /alternatename, a bit of
additional macro magic might do the trick[2]. I'll try to have a stab
at this soon.
I did also some research into the other mocking libraries in C.
Thank you! I barely have any experience with unit testing in C (only
one time, long ago, wrote part of unit test for timelib which is using
Cpputest), but from a cursory glance cmocka appears to be suitable for
our purposes.
In terms of CI. It has got its own build which is very simple and it tests
just specific parts so we could just limit it to run only for changed files
which might be quite convenient.
If the tests won't take long, in my opinion we could run them for every
push. Or at least in nightly (plus when relevant files changed), to
avoid that the suite doesn't run for a long time (possibly weeks), and
then suddenly is broken for external reasons.
[1] https://github.com/php/policies/blob/main/third-party-code.rst
[2]
https://stackoverflow.com/questions/33790425/visual-studio-c-linker-wrap-option
Christoph
Especially on Windows, where we have different code paths, and sometimes
even completely different code, it would be great to also have these
unit tests. Given that link.exe supports /alternatename, a bit of
additional macro magic might do the trick[2]. I'll try to have a stab
at this soon.
I had a closer look, and it's getting pretty tricky. First, we would
need a static php.lib (doable, but so far not supported by the build
system). Then, apparently, we would need to get rid of the
__declspec(dllimport) at least for the functions we want to mock when
building php.lib; to do that it might be necessary to use modified
copies of the WindowSDK headers. Ugly.
And then we might need an own unit test suite for Windows; at least as
is, test_network.c makes quite some assumptions regarding code paths
(e.g. that poll(2) is called), which are not portable. I'm not sure
it's worth pursuing this for other platforms than Linux.
Christoph
On Thu, Dec 19, 2024 at 1:10 PM Christoph M. Becker cmbecker69@gmx.de
wrote:
Especially on Windows, where we have different code paths, and sometimes
even completely different code, it would be great to also have these
unit tests. Given that link.exe supports /alternatename, a bit of
additional macro magic might do the trick[2]. I'll try to have a stab
at this soon.I had a closer look, and it's getting pretty tricky. First, we would
need a static php.lib (doable, but so far not supported by the build
system). Then, apparently, we would need to get rid of the
__declspec(dllimport) at least for the functions we want to mock when
building php.lib; to do that it might be necessary to use modified
copies of the WindowSDK headers. Ugly.And then we might need an own unit test suite for Windows; at least as
is, test_network.c makes quite some assumptions regarding code paths
(e.g. that poll(2) is called), which are not portable. I'm not sure
it's worth pursuing this for other platforms than Linux.
Agreed that running the tests on Windows might not be worth the effort.
However, many of the tests likely aren't strictly Linux-specific, so even
if they only run on Linux, they can still help improve the quality of code
across all platforms. So it's good for Windows as well :)
Regards,
Jakub
On Mon, Dec 16, 2024 at 9:05 PM Christoph M. Becker cmbecker69@gmx.de
wrote:
There was a suggestion of RFC but that might be a bit too much as it's
just
an internal change / addition. But certainly some overview on internals
should be done so writing this instead.I'm fine with not going through the RFC process, although the policy[1]
police might come after us. :)
I think it fits to all inclusion criteria and doesn't go against any
exclusion. Maybe except that "de facto standard" but for our use there was
really no other option as I mentioned in my comparison so it was the only
library left for our needs if there is only one, then it's "de facto
standard" we could say.
Btw. It was probably mistake to set that policy for C code because we don't
really need to care if PHP recommends any tool there - I completely missed
it when voting for it. This should be just for PHP application that we care
about. We should modify that policy accordingly - I need to make a list of
changes that to the policies as there are quite a few points.
Regards,
Jakub
There was a suggestion of RFC but that might be a bit too much as it's just
an internal change / addition. But certainly some overview on internals
should be done so writing this instead.I'm fine with not going through the RFC process, although the policy[1]
police might come after us. :)I think it fits to all inclusion criteria and doesn't go against any
exclusion. Maybe except that "de facto standard" but for our use there
was really no other option as I mentioned in my comparison so it was
the only library left for our needs if there is only one, then it's "de
facto standard" we could say.Btw. It was probably mistake to set that policy for C code because we
don't really need to care if PHP recommends any tool there - I
completely missed it when voting for it. This should be just for PHP
application that we care about. We should modify that policy
accordingly - I need to make a list of changes that to the policies as
there are quite a few points.Regards,
Jakub
Point of order: The recently adopted 3rd party code policy does not apply to C tooling. It mentions "PHP Tooling", which is defined as "PHP code run by PHP.net". The website, docs tooling, etc. It has no bearing on what C libraries or toolchains can or should be used in php-src itself. (Whether that's unit testing, url parsers, HTML parsers, threading libraries, etc.)
I have no opinion or experience on C testing frameworks, other than "yes, tests please!" :-)
--Larry Garfield
On Tue, Dec 24, 2024 at 8:40 PM Larry Garfield larry@garfieldtech.com
wrote:
On Mon, Dec 16, 2024 at 9:05 PM Christoph M. Becker cmbecker69@gmx.de
wrote:There was a suggestion of RFC but that might be a bit too much as
it's just
an internal change / addition. But certainly some overview on
internals
should be done so writing this instead.I'm fine with not going through the RFC process, although the policy[1]
police might come after us. :)I think it fits to all inclusion criteria and doesn't go against any
exclusion. Maybe except that "de facto standard" but for our use there
was really no other option as I mentioned in my comparison so it was
the only library left for our needs if there is only one, then it's "de
facto standard" we could say.Btw. It was probably mistake to set that policy for C code because we
don't really need to care if PHP recommends any tool there - I
completely missed it when voting for it. This should be just for PHP
application that we care about. We should modify that policy
accordingly - I need to make a list of changes that to the policies as
there are quite a few points.Regards,
Jakub
Point of order: The recently adopted 3rd party code policy does not apply
to C tooling. It mentions "PHP Tooling", which is defined as "PHP code run
by PHP.net". The website, docs tooling, etc. It has no bearing on what C
libraries or toolchains can or should be used in php-src itself. (Whether
that's unit testing, url parsers, HTML parsers, threading libraries, etc.)
Ah ok, thanks for the clarification. This part (Definitions -> Librarires)
is slightly confusing in this regard though:
Refers to existing third party code packages or tools, either C
extensions or PHP code, maintained by someone other than the PHP Internals
team.
Specifically that C extensions part made it sound like it also applies to C
code. But after reading the introduction, it's probably more as you say,
and it applies only to PHP code.
Cheers
Jakub
Point of order: The recently adopted 3rd party code policy does not apply to C tooling. It mentions "PHP Tooling", which is defined as "PHP code run by PHP.net". The website, docs tooling, etc. It has no bearing on what C libraries or toolchains can or should be used in php-src itself. (Whether that's unit testing, url parsers, HTML parsers, threading libraries, etc.)
Ah ok, thanks for the clarification. This part (Definitions ->
Librarires) is slightly confusing in this regard though:Refers to existing third party code packages or tools, either C extensions or PHP code, maintained by someone other than the PHP Internals team.
Specifically that C extensions part made it sound like it also applies
to C code. But after reading the introduction, it's probably more as
you say, and it applies only to PHP code.Cheers
Jakub
Ah, I can see how that could be interpreted that way, but that wasn't the intent. "C extensions" in this case means things like the MongoDB PECL extension, or Redis PECL extension. It's saying that if we wanted to start using Redis in the website somewhere (I dunno why, this is hypothetical), and that means using the Redis C PECL extension, that's allowed.
--Larry Garfield
Hi,I have been looking into how to test some cases where integration tests are very difficult or even impossible to create for. Those are often found in networking related and system specific code code (network.c, streams, FPM and more). I was recently fixing one such bug and decided to give a try which resulted in this PR: https://github.com/php/php-src/pull/16987 .
There was a suggestion of RFC but that might be a bit too much as it's just an internal change / addition. But certainly some overview on internals should be done so writing this instead.
I decided to use cmocka in that PR because I had some experience with that. It's quite small and still very powerful and allow vast mocking options. It's a bit manual but it gives a bigger control over the mock. It relies on --wrap linking option that replaces original functions with wraps. This is however available only on Linux (or maybe some other Unix variants) but doesn't work on MacOS or Windows. The developers that want to use it on those platforms would need to use some Linux Virtualisation option (e.g. Docker). It also requires static library which is supported by embed SAPI that can be compiled statically. That limits number of extensions to use but the main use cases don't really have deps so it should be fine.
I did also some research into the other mocking libraries in C. There is a Unity with CMock, FFF and some C++ libs like GUnit, Criterion and Trompeloeil that I looked into. I quickly discarded GUnit and Trompeloeil as they relay on C++ virtual methods and require wrapping C code to C++ which is very inconvenient. FFF seems too simple and maybe quite inflexible for our needs as well. Criterion also optionally uses wrap so I didn't see much advantages compare to cmocka. So it left Unity with CMock that allows generating custom mocks using a Ruby script. That seemed initially quite nice but after spending around two hours with trying to make it works for PHP codebase, I just gave up. It gets quite messy for complex scenarios and I just didn't figure out how to nicely mock libc functions without any modification to php-src.
In terms of CI. It has got its own build which is very simple and it tests just specific parts so we could just limit it to run only for changed files which might be quite convenient.
So the proposed PR is probably the only reasonable unit testing that I can come up with. I think it should be completely optional initially for people to use - more like an experiment. If it becomes used, then good of course. And if it becomes pain, we can just get rid of it. Has anyone got any objections to get this merged? If not I plan to merge it early in January.
Cheers
Jakub
FWIW, as someone still very new to C, I found Criterion quite easy to use, and I was able to quickly grasp its concepts and start using it right away. I can’t say the same for other C testing libraries I tried.
Cheers,
Ben
Hi,I have been looking into how to test some cases where integration tests
are very difficult or even impossible to create for. Those are often found
in networking related and system specific code code (network.c, streams,
FPM and more). I was recently fixing one such bug and decided to give a try
which resulted in this PR: https://github.com/php/php-src/pull/16987 .There was a suggestion of RFC but that might be a bit too much as it's
just an internal change / addition. But certainly some overview on
internals should be done so writing this instead.I decided to use cmocka in that PR because I had some experience with
that. It's quite small and still very powerful and allow vast mocking
options. It's a bit manual but it gives a bigger control over the mock. It
relies on --wrap linking option that replaces original functions with
wraps. This is however available only on Linux (or maybe some other Unix
variants) but doesn't work on MacOS or Windows. The developers that want to
use it on those platforms would need to use some Linux Virtualisation
option (e.g. Docker). It also requires static library which is supported by
embed SAPI that can be compiled statically. That limits number of
extensions to use but the main use cases don't really have deps so it
should be fine.I did also some research into the other mocking libraries in C. There is a
Unity with CMock, FFF and some C++ libs like GUnit, Criterion and
Trompeloeil that I looked into. I quickly discarded GUnit and Trompeloeil
as they relay on C++ virtual methods and require wrapping C code to C++
which is very inconvenient. FFF seems too simple and maybe quite inflexible
for our needs as well. Criterion also optionally uses wrap so I didn't see
much advantages compare to cmocka. So it left Unity with CMock that allows
generating custom mocks using a Ruby script. That seemed initially quite
nice but after spending around two hours with trying to make it works for
PHP codebase, I just gave up. It gets quite messy for complex scenarios and
I just didn't figure out how to nicely mock libc functions without any
modification to php-src.In terms of CI. It has got its own build which is very simple and it tests
just specific parts so we could just limit it to run only for changed files
which might be quite convenient.So the proposed PR is probably the only reasonable unit testing that I can
come up with. I think it should be completely optional initially for people
to use - more like an experiment. If it becomes used, then good of course.
And if it becomes pain, we can just get rid of it. Has anyone got any
objections to get this merged? If not I plan to merge it early in January.Cheers
Jakub
FWIW, as someone still very new to C, I found Criterion quite easy to use,
and I was able to quickly grasp its concepts and start using it right away.
I can’t say the same for other C testing libraries I tried.
I just checked Criterion a bit more and it actually does not have built-in
mocking. It could be used with --wrap but it doesn't have any expectations
like cmocka. There's Mimick [1] from the same author which is indendent but
its docs still say that it's experimental so probably not a good idea to
use an experimental tool as its API can change. So all in all it's not
probably an option for us.
[1] https://github.com/Snaipe/Mimick
Cheers
Jakub