Everything

php

C Unit testing and mocking

php.internals

Jakub Zelenka

1 year ago

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

Calvin Buckley

1 year ago

On Dec 16, 2024, at 9:18 AM, Jakub Zelenka <bukka@php.net> wrote:

>
> 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.

Jakub Zelenka

1 year ago

On Mon, Dec 16, 2024 at 4:04 PM Calvin Buckley <calvin@cmpct.info> wrote:

> On Dec 16, 2024, at 9:18 AM, Jakub Zelenka <bukka@php.net> wrote:
> >
> > 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

Christoph Becker

1 year ago

On 16.12.2024 at 14:18, Jakub Zelenka wrote:

> 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

Christoph Becker

1 year ago

On 16.12.2024 at 21:05, Christoph M. Becker 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.

Christoph

Jakub Zelenka

1 year ago

On Thu, Dec 19, 2024 at 1:10 PM Christoph M. Becker <cmbecker69@gmx.de>
wrote:

> On 16.12.2024 at 21:05, Christoph M. Becker 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

Jakub Zelenka

1 year ago

On Mon, Dec 16, 2024 at 9:05 PM Christoph M. Becker <cmbecker69@gmx.de>
wrote:

> On 16.12.2024 at 14:18, Jakub Zelenka 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

Larry Garfield

1 year ago

On Sun, Dec 22, 2024, at 9:23 AM, Jakub Zelenka wrote:

> On Mon, Dec 16, 2024 at 9:05 PM Christoph M. Becker <cmbecker69@gmx.de> wrote:
>> On 16.12.2024 at 14:18, Jakub Zelenka 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.)

I have no opinion or experience on C testing frameworks, other than "yes, tests please!" :-)

--Larry Garfield

Jakub Zelenka

1 year ago

On Tue, Dec 24, 2024 at 8:40 PM Larry Garfield <larry@garfieldtech.com>
wrote:

> On Sun, Dec 22, 2024, at 9:23 AM, Jakub Zelenka wrote:
> > On Mon, Dec 16, 2024 at 9:05 PM Christoph M. Becker <cmbecker69@gmx.de>
> wrote:
> >> On 16.12.2024 at 14:18, Jakub Zelenka 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

Larry Garfield

1 year ago

On Thu, Dec 26, 2024, at 6:25 AM, Jakub Zelenka wrote:

>> 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

Ben Ramsey

1 year ago

> On Dec 16, 2024, at 07:21, Jakub Zelenka <bukka@php.net> wrote:
>
> ﻿
> 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

Jakub Zelenka

1 year ago

On Tue, Dec 24, 2024 at 8:22 PM Ben Ramsey <ben@benramsey.com> wrote:

> On Dec 16, 2024, at 07:21, Jakub Zelenka <bukka@php.net> wrote:
>
> ﻿
> 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

Ben Ramsey

1 year ago