[Dnsmasq-discuss] unittests

Tue Oct 5 15:13:48 UTC 2021

Hey Geert,

On 10/2/21 14:40, Geert Stappers via Dnsmasq-discuss wrote:
> In-Reply-To: <8a018620-25a7-a292-c951-dd2017d54236 at redhat.com>
> On Mon, May 03, 2021 at 12:53:39PM +0200, Petr Menšík wrote:
>> On 4/30/21 12:42 AM, Simon Kelley wrote:
>>> On 14/04/2021 18:35, Petr Menšík wrote:
>>>> Hi Simon and other dnsmasq friends,
>>>>
>>>> after some struggling with Makefile support, I am sending my dnsmasq
>>>> unit tests. It uses another directory with tests specific code. I moved
>>>> some common parts to Makefile.config, in order to be able to reuse them.
>>>> Unit tests are under tests directory with own Makefile.
>>>>
>>>> New target make check should work also from top directory. Some checks
>>>> would work only from tests directory (make kyua). Current coverage is
>>>> rather poor, but I hope can be used as a building block to better tests.
>>>> Especially option parsing tests are easy to write. Testing of sending
>>>> and receiving packets seems to be difficult, it should be tested by
>>>> different kind of test IMHO.
>>>>
>>>> First is attempt to refactor, the second is what evolved into more
>>>> complex set of tests.
>>>>
>>>> Original separate commits are still available on github [1].
>>>>
>>>> What do you think?
>>> Well, I applied the patch, and run "make check" and all the tests passed!
>>>
>>> Now I have to understand how to write new tests.
>> Configuration parsing tests are easy, just provide input parameters
>> similar way to existing test and then check expected values are provided.
>>> Would it make sense to consider some changes to the main code to make
>>> the tests easier? I see that die() is a problem. Can we change the code
>>> in die() to do something useful when testing?
>> I have chosen to omit dnsmasq.c code from tests. It contains main()
>> function, cannot be part of test anyway. Sure, some code changes would
>> help with reducing needed repetitions in tests. Especially init code
>> required in tests should be moved out of dnsmasq.c, where it could be
>> called directly from tests. Shared init code must not be static
>> functions of course.
>>
>> die does make sense everywhere where it is a corner case. If we move
>> die() calls to dnsmasq.c, it would be okay. Other files should return
>> indication of fatal error, but not die directly. It would need
>> additional wrappers in dnsmasq.c, but such functions would be more testable.
>>> Also the tests seem to can copies of initialisation code, does it make
>>> sense to abstract the initialisation in main() so that it can be used by
>>> the tests standalone?
>> Yes, it make sense to move parts of initialization to subsystem-specific
>> initialization functions. I would move dns_init() into rfc1035.c,
>> dhcp_init() into dhcp-common.c etc. It should make main source file
>> shorter and it would be more obvious, which subsystems are initialized
>> in which order, whether they depend on anything before it. I think the
>> best practice is to break long functions into several shorter, more
>> readable functions. I think current main() is a great example to break
>> into more smaller functions and move some of them to shareable files.
>> Parts required by current tests are small enough.
>>> I'm thinking of changing the existing main()
>>>
>>> main()
>>> {
>>>     <initialise stuff>
>>>     while (1)
>>>         events()
>>> }
>>>
>>> into
>>>
>>> main()
>>> {
>>>     init();
>>>     while (1)
>>>       events()
>>> }
>>>
>>> So that init() is available for testing.
>>>
>>>
>>> Cheers,
>>>
>>> Simon.
>>>
>>>> PS: sending this message again, because patch #2 were big enough to
>>>> require moderator's approval. Compressed it as a workaround.
>>>>
>>>> Cheers,
>>>> Petr
>>>>
>>>> 1. https://github.com/InfrastructureServices/dnsmasq/tree/unittests
> What was / is the posting from Simon asking something
>
>   Would unittest have detect this side-effect of the change?

I doubt unit tests would find that. Unit tests should test some
functions that they work correctly. My unit tests were just attempt to
make *some* tests, but just very basic. It was intended more to check
options parsing correctness and obvious breakages in these parts. There
is no function in dnsmasq, where you put "fake" incoming packet and it
would respond reply would look like this.

Unit tests usually require code like Lego, which uses parts of code to
prepare reply to a request. Then virtual responder can be made. Many
parts of dnsmasq are not ready for that. It provides no strong library,
which can replicate internal data processing. Response to a dns packet
is somehow hard to validate just from the code. cmocka library is a good
one for unit tests.

It would be beneficial to have also behavior tests. Which would start
dnsmasq with some parameters and use standard tools like dhclient or dig
to make a request. Then verify response received is correct. That is
what I attempted at separate tests [2], but have not worked on it
recently. It would be much easier to test logic handling interfaces,
processing any weird parameters dig can request, like EDNS client-subnet
support.

I think kyua development has stalled, maybe because it requires legal
agreement on contribution. We use a new tests framework called tmt [3]
in Red Hat, which would be better replacement. A lot of effort is spent
on it, it is quite good. Unfortunately no package for it exists on
Debian. I would move to beakerlib/tmt from my kyua/bats attempt. While
they are not unusable, it would be easier to work with. Combined with
network namespaces of Linux, it is simple to prepare even forwarding
packets from one interface to another one. Directives like --local,
--address or --server would be easier to test using tmt, dig and bunch
of scripts around that.

Well, someday I may find enough time and focus to move it forward
properly. It is a long journey ahead unfortunately. Difference between
test coverage of bind [4] and dnsmasq [5] on Fedora is just a good
illustration.

> I couldn't find it, but could find the above posting.
> Reason for starting a fresh thread is for
> having fresh attention for unittests.
>
>
> Groeten
> Geert Stappers
Regards,
Petr

2. https://github.com/InfrastructureServices/dnsmasq-tests

3. https://github.com/psss/tmt

4. https://src.fedoraproject.org/tests/bind/tree/main

5. https://src.fedoraproject.org/tests/dnsmasq/tree/main

-- 
Petr Menšík
Software Engineer
Red Hat, http://www.redhat.com/
email: pemensik at redhat.com
PGP: DFCF908DB7C87E8E529925BC4931CA5B6C9FC5CB