Re: [Dnsmasq-discuss] [PATCH] unittests
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() > { > > 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 -- Petr Menšík Software Engineer Red Hat, http://www.redhat.com/ email: pemen...@redhat.com PGP: DFCF908DB7C87E8E529925BC4931CA5B6C9FC5CB OpenPGP_signature Description: OpenPGP digital signature ___ Dnsmasq-discuss mailing list Dnsmasq-discuss@lists.thekelleys.org.uk https://lists.thekelleys.org.uk/cgi-bin/mailman/listinfo/dnsmasq-discuss
Re: [Dnsmasq-discuss] [PATCH] unittests
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. 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? 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? I'm thinking of changing the existing main() main() { 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 > > > ___ > Dnsmasq-discuss mailing list > Dnsmasq-discuss@lists.thekelleys.org.uk > https://lists.thekelleys.org.uk/cgi-bin/mailman/listinfo/dnsmasq-discuss > ___ Dnsmasq-discuss mailing list Dnsmasq-discuss@lists.thekelleys.org.uk https://lists.thekelleys.org.uk/cgi-bin/mailman/listinfo/dnsmasq-discuss
Re: [Dnsmasq-discuss] [PATCH] unittests
Hi, > https://github.com/themiron/dnsmasq Thank you for asking, it's my private mirrored master branch and related openssl fork (in separate branch). Updated occasionally from release to release, so hardly can be ~official mirror. -- Best Regards, Vladislav Grishenko > -Original Message- > From: - Neustradamus - > Sent: Thursday, April 15, 2021 7:27 PM > To: Petr Menšík ; dnsmasq- > disc...@lists.thekelleys.org.uk > Cc: webs...@sarvepalli.net; themiron...@gmail.com > Subject: Re: [Dnsmasq-discuss] [PATCH] unittests > Importance: High > > Hello Simon, and all others, > > Here, I see a fork of Dnsmasq on GitHub, thanks Petr :) > > Can you look for https://github.com/dnsmasq organization and add the > https://github.com/dnsmasq/dnsmasq after it? > And remove the old https://github.com/simonkelley/dnsmasq-collab which is a > fork of https://github.com/sei-vsarvepalli/dnsmasq-collab. > > The goal is to have a best development of Dnsmasq, and more contributors, > improvements etc. > It will be better to devs to fork directly the dnsmasq/dnsmasq. > > If this repo is a real mirror (without changes), I can see with Vladislav Grishenko > (themiron) to transfer directly into dnsmasq organization. > - https://github.com/themiron/dnsmasq > > Thanks in advance. > > Regards, > > Neustradamus > > > From: Dnsmasq-discuss on > behalf of Petr Menšík > Sent: Wednesday, April 14, 2021 19:35 > To: dnsmasq-discuss@lists.thekelleys.org.uk > Subject: [Dnsmasq-discuss] [PATCH] unittests > > 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? > > 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 > -- > Petr Menšík > Software Engineer > Red Hat, http://www.redhat.com/ > email: pemen...@redhat.com > PGP: DFCF908DB7C87E8E529925BC4931CA5B6C9FC5CB ___ Dnsmasq-discuss mailing list Dnsmasq-discuss@lists.thekelleys.org.uk https://lists.thekelleys.org.uk/cgi-bin/mailman/listinfo/dnsmasq-discuss
Re: [Dnsmasq-discuss] [PATCH] unittests
Hello Simon, and all others, Here, I see a fork of Dnsmasq on GitHub, thanks Petr :) Can you look for https://github.com/dnsmasq organization and add the https://github.com/dnsmasq/dnsmasq after it? And remove the old https://github.com/simonkelley/dnsmasq-collab which is a fork of https://github.com/sei-vsarvepalli/dnsmasq-collab. The goal is to have a best development of Dnsmasq, and more contributors, improvements etc. It will be better to devs to fork directly the dnsmasq/dnsmasq. If this repo is a real mirror (without changes), I can see with Vladislav Grishenko (themiron) to transfer directly into dnsmasq organization. - https://github.com/themiron/dnsmasq Thanks in advance. Regards, Neustradamus From: Dnsmasq-discuss on behalf of Petr Menšík Sent: Wednesday, April 14, 2021 19:35 To: dnsmasq-discuss@lists.thekelleys.org.uk Subject: [Dnsmasq-discuss] [PATCH] unittests 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? 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 -- Petr Menšík Software Engineer Red Hat, http://www.redhat.com/ email: pemen...@redhat.com PGP: DFCF908DB7C87E8E529925BC4931CA5B6C9FC5CB ___ Dnsmasq-discuss mailing list Dnsmasq-discuss@lists.thekelleys.org.uk https://lists.thekelleys.org.uk/cgi-bin/mailman/listinfo/dnsmasq-discuss
Re: [Dnsmasq-discuss] [PATCH] unittests
The Makefile stuff looks good to me. Will play with the tests soon. de-duplication patch applied. Cheers, Simon. 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? > > 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 > > > ___ > Dnsmasq-discuss mailing list > Dnsmasq-discuss@lists.thekelleys.org.uk > https://lists.thekelleys.org.uk/cgi-bin/mailman/listinfo/dnsmasq-discuss > ___ Dnsmasq-discuss mailing list Dnsmasq-discuss@lists.thekelleys.org.uk https://lists.thekelleys.org.uk/cgi-bin/mailman/listinfo/dnsmasq-discuss