Re: [Dnsmasq-discuss] [PATCH] dnsmasq unit tests!
On Thu, May 21, 2020 at 04:32:01PM +0200, Geert Stappers wrote: > On Thu, May 14, 2020 at 09:08:09PM +0200, Petr Menšík wrote: > > My previous message is waiting for moderation, since my patch is a big one. > > > > On 5/12/20 10:51 PM, Geert Stappers wrote: > > > On Mon, May 04, 2020 at 05:02:38PM +0200, Petr Menšík wrote: > > ... > > > > > > I think it is great to have unittests. > > > > > > > > > > > > To be continued ... > > OK, patch cleaned up, should apply without any warning now. > > > > Yes, works fine. Fixes the parameter count mismatch that was seen > earlier. One unittest failed as warned before. And yes indeed > the Harald Jensas's patch 'Fix regression in s_config_in_context() > method' does solve it. > > > Cheers > Geert Stappers > (Curious who also > says: "Unittests? Yes, have it in the dnsmasq source tree!" ) > Rewording my curiousity: Who would like to have unittests in the dnsmasq source tree AND has courage to express that? Groeten Geert Stappers -- stappers@alpaca:~/src/dnsmasq $ make check make: *** No rule to make target 'check'. Stop. stappers@alpaca:~/src/dnsmasq $ ___ 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] dnsmasq unit tests!
On Thu, May 14, 2020 at 09:08:09PM +0200, Petr Menšík wrote: > My previous message is waiting for moderation, since my patch is a big one. > > On 5/12/20 10:51 PM, Geert Stappers wrote: > > On Mon, May 04, 2020 at 05:02:38PM +0200, Petr Menšík wrote: > ... > > > > I think it is great to have unittests. > > > > > > > > To be continued ... > OK, patch cleaned up, should apply without any warning now. > Yes, works fine. Fixes the parameter count mismatch that was seen earlier. One unittest failed as warned before. And yes indeed the Harald Jensas's patch 'Fix regression in s_config_in_context() method' does solve it. Cheers Geert Stappers (Curious who also says: "Unittests? Yes, have it in the dnsmasq source tree!" ) ___ Dnsmasq-discuss mailing list Dnsmasq-discuss@lists.thekelleys.org.uk http://lists.thekelleys.org.uk/mailman/listinfo/dnsmasq-discuss
Re: [Dnsmasq-discuss] [PATCH] dnsmasq unit tests!
Hi Geert, I have made some modifications and cleanup. I added make check target to main Makefile, otherwise it is kept separate. Because I have discovered today kyua tool is not packaged (yet) in Debian (and also Ubuntu), I made just a simple shell wrapper around tests in Makefile. It produces more detailed text output. I were also able to run automated checks on every push, using commit [1]. Check actions in my project copy [2]. Seems pretty cool. 1. https://github.com/InfrastructureServices/dnsmasq/commit/2411fe8aa4aefefedb01c7c3a0846995f65fd789 2. https://github.com/InfrastructureServices/dnsmasq/actions On 5/12/20 10:51 PM, Geert Stappers wrote: > On Mon, May 04, 2020 at 05:02:38PM +0200, Petr Menšík wrote: >> Hello everyone, >> >> we have merged support for multiple IPv6 addresses to our release in >> RHEL. We tried to ensure it does not break anything and we failed. >> >> I made already some dnsmasq tests in separate repository [1], running in >> network namespaces. There are two kinds of tests. Simple shell backed >> bats tests in bash. bats and bash packages are required to run them. >> Second kind are few tests in beakerlib [2], which is test framework used >> in Fedora and RHEL testing. They exist and can test few things. >> >> But now, I have accomplished creating few unit tests [3] for dnsmasq. >> They are kind of hack, but they should allow basic testing of options >> working. I used cmocka library. Dnsmasq is not very well prepared for >> unit testing, but some parts can be tested. It is much easier to test >> just code, without providing fake network configuration. I want to use >> it to ensure no change in DHCP breaks expected behaviour. It is much >> easier to prepare code changes than full fledged functional test, >> emulating real request over network. >> >> I would love if you could try it and tell me what you think about it. > > I think it is great to have unittests. > > >> I am attaching squished patch, separate commits are at our github [3]. >> If someone would like to add some test, please create a pull request! >> >> If you would like to try it: >> git clone -b unittests https://github.com/InfrastructureServices/dnsmasq.git >> cd dnsmasq >> make >> cd tests >> make >> ./option_test >> ./dhcp_test >> >> Since these are related to dnsmasq internals, I think merge to master >> would be nice eventually. Some parts of dnsmasq should be adjusted for >> easier testing, I have to prepare some changes. It might be starting >> block to ensure new releases do not break existing functionality. >> >> Any opinions would be appreciated too. > > I do not know Harry Callahan. What would he say? > > > > >> Cheers, >> Petr >> >> 1. https://github.com/InfrastructureServices/dnsmasq-tests >> 2. https://github.com/beakerlib/beakerlib >> 3. https://github.com/InfrastructureServices/dnsmasq/tree/unittests/tests >> >> Petr Menšík >> Software Engineer > > Be aware that I'm a "sysadmin", not a programmer ... > I have got also few functional tests[1] if you are more interested in those. But also sysadmin experience is welcome ;) > >> >From 9395cc84f93c63573ba28e4e349c44adb5dbb34d Mon Sep 17 00:00:00 2001 >> From: =?UTF-8?q?Petr=20Men=C5=A1=C3=ADk?= >> Date: Mon, 4 May 2020 16:26:17 +0200 >> Subject: [PATCH] Create unittests with dhcp and option tests >> MIME-Version: 1.0 >> Content-Type: text/plain; charset=UTF-8 >> Content-Transfer-Encoding: 8bit >> >> diff --git a/Makefile b/Makefile >> index 78e25f0..e390745 100644 >> --- a/Makefile >> +++ b/Makefile >> @@ -24,7 +24,7 @@ MANDIR= $(PREFIX)/share/man >> LOCALEDIR = $(PREFIX)/share/locale >> BUILDDIR = $(SRC) >> DESTDIR = >> -CFLAGS= -Wall -W -O2 >> +CFLAGS= -Wall -W -O2 -ggdb > > How does that effect the regular builds? Adds unnecessary debug information. This was added by my mistake, it was not intended to be there. In new patch, it is removed. > > > >> LDFLAGS = >> COPTS = >> RPM_OPT_FLAGS = > > > > > > stappers@alpaca:~/src/dnsmasq > $ git am unittests.patch > Patch format detection failed. > stappers@alpaca:~/src/dnsmasq > $ git am < unittests.patch > Applying: Create unittests with dhcp and option tests > /home/stappers/src/dnsmasq/.git/rebase-apply/patch:469: trailing > whitespace. > > /home/stappers/src/dnsmasq/.git/rebase-apply/patch:535: trailing > whitespace. > /* Min buffer size: we check after adding each record, so there must > * be > /home/stappers/src/dnsmasq/.git/rebase-apply/patch:538: trailing > whitespace. > This might be increased is EDNS packet size if greater than the > minimum. */ > /home/stappers/src/dnsmasq/.git/rebase-apply/patch:541: trailing > whitespace. > > /home/stappers/src/dnsmasq/.git/rebase-apply/patch:610: trailing > whitespace. > size_t answer_auth(struct dns_header *header, char *limit, size_t qlen, > time_t now, union mysockaddr *peer_addr, > warning: squelched 1 whitespace error > warning: 6 lines
Re: [Dnsmasq-discuss] [PATCH] dnsmasq unit tests!
My previous message is waiting for moderation, since my patch is a big one. On 5/12/20 10:51 PM, Geert Stappers wrote: > On Mon, May 04, 2020 at 05:02:38PM +0200, Petr Menšík wrote: ... > > I think it is great to have unittests. > > > > > > > > >> 1. https://github.com/InfrastructureServices/dnsmasq-tests >> 2. https://github.com/beakerlib/beakerlib >> 3. https://github.com/InfrastructureServices/dnsmasq/tree/unittests/tests > > Be aware that I'm a "sysadmin", not a programmer ... > > >> diff --git a/Makefile b/Makefile >> index 78e25f0..e390745 100644 >> --- a/Makefile >> +++ b/Makefile >> @@ -24,7 +24,7 @@ MANDIR= $(PREFIX)/share/man >> LOCALEDIR = $(PREFIX)/share/locale >> BUILDDIR = $(SRC) >> DESTDIR = >> -CFLAGS= -Wall -W -O2 >> +CFLAGS= -Wall -W -O2 -ggdb > > How does that effect the regular builds? It was added by mistake, thank you for pointing to it. >> LDFLAGS = >> COPTS = >> RPM_OPT_FLAGS = > > > > To be continued ... OK, patch cleaned up, should apply without any warning now. > -- Petr Menšík Software Engineer Red Hat, http://www.redhat.com/ email: pemen...@redhat.com PGP: DFCF908DB7C87E8E529925BC4931CA5B6C9FC5CB >From 5a6b0fdd66d2e6854f1a08e900a6db5d2e7b2d09 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20Men=C5=A1=C3=ADk?= Date: Thu, 14 May 2020 16:04:49 +0200 Subject: [PATCH] Create unit tests for dnsmasq MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Squashed commit of the following: commit 966f5c3b62548e51c39a77f5883c786bd929fe7e Author: Petr Menšík Date: Thu May 14 15:53:47 2020 +0200 Remove trailing spaces commit 4a1dfb40f5d15097d5dd1d9e37412c4a05427720 Author: Petr Menšík Date: Thu May 14 14:41:19 2020 +0200 Make check target work even without kyua Debian still does not have kyua package in repositories. Make simpler custom runner in Makefile to run unit tests without it. commit 8c3dea491eb60d177718998443225884bf771099 Author: Petr Menšík Date: Mon May 11 19:40:52 2020 +0200 Add check target to main Makefile It is common to use check target. Make it available for dnsmasq. commit fc895df9b265d899177dda6901c8ef323a501c27 Author: Petr Menšík Date: Thu May 7 14:41:36 2020 +0200 Add makefile rule to first build dnsmasq If dnsmasq sources are changed, keep dnsmasq up-to-date. commit 2082bda8a1a2255bb83dc19e66fe08ecf7b2d3b4 Author: Petr Menšík Date: Thu May 7 14:40:40 2020 +0200 Stop passing CONFIG_ADDR to find_config Fixes also error in alt-host check, which fixes test for correct regression fix. commit 84e5823ad370d151739933f3d9488385d8655922 Author: Petr Menšík Date: Thu May 7 13:54:18 2020 +0200 Add speed test for searching dhcp entries Allows simple customization in number of repeats and hosts count. commit ba4d72ac1e96d4c9775971461e3ccc9836d76635 Author: Petr Menšík Date: Wed May 6 19:41:31 2020 +0200 Rename functions to correlate to dhcp_config Rename macro to a name well describing what it does. Rename sort functions to dhcp_config prefix. commit b0472dbcfdea26a065245762f178176c461675cb Author: Petr Menšík Date: Tue May 5 23:24:19 2020 +0200 Create wildcard specific comparison for macs. commit 6b041c9b6c7c1ad8ceb4cd99f1855be50bf5e87a Author: Petr Menšík Date: Tue May 5 23:23:27 2020 +0200 Add tagged host and wildcard hwaddr checks tags are now supported in dhcp-host. Ensure the most specific tag is used if it matches. Check also wildcard match does work. commit d6f9530aba65cc296ed943352e1428ae91eeb515 Author: Petr Menšík Date: Tue May 5 23:20:25 2020 +0200 Create comparing function for config sorting Prepare set of functions for comparing two configs. Allows sorting of dhcp-hosts lines from configuration according to their importance. Sorted entries would allow searching most similar record without walking the list again and again. Just return first entry matching criteria. commit bc04b97874df589be00c569ae2d0db2c5211da1a Author: Petr Menšík Date: Tue May 5 14:21:12 2020 +0200 Add testing of hex parsing and handling It is used for mac address parsing for DHCP and in DHCP options. commit 19ca3d8b5afe0f28b39569c6b5d8f311fb322170 Author: Petr Menšík Date: Mon May 4 21:36:40 2020 +0200 Add test target using kyua List all tests in Kyuafile and run test. Requires kyua. commit 429eedae1505c295522c5685ab088318081ef542 Author: Petr Menšík Date: Mon May 4 16:00:31 2020 +0200 Add mixed DHCP test entries for both IPv4 and IPv6 Ensure it works also with both ways. commit 51167220db819ab736f38c905eee22e792bd0dbf Author: Petr Menšík Date: Mon May 4 15:42:45 2020 +0200 Merge all objs and add licenses Stop using workarounds for option test, use full objects from dnsmasq, without few exceptions. Updated also all files to include license headers.
Re: [Dnsmasq-discuss] [PATCH] dnsmasq unit tests!
On Mon, May 04, 2020 at 05:02:38PM +0200, Petr Menšík wrote: > Hello everyone, > > we have merged support for multiple IPv6 addresses to our release in > RHEL. We tried to ensure it does not break anything and we failed. > > I made already some dnsmasq tests in separate repository [1], running in > network namespaces. There are two kinds of tests. Simple shell backed > bats tests in bash. bats and bash packages are required to run them. > Second kind are few tests in beakerlib [2], which is test framework used > in Fedora and RHEL testing. They exist and can test few things. > > But now, I have accomplished creating few unit tests [3] for dnsmasq. > They are kind of hack, but they should allow basic testing of options > working. I used cmocka library. Dnsmasq is not very well prepared for > unit testing, but some parts can be tested. It is much easier to test > just code, without providing fake network configuration. I want to use > it to ensure no change in DHCP breaks expected behaviour. It is much > easier to prepare code changes than full fledged functional test, > emulating real request over network. > > I would love if you could try it and tell me what you think about it. I think it is great to have unittests. > I am attaching squished patch, separate commits are at our github [3]. > If someone would like to add some test, please create a pull request! > > If you would like to try it: > git clone -b unittests https://github.com/InfrastructureServices/dnsmasq.git > cd dnsmasq > make > cd tests > make > ./option_test > ./dhcp_test > > Since these are related to dnsmasq internals, I think merge to master > would be nice eventually. Some parts of dnsmasq should be adjusted for > easier testing, I have to prepare some changes. It might be starting > block to ensure new releases do not break existing functionality. > > Any opinions would be appreciated too. > Cheers, > Petr > > 1. https://github.com/InfrastructureServices/dnsmasq-tests > 2. https://github.com/beakerlib/beakerlib > 3. https://github.com/InfrastructureServices/dnsmasq/tree/unittests/tests > > Petr Menšík > Software Engineer Be aware that I'm a "sysadmin", not a programmer ... > >From 9395cc84f93c63573ba28e4e349c44adb5dbb34d Mon Sep 17 00:00:00 2001 > From: =?UTF-8?q?Petr=20Men=C5=A1=C3=ADk?= > Date: Mon, 4 May 2020 16:26:17 +0200 > Subject: [PATCH] Create unittests with dhcp and option tests > MIME-Version: 1.0 > Content-Type: text/plain; charset=UTF-8 > Content-Transfer-Encoding: 8bit > > diff --git a/Makefile b/Makefile > index 78e25f0..e390745 100644 > --- a/Makefile > +++ b/Makefile > @@ -24,7 +24,7 @@ MANDIR= $(PREFIX)/share/man > LOCALEDIR = $(PREFIX)/share/locale > BUILDDIR = $(SRC) > DESTDIR = > -CFLAGS= -Wall -W -O2 > +CFLAGS= -Wall -W -O2 -ggdb How does that effect the regular builds? > LDFLAGS = > COPTS = > RPM_OPT_FLAGS = stappers@alpaca:~/src/dnsmasq $ git am unittests.patch Patch format detection failed. stappers@alpaca:~/src/dnsmasq $ git am < unittests.patch Applying: Create unittests with dhcp and option tests /home/stappers/src/dnsmasq/.git/rebase-apply/patch:469: trailing whitespace. /home/stappers/src/dnsmasq/.git/rebase-apply/patch:535: trailing whitespace. /* Min buffer size: we check after adding each record, so there must * be /home/stappers/src/dnsmasq/.git/rebase-apply/patch:538: trailing whitespace. This might be increased is EDNS packet size if greater than the minimum. */ /home/stappers/src/dnsmasq/.git/rebase-apply/patch:541: trailing whitespace. /home/stappers/src/dnsmasq/.git/rebase-apply/patch:610: trailing whitespace. size_t answer_auth(struct dns_header *header, char *limit, size_t qlen, time_t now, union mysockaddr *peer_addr, warning: squelched 1 whitespace error warning: 6 lines add whitespace errors. stappers@alpaca:~/src/dnsmasq $ To be continued ... ___ Dnsmasq-discuss mailing list Dnsmasq-discuss@lists.thekelleys.org.uk http://lists.thekelleys.org.uk/mailman/listinfo/dnsmasq-discuss
